Re: Automatic code formatting

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Mon, Jul 11 2022, Phillip Wood wrote:

> Hi brian
>
> On 10/07/2022 22:50, brian m. carlson wrote:
>> Most projects written in languages like Rust or Go use an automatic
>> formatter.  In Go's case, the formatter is specifically stated to be a
>> fixed style that is nobody's favourite, but because there's an automatic
>> formatter, everybody just uses it.  Personally, I don't love our coding
>> style now (I'm a 4-space person in C), but I would love it a lot more if
>> I didn't have to think about it.  I am substantially less picky about
>> what the style is than that we have an automated tool to tidy our code,
>> and I'm okay with us producing the occasional slightly suboptimal style
>> for the improved efficiency we get.
>> [...]
>> I should note that we already have a .clang-format file, so we can
>> already use clang-format.  However, we cannot blindly apply it because
>> it produces output that is not always conformant with our style.  My
>> proposal here is to define our style in terms of the formatter to avoid
>> this problem.
>
> I agree it is lovely not to have to think about the style rules when
> writing code for a project that has an automatic formatter. As Junio 
> said I think it would take a bit of work to persuade clang-format to
> match our style and I'm concerned that adopting the style it produces 
> now would lead to a lot of churn. Below is an example taken from [1]
> that shows one area for improvement. At the moment its struct
> formatting seems inconsistent (one struct ends up with one field per
> line and the second has more than one field per line with a completely
> different indentation to the first) and I'm not sure we want it
> reformatting the whole definition when a new member is added. When
> Han-Wen Nienhuys submitted that patch I did have a brief play with the
> clang-format rules to try and improve it but didn't get anywhere.
>
> [1]
> https://lore.kernel.org/git/2c2f94ddc0e77c8c70041a2a736e3a56698f058c.1589226388.git.gitgitgadget@gmail.com
>
> @@ -3173,30 +3282,31 @@ static int files_init_db(struct ref_store
> *ref_store, struct strbuf *err)
>  	return 0;
>  }
>
> -struct ref_storage_be refs_be_files = {
> -	NULL,
> -	"files",
> -	files_ref_store_create,
> -	files_init_db,
> -	files_transaction_prepare,
> -	files_transaction_finish,
> -	files_transaction_abort,
> -	files_initial_transaction_commit,
> -
> -	files_pack_refs,
> -	files_create_symref,
> -	files_delete_refs,
> -	files_rename_ref,
> -	files_copy_ref,
> -
> -	files_ref_iterator_begin,
> -	files_read_raw_ref,
> -
> -	files_reflog_iterator_begin,
> -	files_for_each_reflog_ent,
> -	files_for_each_reflog_ent_reverse,
> -	files_reflog_exists,
> -	files_create_reflog,
> -	files_delete_reflog,
> -	files_reflog_expire
> -};
> +struct ref_storage_be refs_be_files = { NULL,
> +					"files",
> +					files_ref_store_create,
> +					files_init_db,
> +					files_transaction_prepare,
> +					files_transaction_finish,
> +					files_transaction_abort,
> +					files_initial_transaction_commit,
> +
> +					files_pack_refs,
> +					files_create_symref,
> +					files_delete_refs,
> +					files_rename_ref,
> +					files_copy_ref,
> +
> +					files_write_pseudoref,
> +					files_delete_pseudoref,
> +
> +					files_ref_iterator_begin,
> +					files_read_raw_ref,
> +
> +					files_reflog_iterator_begin,
> +					files_for_each_reflog_ent,
> +					files_for_each_reflog_ent_reverse,
> +					files_reflog_exists,
> +					files_create_reflog,
> +					files_delete_reflog,
> +					files_reflog_expire };

As my RFC series in the side-thread notes there's a lot of little
outstanding issues with its formatting, but this in particular isn't one
of them.

This one was resolved in my 32bff617c6a (refs: use designated
initializers for "struct ref_storage_be", 2022-03-17), and clang-format
currently has no formatting suggstions for these assignments on
"master".

And in general I think all such large assignments we wanted to convert
to designated initializers anyway, and/or have already done so.

With that series try:

	make style-all-diff-apply &&
	git diff

There's lots of outstanding issues for sure, for structs we lose some
borderline ascii-art alignment in some cases (e.g. the one in tar.h),
but I'd think those would be OK to either just format consistently, or
use "/* clang-format off */".