From: Junio C Hamano <gitster@pobox.com>
To: "Windl, Ulrich" <u.windl@ukr.de>
Cc: "René Scharfe" <l.s.r@web.de>,
"git@vger.kernel.org" <git@vger.kernel.org>,
"Phillip Wood" <phillip.wood@dunelm.org.uk>
Subject: Re: [EXT] [PATCH v3 6/6] add-patch: reset "permitted" at loop start
Date: Fri, 31 Oct 2025 08:16:13 -0700 [thread overview]
Message-ID: <xmqqfraz2jb6.fsf@gitster.g> (raw)
In-Reply-To: <77991a11c53f40b8b0a050a4d081809a@ukr.de> (Ulrich Windl's message of "Fri, 31 Oct 2025 10:28:47 +0000")
"Windl, Ulrich" <u.windl@ukr.de> writes:
> Just a comment of personal taste: I think declaring an anonymous
> enum inside a loop is just bad style. I think that gcc is smart
> enough to optimize if "permitted" is declared outside the loop, or
> make the "permitted" use a typedef for a "named enum" (declared
> outside the loop while the variable may be inside the loop).
If this is more than just a personal preference (which to me does
sound like), a patch to improve it on top is very much welcomed.
The change itself would be just reverting the code movement, drop
the 0 initialization and resetting the ariable at the top of the
loop every iteration. But the rationale being that it would give
compilers a chance to do a better job, I'd prefer to see a compiler
person write the proposed log message, possibly backed by data
(perhaps "generated assembly is objectively better---compare this
and that" in this case? I dunno).
Thanks.
>> -----Original Message-----
>> From: René Scharfe <l.s.r@web.de>
>> Sent: Monday, October 6, 2025 7:24 PM
>> To: git@vger.kernel.org
>> Cc: Windl, Ulrich <u.windl@ukr.de>; Junio C Hamano <gitster@pobox.com>;
>> Phillip Wood <phillip.wood@dunelm.org.uk>
>> Subject: [EXT] [PATCH v3 6/6] add-patch: reset "permitted" at loop start
>>
> [...]
>> for (;;) {
>> + enum {
>> + ALLOW_GOTO_PREVIOUS_HUNK = 1 << 0,
>> + ALLOW_GOTO_PREVIOUS_UNDECIDED_HUNK = 1 <<
>> 1,
>> + ALLOW_GOTO_NEXT_HUNK = 1 << 2,
>> + ALLOW_GOTO_NEXT_UNDECIDED_HUNK = 1 << 3,
>> + ALLOW_SEARCH_AND_GOTO = 1 << 4,
>> + ALLOW_SPLIT = 1 << 5,
>> + ALLOW_EDIT = 1 << 6
>> + } permitted = 0;
>> +
>> if (hunk_index >= file_diff->hunk_nr)
>> hunk_index = 0;
>> hunk = file_diff->hunk_nr
next prev parent reply other threads:[~2025-10-31 15:16 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-02 9:23 Broken handling of "J" hunks for "add --interactive"? Windl, Ulrich
2025-10-03 12:16 ` [PATCH] add-patch: roll over to next undecided hunk René Scharfe
2025-10-03 13:41 ` Phillip Wood
2025-10-03 14:10 ` René Scharfe
2025-10-08 13:47 ` Phillip Wood
2025-10-03 16:11 ` Junio C Hamano
2025-10-03 19:53 ` René Scharfe
2025-10-03 20:39 ` Junio C Hamano
2025-10-03 20:42 ` Junio C Hamano
2025-10-03 21:18 ` Junio C Hamano
2025-10-05 15:45 ` [PATCH v2 0/5] " René Scharfe
2025-10-05 15:55 ` [PATCH v2 1/5] add-patch: improve help for options j, J, k, and K René Scharfe
2025-10-05 21:30 ` Junio C Hamano
2025-10-06 17:17 ` René Scharfe
2025-10-06 17:58 ` Junio C Hamano
2025-10-31 10:08 ` [EXT] " Windl, Ulrich
2025-11-01 8:18 ` Junio C Hamano
2025-11-03 12:43 ` [EXT] " Windl, Ulrich
2025-10-05 15:55 ` [PATCH v2 2/5] add-patch: document that option J rolls over René Scharfe
2025-10-05 21:30 ` Junio C Hamano
2025-10-05 15:55 ` [PATCH v2 3/5] add-patch: let options y, n, j, and e roll over to next undecided René Scharfe
2025-10-05 15:55 ` [PATCH v2 4/5] add-patch: let options k and K roll over like j and J René Scharfe
2025-10-05 20:55 ` Junio C Hamano
2025-10-06 17:18 ` René Scharfe
2025-10-05 15:55 ` [PATCH v2 5/5] add-patch: reset "permitted" at loop start René Scharfe
2025-10-06 17:18 ` [PATCH v3 0/6] add-patch: roll over to next undecided hunk René Scharfe
2025-10-06 17:19 ` [PATCH v3 1/6] add-patch: improve help for options j, J, k, and K René Scharfe
2025-10-06 17:20 ` [PATCH v3 2/6] add-patch: document that option J rolls over René Scharfe
2025-10-06 17:21 ` [PATCH v3 3/6] add-patch: let options y, n, j, and e roll over to next undecided René Scharfe
2025-10-06 17:22 ` [PATCH v3 4/6] add-patch: let options k and K roll over like j and J René Scharfe
2025-10-06 17:23 ` [PATCH v3 5/6] add-patch: let options a and d roll over like y and n René Scharfe
2025-10-06 17:24 ` [PATCH v3 6/6] add-patch: reset "permitted" at loop start René Scharfe
2025-10-31 10:28 ` [EXT] " Windl, Ulrich
2025-10-31 15:16 ` Junio C Hamano [this message]
2025-10-06 18:00 ` [PATCH v3 0/6] add-patch: roll over to next undecided hunk Junio C Hamano
2025-10-06 20:05 ` René Scharfe
2025-10-06 22:01 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqfraz2jb6.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=l.s.r@web.de \
--cc=phillip.wood@dunelm.org.uk \
--cc=u.windl@ukr.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.