Pull to refresh

Comments 31

Костыльное решение. А не проще сделать день рефакторинга и переформатировать весь проект с помощью clang-format. А затем прикрутить clang-format к IDE. И дальше жить спокойно)
Как раз вашего варианта мы и хотели избежать. Потому что дождаться такого решения от PM-ов и всевозможных Lead-ов нереально. Дополнительный коммит, который только форматирует код, тем более глобально, затрудняет чтение истории изменений.
UFO landed and left these words here
Если проекту лет так дцать, то ситуация «писали вразнобой» более чем вероятна. Дело не в том что никто не соблюдал, а скорее что какие-то древние фрагменты писались просто до возникновения кодестайла.
Конечно, сode style на проекте был, но исходно его делали вручную, т.к. некоторые файлы нельзя было форматировать целиком. Это отнимало время и порождало ненужные дискуссии. Те, кто делал review коммитов, писали замечания по каждому пропущенному пробелу или переносу строки, заворачивали merge request-ы на доработку.

Разработчиков не радовало, что из-за таких пустяков приходилось возвращаться к задаче. Тем более что несвоевременно влитый merge request это ещё и merge с новыми изменениями, сделанными другими.

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

Сейчас разработчики просто коммитят свой код, не заботясь о том, правильно ли там расставлены пробелы с переносами. Проблемы возникают только при ошибках форматера, но их гораздо проще обойти при коммите, чем потом ловить замечания от Lead-ов. :-)

Пытался использовать clang-format но он не работал с переносами строчек нормально. Была у меня из Qt функция connect принимает 4 аргумента: sender, signal, receiver, slot. Все четыре не помещались в строчку, тогда я сделал так


connect(sender, signal,
        receiver, slot);

Запустил шланг и он мне выдал такое уродство:


connect(sender, signal, receiver,
        slot);

Не вижу принципиальной разницы между этими двумя уродствами. Если параметры не умещаются в одну строку или если они не тривиальные, то лучше каждому параметру выделить отдельную:


connect(
    sender, 
    signal, 
    receiver,
    slot,
)

после каждой запятой делать перенос, не всегда удобно. В другом примере у меня были сгруппированы по логике елементы массива в несколько строчек, а он из них сделал матрицу 2x4. Я так и не нашёл как это настраивается.

// clang-format off
... My beautiful something ...
// clang-format on

Касательно переносов и прочего форматирования — день поработать с автоформатированием, включённым при сохранении файла ( Qt Creator так умеет) — вообще перестаёшь задумываться над форматированием.
Я пишу жуткую фигню как придётся, потом жму сохранить и раз — красивый код, сколько там трудов ушло на расставление переносов — знать не знаю.
Мне это на столько удобно, что трюк выше я пока использовал всего один раз когда цланг портил колонки данных в константе, при этом в случае массива объектов он неплохо справляется с форматированием.
Любой. Вначале глаз сильно цепляется за отдельные некрасивости, но быстро приходит понимание, что лучше иметь код, который единообразен, пусть и чуток некрасив кое-где, чем код, который местами — хотя на выставку произведений искусства посылай, а местами — коряв просто потому, что вы неудачно пробелы поставили.

Да, clang-format никогда не научится понимать, что какие-то параметры тесно связаны между собой — ну и фиг с ним. Лучше выставить ширину строки где-нибудь на 100-120 символов и всё: случаев, когда вызов функции с четырьмя аргументами перестанет помещаться в строку, станет меньше — и их уже можно будет терпеть переформатированными в столбик.
Можно увидеть ваш коннект в оригинале, без сопутствующего кода? Плюс конфиг .clang-format.

Например:


  connect(this, &ColorListWindow::colorChanged,
          mapper, &QDataWidgetMapper::submit);

Cтиль пробовал просто от Google.

У меня получился вот такой код.
int main() {
  connect(this, &ColorListWindow::colorChanged, mapper,
          &QDataWidgetMapper::submit);
}

Это потому что в конфиге от гугла есть: ColumnLimit: 80
Не вижу ничего ужасного тут, всё вполне логично. Длинные строки кода надо переносить ибо это нечитаемо.
Если у вас другое мнение на этот счёт, изучайте опции конфига clang-format. Насколько помню, там можно разрешить ручной перенос аргументов функции на разные строки.

Я с самого начала о том и писал что он поменял расположение переноса.

У меня та же ерунда: взял clang-format файл от Qt — вроде всё хорошо, но лямбда функции форматирует ужасно и connect, если в строку не помещается разбивает на 4 строки, хотя можно было бы так:
  connect(this, &ColorListWindow::colorChanged, 
          mapper, &QDataWidgetMapper::submit);

