bem / bh

BH template engine
http://bem.github.io/bh/
MIT License
68 stars 31 forks source link

add custom short tags #149

Closed awinogradov closed 9 years ago

awinogradov commented 9 years ago

Уж очень хочется кастомные теги

coveralls commented 9 years ago

Coverage Status

Coverage decreased (-0.25%) to 99.75% when pulling 0356b904bb4d6a2a489978c4013facc5b0a9c7d9 on theprotein:feature/tags-as-options into b70bfea37e1da7e4b25f53fd039ffb3a0b4ab367 on bem:master.

mishanga commented 9 years ago

Можешь привести пример? Ну и большая просьба добавить тесты и документацию про это.

awinogradov commented 9 years ago

Ex: <g><rect /></g>

Тестов добавлю!

Ср, 3 июня 2015 г. в 18:28, Mikhail Troshev notifications@github.com:

Можешь привести пример? Ну и большая просьба добавить тесты и документацию про это.

— Reply to this email directly or view it on GitHub https://github.com/bem/bh/pull/149#issuecomment-108483657.

sladex commented 9 years ago

@verybigman только в pr ты перезаписываешь объект по которому проверяется принадлежность к самозакрывающимся тегам. По идее надо добавлять опциональные теги к имеющемуся набору?

awinogradov commented 9 years ago

Была мысль про extend да, но быстро сделать не получится. Надо дотаскивать this.extend

sladex commented 9 years ago

В любом случае так задавать свои теги издевательство:

shortTags: { rect: 1, path: 1 }

В таком виде они хранятся из соображений производительности. Разумнее списком: ['rect', 'path']. И копировать его обычным for-ом в setOptions.

mishanga commented 9 years ago

+1 к @sladex

awinogradov commented 9 years ago

Тогда можно и дефолтные так хранить, а новые пушить в setOptions Чт, 4 июня 2015 г. в 15:59, Mikhail Troshev notifications@github.com:

+1 к @sladex https://github.com/sladex

— Reply to this email directly or view it on GitHub https://github.com/bem/bh/pull/149#issuecomment-108886828.

qfox commented 9 years ago

Тогда можно и дефолтные так хранить, а новые пушить в setOptions

@verybigman Ну тут речь про хранить против передавать. Передавать в виде массива, а внутри хранить хешом.

awinogradov commented 9 years ago

Какой сакральный смысл хранить хешом и не создавать его в опциях? Это упростит экстенд в разы Чт, 4 июня 2015 г. в 20:39, Alexej Yaroshevich notifications@github.com:

Тогда можно и дефолтные так хранить, а новые пушить в setOptions

@verybigman https://github.com/verybigman Ну тут речь про хранить против передавать. Передавать в виде массива, а внутри хранить хешом.

— Reply to this email directly or view it on GitHub https://github.com/bem/bh/pull/149#issuecomment-108986902.

awinogradov commented 9 years ago

Айда я сделаю нормальный экстенд?)

sladex commented 9 years ago

@verybigman Смысл не в хранении, а в том как и где это потом используется внутри bh. Проверка по хэшу на порядки быстрее, чем поиск по массиву через Array.prototype.indexOf. Ссылку на сравнение производительности я уже приводил выше. В принципе, если делать копирование кастомных тегов, то можно и дефолтные в коде bh привести в виде массива:

['area', 'base', 'br', 'col', 'command', 'embed', 'hr', 'img', 'input', 'keygen', 'link', 'menuitem', 'meta', 'param', 'source', 'track', 'wbr']

Потом кастомные теги через Array.prototype.concat удобно приклеивать будет. А перебор по массиву и копирование в объект делать придется и так, и так...

awinogradov commented 9 years ago

@sladex все так. Храним дефолтные в массиве. А в месте склеивания превращаем в объект. Производительность никак не страдает. Поиск так и будет по объекту.

sladex commented 9 years ago

@verybigman sounds good.

coveralls commented 9 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 4633e27b273ed5c19d5926dc01dc01a7573ab613 on theprotein:feature/tags-as-options into b70bfea37e1da7e4b25f53fd039ffb3a0b4ab367 on bem:master.

mishanga commented 9 years ago

@verybigman по реализации ок, по тестам отписал. И добавь еще в документацию, пожалуйста.

awinogradov commented 9 years ago

Ок Пт, 5 июня 2015 г. в 22:43, Mikhail Troshev notifications@github.com:

@verybigman https://github.com/verybigman по реализации ок, по тестам отписал. И добавь еще в документацию, пожалуйста.

— Reply to this email directly or view it on GitHub https://github.com/bem/bh/pull/149#issuecomment-109417912.

awinogradov commented 9 years ago

Не в одну строку правда, но кажется так понятней, да и все выше тесты так написаны.

coveralls commented 9 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 955508538e9b74e4e488bf618d06358a3bb17c1b on theprotein:feature/tags-as-options into b70bfea37e1da7e4b25f53fd039ffb3a0b4ab367 on bem:master.

mishanga commented 9 years ago

@verybigman поправишь остальное?

awinogradov commented 9 years ago

Да, конечно. Я в отпуске просто:) поправлю сегодня завтра Пн, 8 июня 2015 г. в 13:52, Mikhail Troshev notifications@github.com:

@verybigman https://github.com/verybigman поправишь остальное?

— Reply to this email directly or view it on GitHub https://github.com/bem/bh/pull/149#issuecomment-109948963.

mishanga commented 9 years ago

@verybigman конфликт при ребейзе. Поправишь?

awinogradov commented 9 years ago

Поправлю. Я в отпуске до 23го)

awinogradov commented 9 years ago

ready to merge!

mishanga commented 9 years ago

Спасибо!