Update 'ignoreComments' option #116

eGavr · 2014-12-30T13:24:07Z

The new realization of this option:

ignoreComments: {
    conditionalComments: false,
    normalComments: true
}

arikon · 2015-02-03T13:48:40Z

@j0tunn @eGavr Стоит сделать

eGavr · 2015-02-03T13:51:15Z

А зачем нужны комментарии?

alexbaumgertner · 2015-02-03T14:14:40Z

@eGavr чтобы не плодить файлы тестов, когда нужно проверить например разные модификаторы или разный проброс параметров в одинаковых блоках – можно в одном файле сделать несколько шаблонов
/cc @Coltspb

alexbaumgertner · 2015-02-03T14:16:54Z

@j0tunn @eGavr @arikon очень стоит, для нас это Блокер

j0tunn · 2015-02-03T14:18:22Z

@alexbaumgertner почему это блокер? Тесты подправить - да и все, тем более, что у тебя там просто нужно свойства местами поменять

eGavr · 2015-02-03T14:22:06Z

Я вижу смысл в том, чтобы сейчас править тесты руками, а более умная проверка conditional comments появится вскоре.

sipayRT · 2015-02-03T14:27:55Z

чтобы не плодить файлы тестов, когда нужно проверить например разные модификаторы или разный проброс параметров в одинаковых блоках – можно в одном файле сделать несколько шаблонов

зачем вообще хотеть такого? Давайте тогда уже сразу все в один файл писать :) Есть один независимый тест, не нужно смешивать.

eGavr · 2015-02-03T14:42:03Z

Итак!

решили, что СС будут проверяться как строка, а внутреннее содержание - как тэги (с игнорированием последовательности атрибутов и т.д.)

Руководствуюсь этим алгоритмом. Все "ЗА"?

alexbaumgertner · 2015-02-03T14:43:56Z

@j0tunn поправил, все равно не проходит:

ПС: убрал еще пробелов и стало проходить

eGavr · 2015-02-03T14:52:25Z

Дело в том, что эти комментарии на данном этапе сравниваются как обычный текст, поэтому пробелы и переносы строк будут учитваться вне зависимости от опции ignoreWhitespaces, так и было задумано по алгоритму, ведь по логике мы и должны так делать.

Кейс с CC я учту ASAP. Уж помучайтесь чуть-чуть и поудаляйте пробелы ;) Поправлю, и все будет как вам надо в скором времени.

Игнорировать комменты мы не можем, так как @sipayRT привел реальный пример баги.

sipayRT · 2015-02-03T14:52:57Z

Руководствуюсь этим алгоритмом. Все "ЗА"?

я ЗА

alexbaumgertner · 2015-02-03T14:53:37Z

@eGavr ок :)
@eGavr @j0tunn @sipayRT спасибо за разъяснения и коментарии, поправлю в библиотеках.

sipayRT · 2015-02-03T15:03:06Z

persidskiy · 2015-02-03T17:49:07Z

Вы главное скажите, ручные комментарии можно будет писать в эталонных шаблонах?

eGavr · 2015-02-03T17:51:05Z

Нет.

persidskiy · 2015-02-03T18:03:06Z

Нет, так не пойдет, давай сделаем сразу хорошо.

Предвещая вопрос зачем мне комментарии и что я этого не должен хотеть, отвечу сразу. Я могу хотеть держать эталонные файлы в опрятном состоянии. На это есть куча причин из жизни и если вы готовы потрать на это наше время, я могу в этом убедить)

eGavr · 2015-02-03T18:39:48Z

Вроде бы как решили выше в переписках, что ничего лишнего быть не должно

sipayRT · 2015-02-04T07:31:39Z

Я могу хотеть держать эталонные файлы в опрятном состоянии.

имхо, эталоны без комментариев - это и есть опрятность. Эталон - это то, что должно сгенериться в результате работы шаблонизатора. В итоговый результат не нужно записывать что-то еще. А название эталона должно говорить что там тестируется.

eGavr · 2015-02-05T14:55:06Z

Давайте определяться...

@sipayRT всех убедил, что игнорировать комментарии мы не будем, и тесты должны быть без лишних комментариев?
Conditional comments будут сравниваться по общему принципу (то есть не будем учитывать порядок следования атрибутов в тэгах)

sipayRT · 2015-02-05T15:18:31Z

