Experimental automated reformatting #1556

unxed · 2023-03-15T14:34:36Z

using astyle -T -S -p -xg -H -U -xU with minor manual fixups. @elfmz please review!

…r manual fixups

unxed · 2023-03-15T15:17:58Z

A little explanation:
-T means "force tabs indentation"
-S means "indent inner switch headers" ('case XXX', they are don't indented by astyle by default)
-p enables space padding around operators
-xg enables space padding after comas
-H enables space padding after if, for, etc
-U removes unnecessary space padding around parenthesis
-xU enables indenting instead of space alignment for multi line assignments or parameters lists

…rmatting tools

…ng tools

elfmz · 2023-03-15T17:20:57Z

is it possible to keep original left code/right comments in cases where it was, like:

?

unxed · 2023-03-15T17:52:06Z

Unfortunately, it looks like astyle has not options to control comments formatting this way. The only way is to disable auto formatting such blocks using

// *INDENT-OFF*
...
// *INDENT-ON*

Should I use it?

Another option is to use semicolons, but it looks like an ugly hack:

    BYTE  HeadId;                // special archive marker = 0x1A or 0xFE
    BYTE  Type;                  // Compression method or
    ;                            //  type of record:  0  End of file,
    ;                            //                   1  Remark,
    ;                            //                   2  Path,
    ;                            //                   3  Security envelope,
    ;                            //                   4  Error correction codes (not implemented in PAK 2.xx)

unxed · 2023-03-15T18:19:20Z

Also astyle looks to be discontinued. Asked on stackoverflow if clang-format can avoid mixing tabs and spaces anyhow:
https://stackoverflow.com/questions/75748624/how-to-make-clang-format-not-to-align-anything-with-spaces-at-all

elfmz · 2023-03-15T20:47:40Z

I used some googled example, modified UseTab and TabWidth and seems its doing things right now:
clang-format.zip

elfmz · 2023-03-15T20:49:08Z

what still missing is that i personally like increments written in such bit weird indentation:

i = 10;
i+= 10;
i*= 2;

elfmz · 2023-03-15T21:50:18Z

some adjustements: clang-format.zip

unxed · 2023-03-15T22:26:09Z

Still produces mixture of tabs and spaces at the beginning of some lines:

Edit::Edit(ScreenObject *pOwner, Callback *aCallback, bool bAllocateData)
   : m_next(nullptr),
	 m_prev(nullptr),

		SelEnd = StrSize;      // а также считаем что все выделено -
							   // надо же отличаться от обычных Edit

				FS << fmt::Cells() << fmt::Skip(CellSelStart)
				   << fmt::Truncate(CellSelEnd - CellSelStart) << OutStr.data();

			while (CurPos > 0 && ! (! IsWordDiv(WordDiv(), Str[CurPos]) &&
											 IsWordDiv(WordDiv(), Str[CurPos - 1]) && ! IsSpace(Str[CurPos]))) {

unxed · 2023-03-15T22:31:43Z

Also things like

	if (Flags.Check(FEDITLINE_CLEARFLAG) && ((Key <= 0xFFFF && Key != KEY_BS) || Key == KEY_CTRLBRACKET ||
														Key == KEY_CTRLBACKBRACKET || Key == KEY_CTRLSHIFTBRACKET ||
														Key == KEY_CTRLSHIFTBACKBRACKET || Key == KEY_SHIFTENTER || Key == KEY_SHIFTNUMENTER)) {

or

	if (
				(
							(
										(Key == KEY_BS || Key == KEY_DEL || Key == KEY_NUMDEL) &&
										Flags.Check(FEDITLINE_DELREMOVESBLOCKS)) ||
							Key == KEY_CTRLD) &&
				! Flags.Check(FEDITLINE_EDITORMODE) &&
				SelStart != -1 &&
				SelStart < SelEnd) {

look a bit weird. Why so much indents?

elfmz · 2023-03-15T22:40:11Z

Looks as it adjusted 'inner' condition to its opening brace, i would do this like:

if (Flags.Check(FEDITLINE_CLEARFLAG) && 
	((Key <= 0xFFFF && Key != KEY_BS) || Key == KEY_CTRLBRACKET ||
		Key == KEY_CTRLBACKBRACKET || Key == KEY_CTRLSHIFTBRACKET ||
		Key == KEY_CTRLSHIFTBACKBRACKET || Key == KEY_SHIFTENTER || Key == KEY_SHIFTNUMENTER)) {

unxed · 2023-03-15T22:43:26Z

Do you mean manual fixing such cases after clang-format?

elfmz · 2023-03-16T08:08:45Z

Just expressed how such code should look.. ideally

elfmz · 2023-03-16T09:41:49Z

again corrected config:
clang-format.zip

unxed · 2023-03-16T10:27:33Z

Still mixture of tabs and spaces at the beginning of lines, but I guess that can not be totally avoided with clang-format so manual fixing will be required anyway:

Edit::Edit(ScreenObject *pOwner, Callback *aCallback, bool bAllocateData)
	: m_next(nullptr), m_prev(nullptr),
	  Str(bAllocateData ? reinterpret_cast<wchar_t *>(malloc(sizeof(wchar_t))) : nullptr),
	  StrSize(0), MaxLength(-1), Mask(nullptr), LeftPos(0), CurPos(0), PrevCurPos(0),
	  MSelStart(-1), SelStart(-1), SelEnd(0), CursorSize(-1), CursorPos(0)
{

		Flags.Clear(FEDITLINE_CLEARFLAG);      // при дроп-даун нам не нужно никакого
											   // unchanged text

			for (int j = 0, S = TabSize - ((LeftPos + OutStrCells) % TabSize);
				 j < S && int(OutStrCells) < EditLength; ++j, ++OutStrCells) {

			FS << fmt::LeftAlign() << fmt::Cells() << fmt::Size(EditLength)
			   << OutStr.data();

	while (TrimmedStrSize > 0 &&
		   (IsSpace(Str[TrimmedStrSize - 1]) || IsEol(Str[TrimmedStrSize - 1]))) {
		--TrimmedStrSize;
	}

	if ((((Key == KEY_BS || Key == KEY_DEL || Key == KEY_NUMDEL) &&
			 Flags.Check(FEDITLINE_DELREMOVESBLOCKS)) ||
			Key == KEY_CTRLD) &&
		! Flags.Check(FEDITLINE_EDITORMODE) && SelStart != -1 && SelStart < SelEnd) {

etc

Also maybe opening curly bracket should be at the new line to avoid situations like this there conditions and the inner block are indented the same way:

					if ((MenuKey >= int(L' ') && MenuKey <= MAX_VKEY_CODE) ||
						MenuKey == KEY_BS || MenuKey == KEY_DEL ||
						MenuKey == KEY_NUMDEL) {
						FARString strPrev;

elfmz · 2023-03-16T10:46:41Z

clang-format.zip

unxed · 2023-03-16T10:50:54Z

Still mixture of tabs and spaces and conditions are indented the same as inner block:

	if (Key != KEY_NONE && Key != KEY_IDLE && Key != KEY_SHIFTINS &&
		Key != KEY_SHIFTNUMPAD0 && Key != KEY_CTRLINS &&
		((unsigned int) Key < KEY_F1 || (unsigned int) Key > KEY_F12) && Key != KEY_ALT &&
		Key != KEY_SHIFT && Key != KEY_CTRL && Key != KEY_RALT && Key != KEY_RCTRL &&
		(Key < KEY_ALT_BASE || Key > KEY_ALT_BASE + 0xFFFF) &&      // ???? 256 ???
		! (((unsigned int) Key >= KEY_MACRO_BASE &&
			   (unsigned int) Key <= KEY_MACRO_ENDBASE) ||
			((unsigned int) Key >= KEY_OP_BASE &&
				(unsigned int) Key <= KEY_OP_ENDBASE)) &&
		Key != KEY_CTRLQ) {
		Flags.Clear(FEDITLINE_CLEARFLAG);
		Show();
	}

Looks like astyle, despite being abandoned, still does the job better.

elfmz · 2023-03-16T10:51:49Z

However i more like

Ctor::Ctor()
    :
    foo(bar)

but this also acceptable, if there is no way to do so..:

Ctor::Ctor()  :
    foo(bar)

unxed · 2023-03-16T11:00:30Z

However i more like

My astyle config does not change this. That was manual formatting. So changed to style you prefer by hands. Also reverted right side comments style change. There are only three such cases throughout the source tree, so can be easily fixed by hands after processing by astyle. Anyway any automatic formatting tool need manual fixes after it.

unxed · 2023-03-16T18:35:27Z

There are one more actively developed formatting tool:
https://github.com/uncrustify/uncrustify

Looks extremely configurable. Now experimenting with it.

unxed · 2023-03-16T20:33:13Z

Unfortunately uncrustify does not have an option similar to astyle -xU. Asked for it:
uncrustify/uncrustify#3961

elfmz · 2023-03-16T21:16:58Z

if set in clang's config ColumnLimit: 110 then it looks better in such complex cases .. like that if (Key != KEY_NONE && Key != KEY_IDLE

elfmz · 2023-03-17T13:23:13Z

some tuning through 'penalties': clang-format.zip
thinking may be to patch clang-format to implement desired a+= b vs a = b indentation..

unxed · 2023-03-17T13:35:39Z

Still conditions and inner block indented equally:

	if (Key != KEY_NONE && Key != KEY_IDLE && Key != KEY_SHIFTINS && Key != KEY_SHIFTNUMPAD0 &&
		Key != KEY_CTRLINS && ((unsigned int) Key < KEY_F1 || (unsigned int) Key > KEY_F12) &&
		Key != KEY_ALT && Key != KEY_SHIFT && Key != KEY_CTRL && Key != KEY_RALT && Key != KEY_RCTRL &&
		(Key < KEY_ALT_BASE || Key > KEY_ALT_BASE + 0xFFFF) &&      // ???? 256 ???
		! (((unsigned int) Key >= KEY_MACRO_BASE && (unsigned int) Key <= KEY_MACRO_ENDBASE) ||
			((unsigned int) Key >= KEY_OP_BASE && (unsigned int) Key <= KEY_OP_ENDBASE)) &&
		Key != KEY_CTRLQ) {
		Flags.Clear(FEDITLINE_CLEARFLAG);
		Show();
	}

Also mixture of tabs and spaces:

		SelEnd = StrSize;      // а также считаем что все выделено -
							   // надо же отличаться от обычных Edit

			for (int j = 0, S = TabSize - ((LeftPos + OutStrCells) % TabSize);
				 j < S && int(OutStrCells) < EditLength; ++j, ++OutStrCells) {

Maybe also ask clang-format devs to implement something like astyle -xU does?
UPD: did it: llvm/llvm-project#61474

elfmz · 2023-03-17T19:40:15Z

bit tweaked clang-format to make assignment spaces behave as descibed and modified config to make continuation indentation to be 2 spaces instead of tab.. Looks not bad?
clang-format-edit.zip

unxed · 2023-04-02T23:47:48Z

как же долго он собирается :)

elfmz · 2023-04-02T23:52:26Z

с make -j16 норм

unxed · 2023-04-03T00:08:27Z

		Flags.Clear(FEDITLINE_CLEARFLAG);		// при дроп-даун нам не нужно никакого unchanged text
		SelStart = 0;
		SelEnd = StrSize;						// а также считаем что все выделено -
												// надо же отличаться от обычных Edit

Воу!

		if (!Flags.Check(FEDITLINE_PERSISTENTBLOCKS) && !(Key == KEY_CTRLINS || Key == KEY_CTRLNUMPAD0)
				&& !(Key == KEY_SHIFTDEL || Key == KEY_SHIFTNUMDEL || Key == KEY_SHIFTDECIMAL) && !Flags.Check(FEDITLINE_EDITORMODE)
				&& Key != KEY_CTRLQ && !(Key == KEY_SHIFTINS || Key == KEY_SHIFTNUMPAD0))		// Key != KEY_SHIFTINS) //??

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

			Attr = MAKELONG(MAKEWORD(FarColorToReal(DialogMode.Check(DMODE_WARNINGSTYLE)
													? (DisabledItem ? COL_WARNDIALOGDISABLED : COL_WARNDIALOGBOXTITLE)
													: (DisabledItem ? COL_DIALOGDISABLED
																	: COL_DIALOGBOXTITLE)),		// Title LOBYTE
									FarColorToReal(DialogMode.Check(DMODE_WARNINGSTYLE)
													? (DisabledItem ? COL_WARNDIALOGDISABLED
																	: COL_WARNDIALOGHIGHLIGHTBOXTITLE)
													: (DisabledItem ? COL_DIALOGDISABLED
																	: COL_DIALOGHIGHLIGHTBOXTITLE))),		// HiText HIBYTE
					MAKEWORD(FarColorToReal(Attr),															// Box LOBYTE
							0)																				// HIBYTE
			);

ух ты! красивое!

							MAKEWORD(		// HIWORD
											//  HILO (Unchanged)
									FarColorToReal(DisabledItem ? COL_WARNDIALOGEDITDISABLED
																: COL_DIALOGEDITUNCHANGED),		//???

вот тут по идее второй камент должен на два таба левее быть

					GotoXY(X1
									+ ((CurItem->Flags & DIF_SEPARATORUSER)
													? X
													: (!DialogMode.Check(DMODE_SMALLDIALOG) ? 3 : 0)),
							Y1 + Y);		//????

а вот здесь-то он зачем после X1 перенос строки сделал?

					const wchar_t Check[] = {L'[',
							(CurItem->Selected ? (((CurItem->Flags & DIF_3STATE) && CurItem->Selected == 2)
												? *Msg::CheckBox2State
												: L'x')
												: L' '),
							L']', L'\0'};

вот тут не очень семантично получилось, по-моему. оригинал читабельнее.

		/*****************************************************************/
		// получить позицию и размеры контрола
		case DM_GETITEMPOSITION:	// Param1=ID, Param2=*SMALL_RECT

			if (Param2) {
				SMALL_RECT Rect;
				if (Dlg->GetItemRect(Param1, Rect)) {
					*reinterpret_cast<PSMALL_RECT>(Param2) = Rect;
					return TRUE;
				}
			}

			return FALSE;
			/*****************************************************************/

нижние звёздочки на таб вправо уехали

в целом - очень круто! каких-то прям адских ошибок не видно, скорее, шероховатости небольшие.

elfmz · 2023-04-03T00:13:40Z

лишние табы подфиксил тока что но не все..

elfmz · 2023-04-03T08:12:06Z

Кстати то что ещё длинные каменты иногда вылезают за экран это я спецфичу сделал сделал - опция AllowLongTrailingComments. Выбрал из двух зол меньшее, имхо это лучше чем вот такое:

enum DLGEDITLINEFLAGS
{
	DLGEDITLINE_CLEARSELONKILLFOCUS = 0x00000001,		// управляет выделением блока при потере фокуса ввода
	DLGEDITLINE_SELALLGOTFOCUS = 0x00000002,	// управляет выделением блока при получении фокуса ввода
	DLGEDITLINE_NOTSELONGOTFOCUS =
			0x00000004,		// не восстанавливать выделение строки редактирования при получении фокуса ввода
	DLGEDITLINE_NEWSELONGOTFOCUS = 0x00000008,		// управляет процессом выделения блока при получении фокуса
	XXXXXXXXXXXXXXXXXXXXXXXXXXX =
			0x00000010,		// при получении фокуса ввода переместить курсор в конец строки
	DLGEDITLINE_GOTOEOLGOTFOCUS =
			0x00000010,		// при получении фокуса ввода переместить курсор в конец строки

};

все же ограничение на ширину экрана это больше для читабельности кода в окне диффа, коменты тут менее важную роль играют, их можно вовсе не читать)

elfmz · 2023-04-03T08:42:27Z

а вот здесь-то он зачем после X1 перенос строки сделал?

Логика форматтера там запутанная но скорее всего однопроходная, то есть если выражение длинное - он переносит токены на новую строку один за одним пока очередной остаток не начнет влазить в экран. То что иногда после этого эффективнее смержить уже перенесенное - этого он видимо не умеет.

elfmz · 2023-04-03T08:57:14Z

вот тут по идее второй камент должен на два таба левее быть

Просто коменты расположенные на смежных строках выравниваются относительно друг друга по максимальной позиции

unxed · 2023-04-03T16:28:14Z

вот тут по идее второй камент должен на два таба левее быть

это, похоже, поправлено. остальное, о чём я писал, пока на месте. впрочем, оно не то чтобы такое уж критичное.

unxed · 2023-04-04T11:16:44Z

so, do you consider... тьфу опять запутался в языках тикетов))

в общем, можно ли считать clang-format достаточно допиленным, чтобы все легаси исходники фара через него прогонять?

elfmz · 2023-04-04T12:15:55Z

еще хочу сделать чтоб вместо
AlignConsecutiveAssignments: true
который применяет выравнивание ко всем присваиваниям, было нечто более тонконастраиваемое, типа
AlignConsecutiveAssignments: OnlyFields
чттобы выравнивались вещи типа

enum Foo
{
VAL1    = 1,
VALUE2  = 2
};

но не выравнивались присвоения в коде

void Foo()
{
int val1 = 1;
int value2  = 2;
};

elfmz · 2023-04-04T12:16:22Z

в целом думаю на след выходных можно будет реформатнкуть

elfmz · 2023-04-04T23:54:05Z

сделал раздельную настройку для мемберов:
clang-format.zip

unxed · 2023-04-05T00:54:21Z

прикольно! а почему он где-то табами выравнивает, а где-то пробелами?

	CVTITEM_TOPLUGIN		= 0,

	DLGIIF_LISTREACTIONFOCUS   = 0x00000001,		// MouseReaction для фокусного элемента

elfmz · 2023-04-06T23:00:15Z

добавил фичу чтоб в таких местах выравнивало пробелами: UseTab: ForContinuationAndIndentationAndComments

elfmz · 2023-04-06T23:07:14Z

clang-format.zip

unxed · 2023-04-06T23:14:47Z

Ага, починилось, ура! А ещё что-нибудь будет доделываться, или оставшиеся шероховатости уже не выглядят критическими?

elfmz · 2023-04-06T23:28:19Z

починить хотелось бы конечно но боюсь не осилю да и не так они критичны
автоформат надо применять наверное тока на старый код, так как в новом и так в принципе все норм, и правильных изменений меньше чем вот таких:
БЫЛО:

СТАЛО:

(такое тоже не осилю чинить)

unxed · 2023-04-06T23:40:09Z

Согласен про старый код, тоже экспериментировал с новым, пришёл к тем же выводам. Мне кажется, наилучшим подходом будет такой:

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

Дальше что-то совсем уж кривое будем сами исправлять руками понемногу по мере обнаружения. Главное, что наличие правил не позволит вносить новый «некрасивый» код, а старый станет нормально читабельным.

elfmz · 2023-04-07T08:10:50Z

еще хорошо бы попереименовывать переменные чтоб было както так:

class Foo
{
    int _private_field:
    void Bar()
    {
         int local_var;
    }
public:
   int public_field;
};

на это вроде clang-tidy способен, но это в другой раз, наверное

unxed · 2023-04-07T09:53:21Z

Это надо прописать как раз в правила оформления нового кода, потому что я вот не знал, например.

Как в итоге форматировать будем? С моей стороны требуется что-то?

elfmz · 2023-04-07T09:55:08Z

я переформатирую, я просто хочу перед комитом убедится что все точно норм, для чего сделаю дизасм оригинальной версии и новой и убедюсь/убежусь/убеждусь что единственные изменения в бинарниках - только номера строк в дебажных принтах (их там есть немного)

elfmz · 2023-04-08T15:14:17Z

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

unxed · 2023-04-08T15:36:09Z

Ага, я посмотрю по логу коммитов на характерные паттерны, и тоже поправлю если где попадется. Круто, спасибо! А доработки форматтера будем в апстрим предлагать? Обидно если потеряются

elfmz · 2023-04-08T16:07:00Z

можно попробовать, для начала стоит спросить не против ли они вообще таких доработок, потому что если предлагать в апстрим - там юниттесты делать нормальные, доки там какието в ихнем своем формате и т.п. Если они согласны можно заморочится)

unxed · 2023-04-26T00:51:50Z

а какой первый шаг для этого? доку какую-то написать на новые опции?

Experimental formatting using astyle -T -S -p -xg -H -U -xU with mino…

55ddf60

…r manual fixups

unxed added 2 commits March 15, 2023 16:26

Avoid using comments formatting style unfriendly to automated code fo…

7b89a6e

…rmatting tools

Avoid comments formatting style unfriendly to automated code formatti…

25ec151

…ng tools

Merge branch 'elfmz:master' into mixed-indentation-fixes

bacdaf1

unxed added 2 commits March 16, 2023 11:58

Reverted whose files to previous comments styling for now

f82e7f3

Reverted to prev comments styling for now

9ea5763

Merge branch 'elfmz:master' into mixed-indentation-fixes

e639b9d

Merge branch 'elfmz:master' into mixed-indentation-fixes

86f9d68

Merge branch 'elfmz:master' into mixed-indentation-fixes

cc88114

unxed closed this Apr 8, 2023

unxed deleted the mixed-indentation-fixes branch April 8, 2023 15:45

Experimental automated reformatting #1556

Experimental automated reformatting #1556

Conversation

unxed commented Mar 15, 2023

unxed commented Mar 15, 2023 • edited

elfmz commented Mar 15, 2023 • edited

unxed commented Mar 15, 2023

unxed commented Mar 15, 2023

elfmz commented Mar 15, 2023

elfmz commented Mar 15, 2023

elfmz commented Mar 15, 2023

unxed commented Mar 15, 2023 • edited

unxed commented Mar 15, 2023 • edited

elfmz commented Mar 15, 2023

unxed commented Mar 15, 2023

elfmz commented Mar 16, 2023

elfmz commented Mar 16, 2023

unxed commented Mar 16, 2023 • edited

elfmz commented Mar 16, 2023

unxed commented Mar 16, 2023

elfmz commented Mar 16, 2023

unxed commented Mar 16, 2023

unxed commented Mar 16, 2023

unxed commented Mar 16, 2023

elfmz commented Mar 16, 2023

elfmz commented Mar 17, 2023

unxed commented Mar 17, 2023 • edited

elfmz commented Mar 17, 2023

unxed commented Apr 2, 2023

elfmz commented Apr 2, 2023

unxed commented Apr 3, 2023 • edited

elfmz commented Apr 3, 2023

elfmz commented Apr 3, 2023

elfmz commented Apr 3, 2023

elfmz commented Apr 3, 2023

unxed commented Apr 3, 2023

unxed commented Apr 4, 2023

elfmz commented Apr 4, 2023 • edited

elfmz commented Apr 4, 2023

elfmz commented Apr 4, 2023

unxed commented Apr 5, 2023

elfmz commented Apr 6, 2023

elfmz commented Apr 6, 2023

unxed commented Apr 6, 2023

elfmz commented Apr 6, 2023

unxed commented Apr 6, 2023 • edited

elfmz commented Apr 7, 2023

unxed commented Apr 7, 2023

elfmz commented Apr 7, 2023

elfmz commented Apr 8, 2023

unxed commented Apr 8, 2023

elfmz commented Apr 8, 2023

unxed commented Apr 26, 2023

unxed commented Mar 15, 2023 •

edited

elfmz commented Mar 15, 2023 •

edited

unxed commented Mar 15, 2023 •

edited

unxed commented Mar 15, 2023 •

edited

unxed commented Mar 16, 2023 •

edited

unxed commented Mar 17, 2023 •

edited

unxed commented Apr 3, 2023 •

edited

elfmz commented Apr 4, 2023 •

edited

unxed commented Apr 6, 2023 •

edited