git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] doc: add shortcut to "am --whitespace=<action>"
@ 2024-02-14 18:44 Junio C Hamano
  2024-02-14 19:01 ` Dragan Simic
  0 siblings, 1 reply; 5+ messages in thread
From: Junio C Hamano @ 2024-02-14 18:44 UTC (permalink / raw)
  To: git

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(-)

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);
-- 
2.44.0-rc1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] doc: add shortcut to "am --whitespace=<action>"
  2024-02-14 18:44 [PATCH] doc: add shortcut to "am --whitespace=<action>" Junio C Hamano
@ 2024-02-14 19:01 ` Dragan Simic
  2024-02-14 19:03   ` Dragan Simic
  2024-02-14 19:09   ` Junio C Hamano
  0 siblings, 2 replies; 5+ messages in thread
From: Dragan Simic @ 2024-02-14 19:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

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);

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] doc: add shortcut to "am --whitespace=<action>"
  2024-02-14 19:01 ` Dragan Simic
@ 2024-02-14 19:03   ` Dragan Simic
  2024-02-14 19:09   ` Junio C Hamano
  1 sibling, 0 replies; 5+ messages in thread
From: Dragan Simic @ 2024-02-14 19:03 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

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);

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] doc: add shortcut to "am --whitespace=<action>"
  2024-02-14 19:01 ` Dragan Simic
  2024-02-14 19:03   ` Dragan Simic
@ 2024-02-14 19:09   ` Junio C Hamano
  2024-02-14 19:20     ` Dragan Simic
  1 sibling, 1 reply; 5+ messages in thread
From: Junio C Hamano @ 2024-02-14 19:09 UTC (permalink / raw)
  To: Dragan Simic; +Cc: git

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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] doc: add shortcut to "am --whitespace=<action>"
  2024-02-14 19:09   ` Junio C Hamano
@ 2024-02-14 19:20     ` Dragan Simic
  0 siblings, 0 replies; 5+ messages in thread
From: Dragan Simic @ 2024-02-14 19:20 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-02-14 19:20 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-14 18:44 [PATCH] doc: add shortcut to "am --whitespace=<action>" Junio C Hamano
2024-02-14 19:01 ` Dragan Simic
2024-02-14 19:03   ` Dragan Simic
2024-02-14 19:09   ` Junio C Hamano
2024-02-14 19:20     ` Dragan Simic

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).