Хотя файл официальный, но после форматирования code review пройти не возможно ;) Пришлось его при сохранении отключить, где настроить данные параметры тоже не нашёл, хотя просмотрел всю документацию по clang-format.
Это какую другую?! В системе используется та же, что и в QtC — 7.0.1 кажется. Попробовать как-то установить 9-ю? Там много поменялось?
Попытка не пытка. Может будет и лучше. У меня в Debian, например доступна ещё и 6-ая наряду с 7-ой. Не обязательно же только повышать версию.
Возможно, вам надо форматировать diff кода перед тем как коммитить его
git diff -U0 --no-color | clang-format-diff-6.0 -i -p1
если под Linux.
Или git-clang-format уже после коммита. Далее смотрите изменения форматера и git commit --amend --no-edit (Хотя сам не пробовал так)
У меня если запускается формат, то только при сохранении — хочется всё-таки сразу видеть результат.
Хотя файл официальный, но после форматирования code review пройти не возможно ;)
А вы пробовали? Обычно если в проекте начинают использовать clang-format, то претензии к расстановке пробелов больше в code review не рассматриваются: как clang-format отформатировал — так и будет.

Иначе в нём и смысла-то никакого нет: зачем нужен clang-format, если он время не экономит?
Смешной вопрос. Конечно пробовал, я же написал выше, что пришлось отключить. Пробелы он отлично расставляет, а вот отступы и переносы не всегда так как хочется. Команда в Qt большая — одни советуют clang-format использовать, а другие — не принимают код отформатированный этим форматом при code review ;)

Имхо опасно делать такие pre-commit hook.


Во-первых, вы должны абсолютно доверять clang-format и молчаливо считать, что его переформатирование абсолютно всегда правильно (готовы это гарантировать? Вообще для любого кода?) Во-вторых, бывают случаи, когда нарушение стиля кода — вынужденная мера. В вашем же случае автор коммита даже не узнает про то, что его код был переформатирован.


Более корректное решение: Hook должен отклонять неподходящий коммит. После чего происходит 1 из 2 вещей: а) автор переформатирует код clang-овским или любым другим тулом и отправляет повторно (он также может заранее попытаться реализовать автоформатирование на своей стороне): б) автор доказывает необходимость ошибки форматирования, и коммит разрешается в административном порядке.

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

Неправда. Там выводиться либо «Nothing to be formatted» либо полный diff форматера. Исключение может быть только в GUI утилитах, которые не показывают вывод git commit.

Более корректное решение: Hook должен отклонять неподходящий коммит.

На практике, в pipeline -ах сервера где делают code review(gitlab например), надо показывать что для .clang-format проекта есть diff в рассматриваемом коммите. Тогда, тот кто делает review решает что дальше.
Тут нет большой разницы что использовать. Т.к. обе утилиты не могут переписать сделанный коммит или изменения в индексе. Они всего лишь берут один diff со входа и выдают другой на выходе. А вы уже сами должны встраивать применение этого diff в коде хука.
Раз уж тут собрались знатоки clang-format, то посмею задать вопрос не совсем в тему статьи. Есть ли какой-нибудь способ объяснить ему несколько допустимых вариантов форматирования? Самый простой пример — разрешить делать однострочные функции, но если в коде уже есть перенос, то не трогать его? Это вкусовщина, но уже привык делать однострочными только конструкции вида return smth; и никогда не заталкивать никакую логику. Кстати, встречал и полностью обратное — однострочные if разрешены, но return всегда должен быть перенесён, чтобы проще было искать точки выхода. Ну или есть ли другие форматилки, которые оставляют свободу выбора?
Есть ли какой-нибудь способ объяснить ему несколько допустимых вариантов форматирования?
Это опции конфига начинающиеся с «Allow». В вашем случае AllowAllParametersOfDeclarationOnNextLine
clang.llvm.org/docs/ClangFormatStyleOptions.html
Хотя иногда, как в примере выше с technic93 это может не работать.
Вообще, составление конфига для ClangFormat это бег по граблям и поиск компромиссов. Надо время и терпение.

Из ответа на stack-overflow следует что clang-format сначало всё парсит в AST а потом собирает назад согласно указанным правилам, т.е. информация о начальном форматировании теряется полностью.

У шланг-формата очень плохо с поддержкой. В минорном апдейте несколько раз ломали дефолтное поведение. Пытались решить это прибив версию, но, как оказалось (создатели это подтвердили) что нет никакой гарантии что билды под макось и линукс одной версии работают одинаково, а по факту всегда есть различия. Штука очень нужная и полезная, но с переносимостью нужно что-то делать, например завернуть в докер и вызывать по хоткею из ide или по при сохранении файла. Прекомит хук нам показался не очень удобным и опасным
Sign up to leave a comment.

Articles