Thread (18 messages) 18 messages, 6 authors, 18d ago

Re: Automatic code formatting

From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2022-07-11 13:29:51

On Mon, Jul 11 2022, Phillip Wood wrote:
quoted hunk ↗ jump to hunk
Hi brian

On 10/07/2022 22:50, brian m. carlson wrote:
quoted
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 (local)
@@ -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 */".
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help