<head>
    <meta http-equiv="X-UA-Compatible" content="IE=edge">
    <!--[if lt IE 9]>
       <script src="//yastatic.net/es5-shims/0.0.1/es5-shims.min.js"></script>
    <![endif]-->
    <meta charset="utf-8">
    <title>bem-components: select</title>
    <!--[if gt IE 8]><!-->
        <link rel="stylesheet" href="_simple.css">
    <!--<![endif]-->
    <!--[if IE 8]>
        <link rel="stylesheet" href="_simple.ie8.css"/>
    <![endif]-->
    <script type="text/javascript" charset="utf-8" src="//yastatic.net/jquery/2.1.3/jquery.min.js"></script>
</head>

persidskiy · 2015-02-05T15:39:56Z

@eGavr Нет, меня он не убедил.

Очень часто возникает необходимость объедить тесты, которые про одно и то же в один тестовый файл. Например, у меня есть блок dropdown, он работает с двумя блоками button и link, для каждого из случаев у меня написаны разные шаблоны. При этом в этих шаблонах есть разные условия про разные способы передачи параметров и разные модификаторы. В случае dropdown в Лего, там по 4 явно выраженных кейса с каждым свитчером.
Я как порядочный разработчик хочу добиться хорошего покрытия и проверить все возможные варианты передачи параметров.
Скажите, я могу хотеть сделать по файлу с тестами на каждый свитчер, чтобы я на одном экране мог сразу видеть, какие кейсы я покрыл? При этом я бы очень хотел их аккуратно отформатировать и подписать комментариями. Кроме этого, на этапе разработки тестов очень часто надо редактировать получившийся эталонный html. Для этого можно легко использовать мультивыделение в редакторе. В случае с несколькими файлами нужно их каждый раз открывать и ты начинаешь в них путаться. Тут тонкий момент. Когда у тебя есть 2 файла(один с bemjson, другой эталонный) ты в голове можешь держать мысль "мне надо во втором тесте исправить то-то"; переключаешься в другой файл и работаешь с ним. Если бы в моем случае было 8 файлов вместо 2, было бы сложно между ними переключаться. А еще бывает так, что на другом уровне переопределения доопределены шаблоны и для этого уровня нужно готовить отдельный файл эталонов. Таким образом, мы имеем 3 файла против 12. Дальше комбинаторика работает в полную силу. И таких блоков достаточно много.

А еще бывает нужно сгенерировать тест-кейсы динамически по какому-то заданному упрощенному массиву параметров, чтобы точно проверить все аспекты поведения. Как в таком случае быть? Тоже выносить тесты по разным файлам?

Так что если вы действительно хотите сделать ваш инструмент удобным потребителям, то давайте работать в этом направлении. Еще раз хочу напомннить, что html-differ в моем понимании, это инструмент, который позволяет сравнивать значимые части html. И conditional comment здесь значимый элемент. Он отвечает за загрузку разных стилей и влияет на итоговый результат работы html в браузере, а обычные комментарии - не значимые части, они нужны только для удобства разработчиков.

eGavr · 2015-02-05T15:46:10Z

С точки зрения реализации, мне не принципиально как делать ;) В итоге все будет сводиться к тому, какие опции заюзать. Опцию ignoreComments я расширю и она будет такой:

ignoreComments: {
    normalComments: true || false,
    conditionalComments: true || false
}

а вот какие значения указать этой опции в БЭМ-пресете уже договариваться вам.

persidskiy · 2015-02-05T15:48:48Z

Так в чем тогда проблема? Я же и просил - дайте возможность писать простые комментарии в эталонных шаблонах. А вы пытаетесь убедить меня, что я не так тесты пишу)

eGavr · 2015-02-05T15:57:51Z

Я пишу html-differ)
А не тесты на шаблоны ;) и спор возник у тебя с человеком, который их тоже пишет и что-то в этом понимает.

arikon · 2015-02-05T15:59:00Z

@eGavr Ок, Женя, запили эту фичу. Подробнее отвечу в Стартреке.

sipayRT · 2015-02-05T16:11:40Z

@Coltspb я все равно не понимаю смысла в комментариях. Эталон - это эталон, а не кусок кода, который можно комментировать. Насчет кучи тестов в одном файле - ты пишешь много проверок в одном it, когда пишешь тесты на js? Если да, то как ты потом понимаешь что именно не так, когда этот тест падает? Такая же (ну почти такая же) ситуация и тут - ты хочешь в одном тесте проверить кучу кейсов. Но если Жека расширяет ignoreComments, то все это уже не важно :)

