diff mbox series

Documentation: amended usages of various (sub)commands

Message ID pull.920.git.1616913298624.gitgitgadget@gmail.com (mailing list archive)
State Accepted
Commit 5ee90326dceb23744a86ad10a60ccda9a2dc3f30
Headers show
Series Documentation: amended usages of various (sub)commands | expand

Commit Message

Chinmoy Chakraborty March 28, 2021, 6:34 a.m. UTC
From: Chinmoy Chakraborty <chinmoy12c@gmail.com>

The Git suite option parsing API's Technical Documentation suggests this
about the option descriptions of a (sub)command:

`description` is a short string to describe the effect of the option.
It shall begin with a lower-case letter and a full stop (.) shall be
omitted at the end.

Various (sub)commands' option arrays don't follow the guideline provided
by the parse_options Documentation regarding the descriptions.

Signed-off-by: Chinmoy Chakraborty <chinmoy12c@gmail.com>
---
    Documentation: amended usages of various (sub)commands
    
    The Git suite option parsing API's Technical Documentation suggests this
    about the option descriptions of a (sub)command:
    
    description is a short string to describe the effect of the option. It
    shall begin with a lower-case letter and a full stop (.) shall be
    omitted at the end.
    
    Various (sub)commands' option arrays don't follow the guideline provided
    by the parse_options Documentation regarding the descriptions.
    
    Signed-off-by: Chinmoy Chakraborty chinmoy12c@gmail.com

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-920%2Fchinmoy12c%2Fissue_636-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-920/chinmoy12c/issue_636-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/920

 builtin/column.c     | 8 ++++----
 builtin/range-diff.c | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)


base-commit: 84d06cdc06389ae7c462434cb7b1db0980f63860

Comments

