Re: [PATCH 2/6] commit: add amend suboption to --fixup to create amend! commit

2 messages, 2 authors, 2021-02-19 · open the first message on its own page

Re: [PATCH 2/6] commit: add amend suboption to --fixup to create amend! commit

From: Junio C Hamano <hidden>
Date: 2021-02-18 20:39:20

Junio C Hamano [off-list ref] writes:
quoted
...Thanks, for pointing this out. Also, in the above method for
alnum I think we can initialize an array of alnum[] instead of
alphas[]. Or otherwise I was thinking to instead check:
           if (!isalnum(*c) && *c == ':')
Sure a loop is fine, or alnum[] is fine, or just alpha[] is OK, I
would think.  Do you foresee you'd need --fixup=chomp124:<commit>?
I somehow doubt it.
Having said that, we may regret if we did not include some
punctuation to allow for a multi-word keyword.  IOW, "alpha plus
dash" might be a reasonable minimum.

But what keyword --fixup=<keyword>:<commit> can take is entirely
under our control, so it is not all that unreasonable if we just
forced our developers some discipline to pick a single-word keyword
for any of their future enhancements.  It's not like we are opening
up extensibility to the end-users, who may complain that the way
they can spell their new <keyword> is too limited.  So if we already
have alpha[] and/or a helper function that does strspn(alpha) that
we can reuse elsewhere, I do not think it is worth to try supporting
punctuation.

Thanks.

Re: [PATCH 2/6] commit: add amend suboption to --fixup to create amend! commit

From: Charvi Mendiratta <hidden>
Date: 2021-02-19 06:12:26

On Fri, 19 Feb 2021 at 02:07, Junio C Hamano [off-list ref] wrote:
Junio C Hamano [off-list ref] writes:
quoted
quoted
...Thanks, for pointing this out. Also, in the above method for
alnum I think we can initialize an array of alnum[] instead of
alphas[]. Or otherwise I was thinking to instead check:
           if (!isalnum(*c) && *c == ':')
Sure a loop is fine, or alnum[] is fine, or just alpha[] is OK, I
would think.  Do you foresee you'd need --fixup=chomp124:<commit>?
I somehow doubt it.
Having said that, we may regret if we did not include some
punctuation to allow for a multi-word keyword.  IOW, "alpha plus
dash" might be a reasonable minimum.

But what keyword --fixup=<keyword>:<commit> can take is entirely
under our control, so it is not all that unreasonable if we just
forced our developers some discipline to pick a single-word keyword
for any of their future enhancements.  It's not like we are opening
up extensibility to the end-users, who may complain that the way
they can spell their new <keyword> is too limited.  So if we already
have alpha[] and/or a helper function that does strspn(alpha) that
we can reuse elsewhere, I do not think it is worth to try supporting
punctuation.
Oh, yes punctuation is another point. Then for now maybe we can just make the
helper function to check for alpha[] only.

Thanks and Regards,
Charvi
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help