eGavr · 2015-02-05T16:20:01Z

Важно, чтобы вы решили, какой БЭМ-пресет будет!
Расширить - я расширю, НО все это крутится в enb-bem-tmpl-specs, и эта тулза использует html-differ с определенными опциями, и надо определиться всем, как именно конфигурировать проверку комментов. Либо проверяем обычные комменты, либо нет.

blond · 2015-02-05T16:21:10Z

Важно, чтобы вы решили, какой БЭМ-пресет будет!

Понятно же, что такой:

ignoreComments: {
    normalComments: true,
    conditionalComments: false
}

eGavr · 2015-02-05T16:22:21Z

@sipayRT не видит смысла игнорировать обычные комментарии, так как они не могут туда попасть, если их не писать дополнительно руками.

blond · 2015-02-05T16:24:34Z

@sipayRT не видит смысла игнорировать обычные комментарии, так как они не могут туда попасть, если их не писать дополнительно руками.

Инструменты не должны никого ограничивать без реальной на то необходимости. Поэтому пресет надо подбирать таким, чтобы используя его были довольны все, и те кто хочет писать коментарии, и те, кто не хочет писать коментарии.

eGavr · 2015-02-05T16:56:30Z

@andrewblond , может сделать так, чтобы conditional comment проверялись всегда, а конфигурировать только проверку обычный комментов?

А смысл НЕ проверять conditional comments? они ломают работоспособность, если отключить проверку и т д.

sipayRT · 2015-02-06T07:02:49Z

@sipayRT не видит смысла игнорировать обычные комментарии, так как они не могут туда попасть, если их не писать дополнительно руками.

@eGavr да, смысла я не вижу. Но если можно расширить инструмент так, чтобы все остались довольны, то так и делай ;)

blond · 2015-02-06T07:06:39Z

может сделать так, чтобы conditional comment проверялись всегда, а конфигурировать только проверку обычный комментов?

По мне идея годная, я тоже не вижу смысла не проверять conditional comments.

eGavr · 2015-02-06T07:28:10Z

Хорошо! Тогда решено, что опция в структуре не меняется и остается как и была, только она не зависимо от ее значения всегда учитывает conditional comments и их содержимое проверяет как обычный HTML.

Для БЭМ-пресета эта опция будет принимать значение true, то есть в итоге обычные комментарии всегда игнорируются (по сильной просьбе @Coltspb )

sipayRT · 2015-02-06T07:29:30Z

может сделать так, чтобы conditional comment проверялись всегда, а конфигурировать только проверку обычный комментов?

Да, мне тоже ок

eGavr added the enhancement label Dec 30, 2014

eGavr mentioned this issue Dec 30, 2014

Issue 114 #115

Merged

eGavr mentioned this issue Feb 6, 2015

Fix the comparison of conditional comments #117

Merged

eGavr closed this as completed in #117 Feb 10, 2015

Update 'ignoreComments' option #116

Update 'ignoreComments' option #116

Comments

eGavr commented Dec 30, 2014

arikon commented Feb 3, 2015

eGavr commented Feb 3, 2015

alexbaumgertner commented Feb 3, 2015

alexbaumgertner commented Feb 3, 2015

j0tunn commented Feb 3, 2015

eGavr commented Feb 3, 2015

sipayRT commented Feb 3, 2015

eGavr commented Feb 3, 2015

alexbaumgertner commented Feb 3, 2015

eGavr commented Feb 3, 2015

sipayRT commented Feb 3, 2015

alexbaumgertner commented Feb 3, 2015

sipayRT commented Feb 3, 2015

persidskiy commented Feb 3, 2015

eGavr commented Feb 3, 2015

persidskiy commented Feb 3, 2015

eGavr commented Feb 3, 2015

sipayRT commented Feb 4, 2015

eGavr commented Feb 5, 2015

sipayRT commented Feb 5, 2015

persidskiy commented Feb 5, 2015

eGavr commented Feb 5, 2015

persidskiy commented Feb 5, 2015

eGavr commented Feb 5, 2015

arikon commented Feb 5, 2015

sipayRT commented Feb 5, 2015

eGavr commented Feb 5, 2015

blond commented Feb 5, 2015

eGavr commented Feb 5, 2015

blond commented Feb 5, 2015

eGavr commented Feb 5, 2015

sipayRT commented Feb 6, 2015

blond commented Feb 6, 2015

eGavr commented Feb 6, 2015

sipayRT commented Feb 6, 2015