Bagas Sanjaya March 28, 2021, 6:56 a.m. UTC | #1
On 28/03/21 13.34, Chinmoy via GitGitGadget wrote:
> From: Chinmoy Chakraborty <chinmoy12c@gmail.com>
> 
> The Git suite option parsing API's Technical Documentation suggests this
> about the option descriptions of a (sub)command:
> 
> `description` is a short string to describe the effect of the option.
> It shall begin with a lower-case letter and a full stop (.) shall be
> omitted at the end.
> 
> Various (sub)commands' option arrays don't follow the guideline provided
> by the parse_options Documentation regarding the descriptions.
> 
> Signed-off-by: Chinmoy Chakraborty <chinmoy12c@gmail.com>
> ---
>      Documentation: amended usages of various (sub)commands
>      
>      The Git suite option parsing API's Technical Documentation suggests this
>      about the option descriptions of a (sub)command:
>      
>      description is a short string to describe the effect of the option. It
>      shall begin with a lower-case letter and a full stop (.) shall be
>      omitted at the end.
>      
>      Various (sub)commands' option arrays don't follow the guideline provided
>      by the parse_options Documentation regarding the descriptions.
>      
>      Signed-off-by: Chinmoy Chakraborty chinmoy12c@gmail.com
> 
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-920%2Fchinmoy12c%2Fissue_636-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-920/chinmoy12c/issue_636-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/920
> 
>   builtin/column.c     | 8 ++++----
>   builtin/range-diff.c | 2 +-
>   2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/builtin/column.c b/builtin/column.c
> index e815e148aa18..40d4b3bee2dd 100644
> --- a/builtin/column.c
> +++ b/builtin/column.c
> @@ -27,10 +27,10 @@ int cmd_column(int argc, const char **argv, const char *prefix)
>   		OPT_STRING(0, "command", &real_command, N_("name"), N_("lookup config vars")),
>   		OPT_COLUMN(0, "mode", &colopts, N_("layout to use")),
>   		OPT_INTEGER(0, "raw-mode", &colopts, N_("layout to use")),
> -		OPT_INTEGER(0, "width", &copts.width, N_("Maximum width")),
> -		OPT_STRING(0, "indent", &copts.indent, N_("string"), N_("Padding space on left border")),
> -		OPT_INTEGER(0, "nl", &copts.nl, N_("Padding space on right border")),
> -		OPT_INTEGER(0, "padding", &copts.padding, N_("Padding space between columns")),
> +		OPT_INTEGER(0, "width", &copts.width, N_("maximum width")),
> +		OPT_STRING(0, "indent", &copts.indent, N_("string"), N_("padding space on left border")),
> +		OPT_INTEGER(0, "nl", &copts.nl, N_("padding space on right border")),
> +		OPT_INTEGER(0, "padding", &copts.padding, N_("padding space between columns")),
>   		OPT_END()
>   	};
>   
> diff --git a/builtin/range-diff.c b/builtin/range-diff.c
> index 78bc9fa77062..50318849d657 100644
> --- a/builtin/range-diff.c
> +++ b/builtin/range-diff.c
> @@ -25,7 +25,7 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)
>   	struct option range_diff_options[] = {
>   		OPT_INTEGER(0, "creation-factor",
>   			    &range_diff_opts.creation_factor,
> -			    N_("Percentage by which creation is weighted")),
> +			    N_("percentage by which creation is weighted")),
>   		OPT_BOOL(0, "no-dual-color", &simple_color,
>   			    N_("use simple diff colors")),
>   		OPT_PASSTHRU_ARGV(0, "notes", &other_arg,
> 
> base-commit: 84d06cdc06389ae7c462434cb7b1db0980f63860
> 
Wait, I expected that this patch touches Documentation/* (as the title implied),
but it seems like the patch content is something else (not related).

Totally wrong patch submitted here.
Bagas Sanjaya March 28, 2021, 8:13 a.m. UTC | #2
On 28/03/21 13.56, Bagas Sanjaya wrote:
> On 28/03/21 13.34, Chinmoy via GitGitGadget wrote:
>> From: Chinmoy Chakraborty <chinmoy12c@gmail.com>
>>
>> The Git suite option parsing API's Technical Documentation suggests this
>> about the option descriptions of a (sub)command:
>>
>> `description` is a short string to describe the effect of the option.
>> It shall begin with a lower-case letter and a full stop (.) shall be
>> omitted at the end.
>>
>> Various (sub)commands' option arrays don't follow the guideline provided
>> by the parse_options Documentation regarding the descriptions.
>>
>> Signed-off-by: Chinmoy Chakraborty <chinmoy12c@gmail.com>
>> ---
>>      Documentation: amended usages of various (sub)commands
>>      The Git suite option parsing API's Technical Documentation suggests this
>>      about the option descriptions of a (sub)command:
>>      description is a short string to describe the effect of the option. It
>>      shall begin with a lower-case letter and a full stop (.) shall be
>>      omitted at the end.
>>      Various (sub)commands' option arrays don't follow the guideline provided
>>      by the parse_options Documentation regarding the descriptions.
>>      Signed-off-by: Chinmoy Chakraborty chinmoy12c@gmail.com
>>
>> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-920%2Fchinmoy12c%2Fissue_636-v1
>> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-920/chinmoy12c/issue_636-v1
>> Pull-Request: https://github.com/gitgitgadget/git/pull/920
>>
>>   builtin/column.c     | 8 ++++----
>>   builtin/range-diff.c | 2 +-
>>   2 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/builtin/column.c b/builtin/column.c
>> index e815e148aa18..40d4b3bee2dd 100644
>> --- a/builtin/column.c
>> +++ b/builtin/column.c
>> @@ -27,10 +27,10 @@ int cmd_column(int argc, const char **argv, const char *prefix)
>>           OPT_STRING(0, "command", &real_command, N_("name"), N_("lookup config vars")),
>>           OPT_COLUMN(0, "mode", &colopts, N_("layout to use")),
>>           OPT_INTEGER(0, "raw-mode", &colopts, N_("layout to use")),
>> -        OPT_INTEGER(0, "width", &copts.width, N_("Maximum width")),
>> -        OPT_STRING(0, "indent", &copts.indent, N_("string"), N_("Padding space on left border")),
>> -        OPT_INTEGER(0, "nl", &copts.nl, N_("Padding space on right border")),
>> -        OPT_INTEGER(0, "padding", &copts.padding, N_("Padding space between columns")),
>> +        OPT_INTEGER(0, "width", &copts.width, N_("maximum width")),
>> +        OPT_STRING(0, "indent", &copts.indent, N_("string"), N_("padding space on left border")),
>> +        OPT_INTEGER(0, "nl", &copts.nl, N_("padding space on right border")),
>> +        OPT_INTEGER(0, "padding", &copts.padding, N_("padding space between columns")),
>>           OPT_END()
>>       };
>> diff --git a/builtin/range-diff.c b/builtin/range-diff.c
>> index 78bc9fa77062..50318849d657 100644
>> --- a/builtin/range-diff.c
>> +++ b/builtin/range-diff.c
>> @@ -25,7 +25,7 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)
>>       struct option range_diff_options[] = {
>>           OPT_INTEGER(0, "creation-factor",
>>                   &range_diff_opts.creation_factor,
>> -                N_("Percentage by which creation is weighted")),
>> +                N_("percentage by which creation is weighted")),
>>           OPT_BOOL(0, "no-dual-color", &simple_color,
>>                   N_("use simple diff colors")),
>>           OPT_PASSTHRU_ARGV(0, "notes", &other_arg,
>>
>> base-commit: 84d06cdc06389ae7c462434cb7b1db0980f63860
>>
> Wait, I expected that this patch touches Documentation/* (as the title implied),
> but it seems like the patch content is something else (not related).
> 
> Totally wrong patch submitted here.
> 
Oops, I don't see that diff above. Sorry for that.

What about this patch title below?:

`Make (sub)command options conform to TD of option parsing`
Chinmoy Chakraborty March 28, 2021, 8:24 a.m. UTC | #3
On 3/28/21 1:43 PM, Bagas Sanjaya wrote:
> Oops, I don't see that diff above. Sorry for that.
That's fine :).
>
> What about this patch title below?:
>
> `Make (sub)command options conform to TD of option parsing`
>
Sure I'll update the title. Thanks for the suggestion.
Junio C Hamano March 28, 2021, 6:32 p.m. UTC | #4
Bagas Sanjaya <bagasdotme@gmail.com> writes:

>>> ...
>> Wait, I expected that this patch touches Documentation/* (as the title implied),
>> but it seems like the patch content is something else (not related).
>> Totally wrong patch submitted here.

Please learn to trim your quotes.

> Oops, I don't see that diff above. Sorry for that.
>
> What about this patch title below?:
>
> `Make (sub)command options conform to TD of option parsing`

"Make X conform" has low information contents (especially when
nobody knows what the TD is), because nobody would try to make the
code worse by making things nonconforming.

    Subject: [PATCH] column, range-diff: downcase option description

    It is customary not to begin the help text for each option given to
    the parse-options API with a capital letter.

    Downcase the first word of some option descriptions for "column"
    and "range-diff".

or something like that?

Even though the guideline for the documentation talks about two
things (i.e. not starting with capital and omitting full-stop at the
end), the help strings being touched by the patch only have problem
with the former, so quoting the guideline in full would not help
people understand what is changed and why.
Bagas Sanjaya March 29, 2021, 5:17 a.m. UTC | #5
On 29/03/21 01.32, Junio C Hamano wrote:
>> What about this patch title below?:
>>
>> `Make (sub)command options conform to TD of option parsing`
> 
> "Make X conform" has low information contents (especially when
> nobody knows what the TD is), because nobody would try to make the
> code worse by making things nonconforming.
> 
>      Subject: [PATCH] column, range-diff: downcase option description
> 
>      It is customary not to begin the help text for each option given to
>      the parse-options API with a capital letter.
> 
>      Downcase the first word of some option descriptions for "column"
>      and "range-diff".
> 
> or something like that?

That's what this patch intended to do (from the diff).
diff mbox series

Patch

diff --git a/builtin/column.c b/builtin/column.c
index e815e148aa18..40d4b3bee2dd 100644
--- a/builtin/column.c
+++ b/builtin/column.c
@@ -27,10 +27,10 @@  int cmd_column(int argc, const char **argv, const char *prefix)
 		OPT_STRING(0, "command", &real_command, N_("name"), N_("lookup config vars")),
 		OPT_COLUMN(0, "mode", &colopts, N_("layout to use")),
 		OPT_INTEGER(0, "raw-mode", &colopts, N_("layout to use")),
-		OPT_INTEGER(0, "width", &copts.width, N_("Maximum width")),
-		OPT_STRING(0, "indent", &copts.indent, N_("string"), N_("Padding space on left border")),
-		OPT_INTEGER(0, "nl", &copts.nl, N_("Padding space on right border")),
-		OPT_INTEGER(0, "padding", &copts.padding, N_("Padding space between columns")),
+		OPT_INTEGER(0, "width", &copts.width, N_("maximum width")),
+		OPT_STRING(0, "indent", &copts.indent, N_("string"), N_("padding space on left border")),
+		OPT_INTEGER(0, "nl", &copts.nl, N_("padding space on right border")),
+		OPT_INTEGER(0, "padding", &copts.padding, N_("padding space between columns")),
 		OPT_END()
 	};
 
diff --git a/builtin/range-diff.c b/builtin/range-diff.c
index 78bc9fa77062..50318849d657 100644
--- a/builtin/range-diff.c
+++ b/builtin/range-diff.c
@@ -25,7 +25,7 @@  int cmd_range_diff(int argc, const char **argv, const char *prefix)
 	struct option range_diff_options[] = {
 		OPT_INTEGER(0, "creation-factor",
 			    &range_diff_opts.creation_factor,
-			    N_("Percentage by which creation is weighted")),
+			    N_("percentage by which creation is weighted")),
 		OPT_BOOL(0, "no-dual-color", &simple_color,
 			    N_("use simple diff colors")),
 		OPT_PASSTHRU_ARGV(0, "notes", &other_arg,