Patch that modifies git usage message

Patch that modifies git usage message

[Alangi Derick]

[-- Attachment #1: Type: text/plain, Size: 132 bytes --]

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

Regards
Alangi Derick Ndimnain

[-- Attachment #2: git_usage_modified.patch --]
[-- Type: text/x-patch, Size: 405 bytes --]

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

Re: Patch that modifies git usage message

[Stefan Beller]

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.

Re: Patch that modifies git usage message

[Alangi Derick]

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:
> 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.

Re: Patch that modifies git usage message

[Junio C Hamano]

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.

Re: Patch that modifies git usage message

[Alangi Derick]

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:
> 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.

Re: Patch that modifies git usage message

[Stefan Beller]

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

Re: Patch that modifies git usage message

[Eric Sunshine]

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

Re: Patch that modifies git usage message

[Alangi Derick]

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.

Re: Patch that modifies git usage message

[Eric Sunshine]

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.

Re: Patch that modifies git usage message

[Stefan Beller]

On Fri, May 1, 2015 at 9:29 AM, Junio C Hamano <gitster@pobox.com> wrote:
> 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

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