All of lore.kernel.org
 help / color / mirror / Atom feed
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

  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.