Junio C Hamano <gitster@xxxxxxxxx> writes: > Offtopic. After applying this topic, I asked clang-format if it > wants to change anything. > > $ git clang-format --diff $(git merge-base HEAD master) > > The result was disasterous. Can "clang-format --diff" mode be > taught a bit more focused to avoid touching existing entries in the > same array (in this case opts[] that has tons of options for the > "git for-each-ref" command), when only one new entry was added, I > wonder? > I couldn't find any way to do something like this. > Also I am not impressed by the change it made to the code that is > commented out (in refs.h). > > Line wrapping it did to refs_ref_iterator_begin() is an improvement, > but those to ref_iterator_seek() and do_for_each_ref_iterator() are > unnecessary (both of these were more readble in the original). > > Even though I found its output better for Toon's "last-modified" > changes, I am not impressed by what clang-format suggested for this > series. > It indeed looks really bad, I had a go with the new changes from 'gitster/kn/clang-format-updates'. Which seems a lot better. However, this does show a problem with using 'RemoveBracesLLVM', where it formats the following: if (...) { ... ... } else { ... } to: if (...) { ... ... } else ... Which isn't our style, I think we should completely drop this too, from my patch series. Let me go ahead and do that. I really want to strip out as many rules as possible to make the number of false positives 0 so we can actually start enforcing clang-format. Once we enforce it, we can slowly see what additional rules work well for us. diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c index 543013cd11..2ec96eff74 100644 --- a/builtin/for-each-ref.c +++ b/builtin/for-each-ref.c @@ -8,7 +8,7 @@ #include "strbuf.h" #include "strvec.h" -static char const * const for_each_ref_usage[] = { +static char const *const for_each_ref_usage[] = { N_("git for-each-ref [<options>] [<pattern>]"), N_("git for-each-ref [--points-at <object>]"), N_("git for-each-ref [--merged [<commit>]] [--no-merged [<commit>]]"), @@ -33,19 +33,19 @@ int cmd_for_each_ref(int argc, struct option opts[] = { OPT_BIT('s', "shell", &format.quote_style, N_("quote placeholders suitably for shells"), QUOTE_SHELL), - OPT_BIT('p', "perl", &format.quote_style, + OPT_BIT('p', "perl", &format.quote_style, N_("quote placeholders suitably for perl"), QUOTE_PERL), - OPT_BIT(0 , "python", &format.quote_style, + OPT_BIT(0, "python", &format.quote_style, N_("quote placeholders suitably for python"), QUOTE_PYTHON), - OPT_BIT(0 , "tcl", &format.quote_style, + OPT_BIT(0, "tcl", &format.quote_style, N_("quote placeholders suitably for Tcl"), QUOTE_TCL), - OPT_BOOL(0, "omit-empty", &format.array_opts.omit_empty, - N_("do not output a newline after empty formatted refs")), + OPT_BOOL(0, "omit-empty", &format.array_opts.omit_empty, + N_("do not output a newline after empty formatted refs")), OPT_GROUP(""), - OPT_INTEGER( 0 , "count", &format.array_opts.max_count, N_("show only <n> matched refs")), - OPT_STRING( 0 , "format", &format.format, N_("format"), N_("format to use for the output")), - OPT_STRING( 0 , "skip-until", &filter.seek, N_("skip-until"), N_("skip references until")), + OPT_INTEGER(0, "count", &format.array_opts.max_count, N_("show only <n> matched refs")), + OPT_STRING(0, "format", &format.format, N_("format"), N_("format to use for the output")), + OPT_STRING(0, "skip-until", &filter.seek, N_("skip-until"), N_("skip references until")), OPT__COLOR(&format.use_color, N_("respect format colors")), OPT_REF_FILTER_EXCLUDE(&filter), OPT_REF_SORT(&sorting_options), diff --git a/refs.c b/refs.c index a4220d3537..d492e1b423 100644 --- a/refs.c +++ b/refs.c @@ -2669,23 +2669,21 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs if (!iter) { iter = refs_ref_iterator_begin(refs, dirname.buf, NULL, 0, DO_FOR_EACH_INCLUDE_BROKEN); - } else if (ref_iterator_seek(iter, dirname.buf, 1) < 0) { - goto cleanup; - } + else if (ref_iterator_seek(iter, dirname.buf, 1) < 0) goto cleanup; - while ((ok = ref_iterator_advance(iter)) == ITER_OK) { - if (skip && - string_list_has_string(skip, iter->refname)) - continue; + while ((ok = ref_iterator_advance(iter)) == ITER_OK) { + if (skip && + string_list_has_string(skip, iter->refname)) + continue; - if (transaction && ref_transaction_maybe_set_rejected( - transaction, *update_idx, - REF_TRANSACTION_ERROR_NAME_CONFLICT)) - continue; + if (transaction && ref_transaction_maybe_set_rejected( + transaction, *update_idx, + REF_TRANSACTION_ERROR_NAME_CONFLICT)) + continue; - strbuf_addf(err, _("'%s' exists; cannot create '%s'"), - iter->refname, refname); - goto cleanup; + strbuf_addf(err, _("'%s' exists; cannot create '%s'"), + iter->refname, refname); + goto cleanup; } if (ok != ITER_DONE) diff --git a/refs.h b/refs.h index c5e08db0ff..41fe96d688 100644 --- a/refs.h +++ b/refs.h @@ -1285,9 +1285,9 @@ enum do_for_each_ref_flags { * The output is ordered by refname. */ struct ref_iterator *refs_ref_iterator_begin( - struct ref_store *refs, - const char *prefix, const char **exclude_patterns, - int trim, enum do_for_each_ref_flags flags); + struct ref_store *refs, + const char *prefix, const char **exclude_patterns, + int trim, enum do_for_each_ref_flags flags); /* * Advance the iterator to the first or next item and return ITER_OK. @@ -1342,5 +1342,4 @@ void ref_iterator_free(struct ref_iterator *ref_iterator); int do_for_each_ref_iterator(struct ref_iterator *iter, each_ref_fn fn, void *cb_data); - #endif /* REFS_H */
Attachment:
signature.asc
Description: PGP signature