threads / discuss / 39197

Patch that modifies git usage message

Subject: Patch that modifies git usage message

## tl;dr

10 messages between May 1, 2015 and May 1, 2015.

replies: 9people: 4as markdown or json

Alangi Derick· May 1, 2015, 11:01 UTC · lore

This patch just modifies the "usage" word to "Usage" which enhances readability. Below is the patch

Regards Alangi Derick Ndimnain

diff --git a/git.c b/git.c
index 42a4ee5..481aa74 100644
--- a/git.c
+++ b/git.c
@@ -667,7 +667,7 @@ int main(int argc, char **av)
 	} else {
 		/* The user didn't specify a command; give them help */
 		commit_pager_choice();
-		printf("usage: %s\n\n", git_usage_string);
+		printf("Usage: %s\n\n", git_usage_string);
 		list_common_cmds_help();
 		printf("\n%s\n", _(git_more_info_string));
 		exit(1);
Stefan Beller· May 1, 2015, 15:51 UTC · re: Alangi Derick · lore

Re: Patch that modifies git usage message

On Fri, May 1, 2015 at 4:01 AM, Alangi Derick <alangiderick@gmail.com> wrote:
Show 5 quoted lines
> This patch just modifies the "usage" word to "Usage" which enhances
> readability. Below is the patch
>
> Regards
> Alangi Derick Ndimnain

It's easier to have the patch in the email itself, this looks it's some form of attachment.

Checkout Documentation/SubmittingPatches (protip: get "git send-email" working, it will send in the preferred way by default for nearly any open source project using email based workflows.)

There are also some resources on the web, how to send patches, although it should be all covered in our Documentation, http://alblue.bandlem.com/2011/12/git-tip-of-week-patches-by-email.html

For the patch itself:

$ grep -r usage *.c builtin/*.c |wc -l 551 $ grep -r Usage *.c builtin/*.c |wc -l 3

The community agreed (maybe subconciously) to prefer lower case for the 'usage' string, so I don't think this is an improvement.

Alangi Derick· May 1, 2015, 15:54 UTC · re: Stefan Beller · lore

Re: Patch that modifies git usage message

I am saying so because there are some strings with upper case letters and others with lower case letters. So there is no consistency. That is why i wanted to keep the consistency by correcting the letters which are to be corrected and allow the once which are to be allowed. Regards Alangi Derick Ndimnain

On Fri, May 1, 2015 at 4:51 PM, Stefan Beller <sbeller@google.com> wrote:
Show 30 quoted lines
> On Fri, May 1, 2015 at 4:01 AM, Alangi Derick <alangiderick@gmail.com> wrote:
>> This patch just modifies the "usage" word to "Usage" which enhances
>> readability. Below is the patch
>>
>> Regards
>> Alangi Derick Ndimnain
>
> It's easier to have the patch in the email itself, this looks it's
> some form of attachment.
>
> Checkout Documentation/SubmittingPatches (protip: get "git send-email"
> working, it will
> send in the preferred way by default for nearly any open source
> project using email based
> workflows.)
>
> There are also some resources on the web, how to send patches,
> although it should be
> all covered in our Documentation,
> http://alblue.bandlem.com/2011/12/git-tip-of-week-patches-by-email.html
>
> For the patch itself:
>
> $ grep -r usage *.c builtin/*.c |wc -l
> 551
> $ grep -r Usage *.c builtin/*.c |wc -l
> 3
>
> The community agreed (maybe subconciously) to prefer lower case
> for the 'usage' string, so I don't think this is an improvement.
Junio C Hamano· May 1, 2015, 16:29 UTC · re: Stefan Beller · lore

Re: Patch that modifies git usage message

Stefan Beller <sbeller@google.com> writes:
Show 9 quoted lines
> For the patch itself:
>
> $ grep -r usage *.c builtin/*.c |wc -l
> 551
> $ grep -r Usage *.c builtin/*.c |wc -l
> 3
>
> The community agreed (maybe subconciously) to prefer lower case
> for the 'usage' string, so I don't think this is an improvement.

I tend to agree with the conclusion, but you need to be a bit careful here. These catch all the variable names that contain "[uU]sage" as substring, but we do not spell in-code variables with camelCase, so the former probably is over-counting. Things like "static const char usage[] = ..." are counted; so are calls to usage_with_options().

If you look for the beginning of a string constant, you would get this:

    $ git grep '"usage' -- \*.c builtin/\*.c
    12
    $ git grep '"Usage' -- \*.c builtin/\*.c
    0

The former undercounts the messages because many usage messages are produced by calling usage_with_options() these days.

The latter being zero made me scratch my head and do this:
    $ git grep Usage -- \*.c builtin/\*.c
    commit.c: * Usage example:
    test-hashmap.c: * Usage: time echo "perfhas...

I cannot find the third one you found for "Usage" in your example, though.

Alangi Derick· May 1, 2015, 16:38 UTC · re: Junio C Hamano · lore

Re: Patch that modifies git usage message

Ok, thanks for the correction. What about the other strings used for error display? For example die("cannot handle %s as a builtin", cmd); Can't i change the "cannot" to "Cannot"? Or is there a problem with that one too? Regards Alangi Derick Ndimnain

On Fri, May 1, 2015 at 5:29 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 38 quoted lines
> Stefan Beller <sbeller@google.com> writes:
>
>> For the patch itself:
>>
>> $ grep -r usage *.c builtin/*.c |wc -l
>> 551
>> $ grep -r Usage *.c builtin/*.c |wc -l
>> 3
>>
>> The community agreed (maybe subconciously) to prefer lower case
>> for the 'usage' string, so I don't think this is an improvement.
>
> I tend to agree with the conclusion, but you need to be a bit
> careful here.  These catch all the variable names that contain
> "[uU]sage" as substring, but we do not spell in-code variables
> with camelCase, so the former probably is over-counting.  Things
> like "static const char usage[] = ..." are counted; so are calls
> to usage_with_options().
>
> If you look for the beginning of a string constant, you would get
> this:
>
>     $ git grep '"usage' -- \*.c builtin/\*.c
>     12
>     $ git grep '"Usage' -- \*.c builtin/\*.c
>     0
>
> The former undercounts the messages because many usage messages are
> produced by calling usage_with_options() these days.
>
> The latter being zero made me scratch my head and do this:
>
>     $ git grep Usage -- \*.c builtin/\*.c
>     commit.c: * Usage example:
>     test-hashmap.c: * Usage: time echo "perfhas...
>
> I cannot find the third one you found for "Usage" in your example,
> though.
Stefan Beller· May 1, 2015, 16:55 UTC · re: Alangi Derick · lore

Re: Patch that modifies git usage message

On Fri, May 1, 2015 at 9:38 AM, Alangi Derick <alangiderick@gmail.com> wrote:
> Can't i change the "cannot" to "Cannot"? Or is there a problem with
> that one too?

Well maybe you can make a strong argument for changing it like you'd find it inconsistencies in different spots.

If you're in the game just to change a thing or get a patch into Git, you may want to look at http://git-blame.blogspot.com/p/leftover-bits.html https://git.wiki.kernel.org/index.php/SmallProjectsIdeas (As you seem to focus on the textual UI, maybe https://git.wiki.kernel.org/index.php/SmallProjectsIdeas#improve_.22git_help.22 might be a good idea to start with?)

Eric Sunshine· May 1, 2015, 17:40 UTC · re: Alangi Derick · lore

Re: Patch that modifies git usage message

(Etiquette on this list is to reply inline rather than top-posting[1].)
On Fri, May 1, 2015 at 12:38 PM, Alangi Derick <alangiderick@gmail.com> wrote:
Show 5 quoted lines
> What about the other strings used for
> error display? For example
> die("cannot handle %s as a builtin", cmd);
> Can't i change the "cannot" to "Cannot"? Or is there a problem with
> that one too?

Despite inconsistencies in existing code, lowercase in new error messages is intentional. Documentation/CodingGuidelines has this to say[2]:

    Error Messages
    - Do not capitalize ("unable to open %s", not "Unable to
      open %s")

Therefore, a goal more aligned with this recommendation would be to submit a patch which changes capitalized error messages to lowercase, however, heed this warning[3] from CodingGuidelines:

   - Fixing style violations while working on a real change as a
     preparatory clean-up step is good, but otherwise avoid useless
     code churn for the sake of conforming to the style.
     "Once it _is_ in the tree, it's not really worth the patch noise
     to go and fix it up."
     Cf. http://article.gmane.org/gmane.linux.kernel/943020

Sometimes there are exceptions. One may be able to argue that making user-facing messages more consistent is worthwhile (for instance [4]).

Finally, changing "usage:" to "Usage:" would undo recent work to improve consistency of usage messages[4].

[1]: https://lkml.org/lkml/2005/1/11/111 [2]: https://github.com/git/git/blob/master/Documentation/CodingGuidelines#L416 [3]: https://github.com/git/git/blob/master/Documentation/CodingGuidelines#L21 [4]: http://thread.gmane.org/gmane.comp.version-control.git/216961

Alangi Derick· May 1, 2015, 17:49 UTC · re: Eric Sunshine · lore

Re: Patch that modifies git usage message

So in essence what you are saying is that i should instead change error messages to lower case. And also i have seen an idea i want to work on from https://git.wiki.kernel.org/index.php/SmallProjectsIdeas I want to work on this and will submit a patch as soon as i am done.

Eric Sunshine· May 1, 2015, 18:01 UTC · re: Alangi Derick · lore

Re: Patch that modifies git usage message

On Fri, May 1, 2015 at 1:49 PM, Alangi Derick <alangiderick@gmail.com> wrote:
> So in essence what you are saying is that i should instead change
> error messages to lower case.

No, I'm not saying that you _should_ do anything. I am saying merely that if you were to submit such a patch for the sake of improving consistency, you might have an easier time convincing people that your patch is worthwhile if you follow the current trend (which is to make error messages lowercase).

However, I'm also saying that such a patch may encounter resistance if it is perceived as mere "noise", as explained by the CodingGuidelines blurb which I cited.

> And also i have seen an idea i want to work on from
> https://git.wiki.kernel.org/index.php/SmallProjectsIdeas
> I want to work on this and will submit a patch as soon as i am done.
Stefan Beller· May 1, 2015, 16:55 UTC · re: Junio C Hamano · lore

Re: Patch that modifies git usage message

On Fri, May 1, 2015 at 9:29 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 18 quoted lines
> Stefan Beller <sbeller@google.com> writes:
>
>> For the patch itself:
>>
>> $ grep -r usage *.c builtin/*.c |wc -l
>> 551
>> $ grep -r Usage *.c builtin/*.c |wc -l
>> 3
>>
>> The community agreed (maybe subconciously) to prefer lower case
>> for the 'usage' string, so I don't think this is an improvement.
>
> I tend to agree with the conclusion, but you need to be a bit
> careful here.  These catch all the variable names that contain
> "[uU]sage" as substring, but we do not spell in-code variables
> with camelCase, so the former probably is over-counting.  Things
> like "static const char usage[] = ..." are counted; so are calls
> to usage_with_options().

I knew my search was off as I did not think it through, but I just wanted to have at least some data to not be hand waving only here.

Maybe we can also rely on the colon in this case: $ grep -r "usage:" *.{c,sh,perl} finds 22 results, 21 thereof look like human readable usage instructions $ grep -r "Usage:" *.{c,sh,perl} finds 2 results which look like human readable text

Show 20 quoted lines
>
> If you look for the beginning of a string constant, you would get
> this:
>
>     $ git grep '"usage' -- \*.c builtin/\*.c
>     12
>     $ git grep '"Usage' -- \*.c builtin/\*.c
>     0
>
> The former undercounts the messages because many usage messages are
> produced by calling usage_with_options() these days.
>
> The latter being zero made me scratch my head and do this:
>
>     $ git grep Usage -- \*.c builtin/\*.c
>     commit.c: * Usage example:
>     test-hashmap.c: * Usage: time echo "perfhas...
>
> I cannot find the third one you found for "Usage" in your example,
> though.

test-submodule-config.c: fprintf(stderr, "Usage: %s [<commit> <submodulepath>] ...\n", argv[0]); (I just realize HEAD wa pointing somewhere in pu)

← back to recent threads