Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

trim cls when proccess it into html #143

Merged
merged 1 commit into from
Apr 28, 2015
Merged

trim cls when proccess it into html #143

merged 1 commit into from
Apr 28, 2015

Conversation

sladex
Copy link

@sladex sladex commented Apr 24, 2015

No description provided.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling b58a412 on sladex:cls-trim into ee446ea on bem:master.

@mishanga
Copy link
Member

@sladex Думаешь, нужно?

@sladex
Copy link
Author

sladex commented Apr 24, 2015

Если напрямую задавать класс, то написать лишний пробел сложно: ctx.cls('btn');.
Ошибиться можно, если: класс приезжает с данными из вне, класс получается из какого-либо выражения (склейка строк, регулярка, ...).
По-моему шаблонизатор вправе делать такие преобразования и это хорошо, если он следит за красивым выводом.

@mishanga
Copy link
Member

Я смотрю на это с точки зрения производительности. Во-первых, пример довольно синтетический. Во-вторых, лишние пробелы ничего не поломают.
Перфекционизм — это прекрасно, но в данном случае он приводит к увеличению: а) объема кода, б) количества тестов, в) времени шаблонизации. А практической пользы — ноль.
Я такие изменения стараюсь не вносить.

@sladex
Copy link
Author

sladex commented Apr 28, 2015

Хорошие замечания.
Учитывая, что задание дополнительного CSS-класса не должно быть востребованной функцией при работе по BEM-методологии.
в) на скорость это изменение либо не повлияет вообще (если json.cls не задан, то и следующий код не выполнится...), либо очень нечзначительно (нативный метод trim очень быстр...), б) по-моему вообще не проблема, а) да, тут нечего противопоставить).

mishanga added a commit that referenced this pull request Apr 28, 2015
trim cls when proccess it into html
@mishanga mishanga merged commit 8f49e4a into bem:master Apr 28, 2015
@mishanga
Copy link
Member

Viva el perfeccionismo!

@qfox
Copy link
Member

qfox commented Apr 28, 2015

@mishanga я тож сомневался по началу 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants