| Message ID | xmqqplwyvqby.fsf@gitster.g (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | a171dac73496138ba1ac15a6689ee5478c9a88d0 |
| Headers | show |
| Series | doc: add shortcut to "am --whitespace=<action>" | expand |
On 2024-02-14 19:44, Junio C Hamano wrote: > We refer readers of "git am --help" to "git apply --help" for many > options that are passed through, and most of them are simple > booleans, but --whitespace takes from a set of actions whose names > may slip users' minds. Give a list of them in "git am --help" to > reduce one level of redirection only to find out what they are. > > In the helper function to parse the available options, there was a > helpful comment reminding the developer to update list of <action>s > in the completion script. Mention the two documentation pages there > as well. > > Signed-off-by: Junio C Hamano <gitster@pobox.com> Looking to me, with one small nitpick below. > --- > Documentation/git-am.txt | 3 +++ > apply.c | 3 ++- > 2 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt > index 82dadbecc8..67b12f315f 100644 > --- a/Documentation/git-am.txt > +++ b/Documentation/git-am.txt > @@ -128,6 +128,9 @@ include::rerere-options.txt[] > These flags are passed to the 'git apply' (see linkgit:git-apply[1]) > program that applies > the patch. It would be nice to, while there, move "the patch." to the line above. > ++ > +Valid <action> for the `--whitespace` option are: > +`nowarn`, `warn`, `fix`, `error`, and `error-all`. > > --patch-format:: > By default the command will try to detect the patch format > diff --git a/apply.c b/apply.c > index 3d69fec836..4e57831aeb 100644 > --- a/apply.c > +++ b/apply.c > @@ -78,7 +78,8 @@ static int parse_whitespace_option(struct > apply_state *state, const char *option > return 0; > } > /* > - * Please update $__git_whitespacelist in git-completion.bash > + * Please update $__git_whitespacelist in git-completion.bash, > + * Documentation/git-apply.txt, and Documentation/git-am.txt > * when you add new options. > */ > return error(_("unrecognized whitespace option '%s'"), option);
On 2024-02-14 20:01, Dragan Simic wrote: > On 2024-02-14 19:44, Junio C Hamano wrote: >> We refer readers of "git am --help" to "git apply --help" for many >> options that are passed through, and most of them are simple >> booleans, but --whitespace takes from a set of actions whose names >> may slip users' minds. Give a list of them in "git am --help" to >> reduce one level of redirection only to find out what they are. >> >> In the helper function to parse the available options, there was a >> helpful comment reminding the developer to update list of <action>s >> in the completion script. Mention the two documentation pages there >> as well. >> >> Signed-off-by: Junio C Hamano <gitster@pobox.com> > > Looking to me, with one small nitpick below. Oops... s/to me/good to me/ Sorry for the noise. >> --- >> Documentation/git-am.txt | 3 +++ >> apply.c | 3 ++- >> 2 files changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt >> index 82dadbecc8..67b12f315f 100644 >> --- a/Documentation/git-am.txt >> +++ b/Documentation/git-am.txt >> @@ -128,6 +128,9 @@ include::rerere-options.txt[] >> These flags are passed to the 'git apply' (see linkgit:git-apply[1]) >> program that applies >> the patch. > > It would be nice to, while there, move "the patch." to the line above. > >> ++ >> +Valid <action> for the `--whitespace` option are: >> +`nowarn`, `warn`, `fix`, `error`, and `error-all`. >> >> --patch-format:: >> By default the command will try to detect the patch format >> diff --git a/apply.c b/apply.c >> index 3d69fec836..4e57831aeb 100644 >> --- a/apply.c >> +++ b/apply.c >> @@ -78,7 +78,8 @@ static int parse_whitespace_option(struct >> apply_state *state, const char *option >> return 0; >> } >> /* >> - * Please update $__git_whitespacelist in git-completion.bash >> + * Please update $__git_whitespacelist in git-completion.bash, >> + * Documentation/git-apply.txt, and Documentation/git-am.txt >> * when you add new options. >> */ >> return error(_("unrecognized whitespace option '%s'"), option);
Dragan Simic <dsimic@manjaro.org> writes: >> @@ -128,6 +128,9 @@ include::rerere-options.txt[] >> These flags are passed to the 'git apply' (see linkgit:git-apply[1]) >> program that applies >> the patch. > > It would be nice to, while there, move "the patch." to the line above. It belongs to a separate topic. These files are sources that will be line filled by AsciiDoc, so there is no reason to make such a change unless the topic of your change were to make the source "prettier", which is not the focus of this patch. Having such a change in the same patch will be distracting.
On 2024-02-14 20:09, Junio C Hamano wrote: > Dragan Simic <dsimic@manjaro.org> writes: > >>> @@ -128,6 +128,9 @@ include::rerere-options.txt[] >>> These flags are passed to the 'git apply' (see >>> linkgit:git-apply[1]) >>> program that applies >>> the patch. >> >> It would be nice to, while there, move "the patch." to the line above. > > It belongs to a separate topic. These files are sources that will > be line filled by AsciiDoc, so there is no reason to make such a > change unless the topic of your change were to make the source > "prettier", which is not the focus of this patch. Having such a > change in the same patch will be distracting. Makes sense, it can become distracting, especially in a short patch like this one. It's better to stick to the patch topic, and leave small cleanups to a separate patch.
diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt index 82dadbecc8..67b12f315f 100644 --- a/Documentation/git-am.txt +++ b/Documentation/git-am.txt @@ -128,6 +128,9 @@ include::rerere-options.txt[] These flags are passed to the 'git apply' (see linkgit:git-apply[1]) program that applies the patch. ++ +Valid <action> for the `--whitespace` option are: +`nowarn`, `warn`, `fix`, `error`, and `error-all`. --patch-format:: By default the command will try to detect the patch format diff --git a/apply.c b/apply.c index 3d69fec836..4e57831aeb 100644 --- a/apply.c +++ b/apply.c @@ -78,7 +78,8 @@ static int parse_whitespace_option(struct apply_state *state, const char *option return 0; } /* - * Please update $__git_whitespacelist in git-completion.bash + * Please update $__git_whitespacelist in git-completion.bash, + * Documentation/git-apply.txt, and Documentation/git-am.txt * when you add new options. */ return error(_("unrecognized whitespace option '%s'"), option);
We refer readers of "git am --help" to "git apply --help" for many options that are passed through, and most of them are simple booleans, but --whitespace takes from a set of actions whose names may slip users' minds. Give a list of them in "git am --help" to reduce one level of redirection only to find out what they are. In the helper function to parse the available options, there was a helpful comment reminding the developer to update list of <action>s in the completion script. Mention the two documentation pages there as well. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- Documentation/git-am.txt | 3 +++ apply.c | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-)