* Patch that modifies git usage message
@ 2015-05-01 11:01 Alangi Derick
2015-05-01 15:51 ` Stefan Beller
0 siblings, 1 reply; 10+ messages in thread
From: Alangi Derick @ 2015-05-01 11:01 UTC (permalink / raw)
To: git
[-- 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);
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: Patch that modifies git usage message
2015-05-01 11:01 Patch that modifies git usage message Alangi Derick
@ 2015-05-01 15:51 ` Stefan Beller
2015-05-01 15:54 ` Alangi Derick
2015-05-01 16:29 ` Junio C Hamano
0 siblings, 2 replies; 10+ messages in thread
From: Stefan Beller @ 2015-05-01 15:51 UTC (permalink / raw)
To: Alangi Derick; +Cc: git@vger.kernel.org
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Patch that modifies git usage message
2015-05-01 15:51 ` Stefan Beller
@ 2015-05-01 15:54 ` Alangi Derick
2015-05-01 16:29 ` Junio C Hamano
1 sibling, 0 replies; 10+ messages in thread
From: Alangi Derick @ 2015-05-01 15:54 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Patch that modifies git usage message
2015-05-01 15:51 ` Stefan Beller
2015-05-01 15:54 ` Alangi Derick
@ 2015-05-01 16:29 ` Junio C Hamano
2015-05-01 16:38 ` Alangi Derick
2015-05-01 16:55 ` Stefan Beller
1 sibling, 2 replies; 10+ messages in thread
From: Junio C Hamano @ 2015-05-01 16:29 UTC (permalink / raw)
To: Stefan Beller; +Cc: Alangi Derick, git@vger.kernel.org
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Patch that modifies git usage message
2015-05-01 16:29 ` Junio C Hamano
@ 2015-05-01 16:38 ` Alangi Derick
2015-05-01 16:55 ` Stefan Beller
2015-05-01 17:40 ` Eric Sunshine
2015-05-01 16:55 ` Stefan Beller
1 sibling, 2 replies; 10+ messages in thread
From: Alangi Derick @ 2015-05-01 16:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stefan Beller, git@vger.kernel.org
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Patch that modifies git usage message
2015-05-01 16:38 ` Alangi Derick
@ 2015-05-01 16:55 ` Stefan Beller
2015-05-01 17:40 ` Eric Sunshine
1 sibling, 0 replies; 10+ messages in thread
From: Stefan Beller @ 2015-05-01 16:55 UTC (permalink / raw)
To: Alangi Derick; +Cc: Junio C Hamano, git@vger.kernel.org
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?)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Patch that modifies git usage message
2015-05-01 16:38 ` Alangi Derick
2015-05-01 16:55 ` Stefan Beller
@ 2015-05-01 17:40 ` Eric Sunshine
2015-05-01 17:49 ` Alangi Derick
1 sibling, 1 reply; 10+ messages in thread
From: Eric Sunshine @ 2015-05-01 17:40 UTC (permalink / raw)
To: Alangi Derick; +Cc: Junio C Hamano, Stefan Beller, git@vger.kernel.org
(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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Patch that modifies git usage message
2015-05-01 17:40 ` Eric Sunshine
@ 2015-05-01 17:49 ` Alangi Derick
2015-05-01 18:01 ` Eric Sunshine
0 siblings, 1 reply; 10+ messages in thread
From: Alangi Derick @ 2015-05-01 17:49 UTC (permalink / raw)
To: Eric Sunshine; +Cc: Junio C Hamano, Stefan Beller, git@vger.kernel.org
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Patch that modifies git usage message
2015-05-01 17:49 ` Alangi Derick
@ 2015-05-01 18:01 ` Eric Sunshine
0 siblings, 0 replies; 10+ messages in thread
From: Eric Sunshine @ 2015-05-01 18:01 UTC (permalink / raw)
To: Alangi Derick; +Cc: Junio C Hamano, Stefan Beller, git@vger.kernel.org
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Patch that modifies git usage message
2015-05-01 16:29 ` Junio C Hamano
2015-05-01 16:38 ` Alangi Derick
@ 2015-05-01 16:55 ` Stefan Beller
1 sibling, 0 replies; 10+ messages in thread
From: Stefan Beller @ 2015-05-01 16:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Alangi Derick, git@vger.kernel.org
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)
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-05-01 18:01 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-05-01 11:01 Patch that modifies git usage message Alangi Derick
2015-05-01 15:51 ` Stefan Beller
2015-05-01 15:54 ` Alangi Derick
2015-05-01 16:29 ` Junio C Hamano
2015-05-01 16:38 ` Alangi Derick
2015-05-01 16:55 ` Stefan Beller
2015-05-01 17:40 ` Eric Sunshine
2015-05-01 17:49 ` Alangi Derick
2015-05-01 18:01 ` Eric Sunshine
2015-05-01 16:55 ` Stefan Beller
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).