git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [L10N] Kick off for Git 1.8.5 l10n round 1
       [not found]   ` <CANYiYbGmiBK23=A=MbkfSWwe8X8q3vYSvYPZXhKTLUsbLbMDsw@mail.gmail.com>
@ 2013-11-02  2:41     ` Trần Ngọc Quân
  2013-11-02 16:11       ` [PATCH] remote: unify main and subcommand usage strings Thomas Rast
  0 siblings, 1 reply; 4+ messages in thread
From: Trần Ngọc Quân @ 2013-11-02  2:41 UTC (permalink / raw)
  To: Jiang Xin; +Cc: Junio C Hamano, git-malling-list

On 02/11/2013 09:23, Jiang Xin wrote:
> 2013/11/2 Trần Ngọc Quân <vnwildman@gmail.com>:
>> Strings in builtin/remote.c line 15 and 42 is similar, please change to
>> same string in order to reduce gettext database (.mo file)
>> --
>>  Trần Ngọc Quân.
> Confirmed, there is a typo in builtin/remote.c line 15. Have you send
> patch to this list for this, Trần?
>
This is minor error, so let Junio C Hamano do it!

-- 
Trần Ngọc Quân.

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

* [PATCH] remote: unify main and subcommand usage strings
  2013-11-02  2:41     ` [L10N] Kick off for Git 1.8.5 l10n round 1 Trần Ngọc Quân
@ 2013-11-02 16:11       ` Thomas Rast
  2013-11-03  7:17         ` Jiang Xin
  0 siblings, 1 reply; 4+ messages in thread
From: Thomas Rast @ 2013-11-02 16:11 UTC (permalink / raw)
  To: Trần Ngọc Quân; +Cc: git, Junio C Hamano, Jiang Xin

We had separate usages for each subcommand, and for the main command,
even though the latter is essentially a concatenation of all of the
former.  This leads to a lot of duplication and unnecessary
differences, e.g., in the 'set-head' case the two strings differ only
in a space.

Unify the strings in the usages by putting each of them in a variable,
and assembling the usage arrays from them.

Note that this patch changes the usage strings for the following
subcommands:

- prune and show: the individual usage only said [<options>].  Kept
  the snippet from the main usage, which is more specific.

- set-branches: kept the main usage, which is more concise in saying
  that --add is optional

Reported-by: Trần Ngọc Quân <vnwildman@gmail.com>
Signed-off-by: Thomas Rast <tr@thomasrast.ch>
---

Trần Ngọc Quân <vnwildman@gmail.com> wrote:
> On 02/11/2013 09:23, Jiang Xin wrote:
> > Confirmed, there is a typo in builtin/remote.c line 15. Have you send
> > patch to this list for this, Trần?
> >
> This is minor error, so let Junio C Hamano do it!

Dunno, this generally isn't the nicest way to get things done, nor the
most productive use of maintainer bandwidth.

How about patching it like this instead?  That should prevent similar
issues from cropping up again.


 builtin/remote.c | 70 +++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 47 insertions(+), 23 deletions(-)

diff --git a/builtin/remote.c b/builtin/remote.c
index 4e14891..2f6366a 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -7,67 +7,91 @@
 #include "run-command.h"
 #include "refs.h"
 
+static const char builtin_remote_add_usage_str[] =
+	N_("git remote add [-t <branch>] [-m <master>] [-f] [--tags|--no-tags] "
+	   "[--mirror=<fetch|push>] <name> <url>");
+static const char builtin_remote_rename_usage_str[] =
+	N_("git remote rename <old> <new>");
+static const char builtin_remote_rm_usage_str[] =
+	N_("git remote remove <name>");
+static const char builtin_remote_sethead_usage_str[] =
+	N_("git remote set-head <name> (-a | --auto | -d | --delete | <branch>)");
+static const char builtin_remote_setbranches_usage_str[] =
+	N_("git remote set-branches [--add] <name> <branch>...");
+static const char builtin_remote_show_usage_str[] =
+	N_("git remote [-v | --verbose] show [-n] <name>");
+static const char builtin_remote_prune_usage_str[] =
+	N_("git remote prune [-n | --dry-run] <name>");
+static const char builtin_remote_update_usage_str[] =
+	N_("git remote [-v | --verbose] update [-p | --prune] "
+	   "[(<group> | <remote>)...]");
+static const char builtin_remote_seturl_usage_str[] =
+	N_("git remote set-url [--push] <name> <newurl> [<oldurl>]");
+static const char builtin_remote_seturl_add_usage_str[] =
+	N_("git remote set-url --add <name> <newurl>");
+static const char builtin_remote_seturl_delete_usage_str[] =
+	N_("git remote set-url --delete <name> <url>");
+
 static const char * const builtin_remote_usage[] = {
 	N_("git remote [-v | --verbose]"),
-	N_("git remote add [-t <branch>] [-m <master>] [-f] [--tags|--no-tags] [--mirror=<fetch|push>] <name> <url>"),
-	N_("git remote rename <old> <new>"),
-	N_("git remote remove <name>"),
-	N_("git remote set-head <name> (-a | --auto | -d | --delete |<branch>)"),
-	N_("git remote [-v | --verbose] show [-n] <name>"),
-	N_("git remote prune [-n | --dry-run] <name>"),
-	N_("git remote [-v | --verbose] update [-p | --prune] [(<group> | <remote>)...]"),
-	N_("git remote set-branches [--add] <name> <branch>..."),
-	N_("git remote set-url [--push] <name> <newurl> [<oldurl>]"),
-	N_("git remote set-url --add <name> <newurl>"),
-	N_("git remote set-url --delete <name> <url>"),
+	builtin_remote_add_usage_str,
+	builtin_remote_rename_usage_str,
+	builtin_remote_rm_usage_str,
+	builtin_remote_sethead_usage_str,
+	builtin_remote_show_usage_str,
+	builtin_remote_prune_usage_str,
+	builtin_remote_update_usage_str,
+	builtin_remote_setbranches_usage_str,
+	builtin_remote_seturl_usage_str,
+	builtin_remote_seturl_add_usage_str,
+	builtin_remote_seturl_delete_usage_str,
 	NULL
 };
 
 static const char * const builtin_remote_add_usage[] = {
-	N_("git remote add [<options>] <name> <url>"),
+	builtin_remote_add_usage_str,
 	NULL
 };
 
 static const char * const builtin_remote_rename_usage[] = {
-	N_("git remote rename <old> <new>"),
+	builtin_remote_rename_usage_str,
 	NULL
 };
 
 static const char * const builtin_remote_rm_usage[] = {
-	N_("git remote remove <name>"),
+	builtin_remote_rm_usage_str,
 	NULL
 };
 
 static const char * const builtin_remote_sethead_usage[] = {
-	N_("git remote set-head <name> (-a | --auto | -d | --delete | <branch>)"),
+	builtin_remote_sethead_usage_str,
 	NULL
 };
 
 static const char * const builtin_remote_setbranches_usage[] = {
-	N_("git remote set-branches <name> <branch>..."),
-	N_("git remote set-branches --add <name> <branch>..."),
+	builtin_remote_setbranches_usage_str,
 	NULL
 };
 
 static const char * const builtin_remote_show_usage[] = {
-	N_("git remote show [<options>] <name>"),
+	builtin_remote_show_usage_str,
 	NULL
 };
 
 static const char * const builtin_remote_prune_usage[] = {
-	N_("git remote prune [<options>] <name>"),
+	builtin_remote_prune_usage_str,
 	NULL
 };
 
 static const char * const builtin_remote_update_usage[] = {
-	N_("git remote update [<options>] [<group> | <remote>]..."),
+	builtin_remote_update_usage_str,
 	NULL
 };
 
 static const char * const builtin_remote_seturl_usage[] = {
-	N_("git remote set-url [--push] <name> <newurl> [<oldurl>]"),
-	N_("git remote set-url --add <name> <newurl>"),
-	N_("git remote set-url --delete <name> <url>"),
+	builtin_remote_seturl_usage_str,
+	builtin_remote_seturl_add_usage_str,
+	builtin_remote_seturl_delete_usage_str,
 	NULL
 };
 
-- 
1.8.4.2.838.g4c8c068

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

* Re: [PATCH] remote: unify main and subcommand usage strings
  2013-11-02 16:11       ` [PATCH] remote: unify main and subcommand usage strings Thomas Rast
@ 2013-11-03  7:17         ` Jiang Xin
  2013-11-03  8:03           ` Jiang Xin
  0 siblings, 1 reply; 4+ messages in thread
From: Jiang Xin @ 2013-11-03  7:17 UTC (permalink / raw)
  To: Thomas Rast; +Cc: Trần Ngọc Quân, Git List, Junio C Hamano

2013/11/3 Thomas Rast <tr@thomasrast.ch>:
> We had separate usages for each subcommand, and for the main command,
> even though the latter is essentially a concatenation of all of the
> former.  This leads to a lot of duplication and unnecessary
> differences, e.g., in the 'set-head' case the two strings differ only
> in a space.
>
> Unify the strings in the usages by putting each of them in a variable,
> and assembling the usage arrays from them.
>
> Note that this patch changes the usage strings for the following
> subcommands:
>
> - prune and show: the individual usage only said [<options>].  Kept
>   the snippet from the main usage, which is more specific.
>
> - set-branches: kept the main usage, which is more concise in saying
>   that --add is optional
>

Differences of git-remote usages after applied your patch.

diff -u before/git-remote-add-usage after/git-remote-add-usage
--- before/git-remote-add-usage 2013-11-03 15:10:06.000000000 +0800
+++ after/git-remote-add-usage 2013-11-03 15:11:32.000000000 +0800
@@ -1,4 +1,4 @@
-usage: git remote add [<options>] <name> <url>
+usage: git remote add [-t <branch>] [-m <master>] [-f]
[--tags|--no-tags] [--mirror=<fetch|push>] <name> <url>

     -f, --fetch           fetch the remote branches
     --tags                import all tags and associated objects when fetching

diff -u before/git-remote-prune-usage after/git-remote-prune-usage
--- before/git-remote-prune-usage 2013-11-03 15:10:06.000000000 +0800
+++ after/git-remote-prune-usage 2013-11-03 15:11:32.000000000 +0800
@@ -1,4 +1,4 @@
-usage: git remote prune [<options>] <name>
+usage: git remote prune [-n | --dry-run] <name>

     -n, --dry-run         dry run

diff -u before/git-remote-set-branches-usage after/git-remote-set-branches-usage
--- before/git-remote-set-branches-usage 2013-11-03 15:10:06.000000000 +0800
+++ after/git-remote-set-branches-usage 2013-11-03 15:11:32.000000000 +0800
@@ -1,5 +1,4 @@
-usage: git remote set-branches <name> <branch>...
-   or: git remote set-branches --add <name> <branch>...
+usage: git remote set-branches [--add] <name> <branch>...

     --add                 add branch

diff -u before/git-remote-show-usage after/git-remote-show-usage
--- before/git-remote-show-usage 2013-11-03 15:10:06.000000000 +0800
+++ after/git-remote-show-usage 2013-11-03 15:11:32.000000000 +0800
@@ -1,4 +1,4 @@
-usage: git remote show [<options>] <name>
+usage: git remote [-v | --verbose] show [-n] <name>

     -n                    do not query remotes

diff -u before/git-remote-update-usage after/git-remote-update-usage
--- before/git-remote-update-usage 2013-11-03 15:10:06.000000000 +0800
+++ after/git-remote-update-usage 2013-11-03 15:11:32.000000000 +0800
@@ -1,4 +1,4 @@
-usage: git remote update [<options>] [<group> | <remote>]...
+usage: git remote [-v | --verbose] update [-p | --prune] [(<group> |
<remote>)...]

     -p, --prune           prune remotes after fetching




-- 
Jiang Xin

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

* Re: [PATCH] remote: unify main and subcommand usage strings
  2013-11-03  7:17         ` Jiang Xin
@ 2013-11-03  8:03           ` Jiang Xin
  0 siblings, 0 replies; 4+ messages in thread
From: Jiang Xin @ 2013-11-03  8:03 UTC (permalink / raw)
  To: Thomas Rast; +Cc: Trần Ngọc Quân, Git List, Junio C Hamano

2013/11/3 Jiang Xin <worldhello.net@gmail.com>:
> 2013/11/3 Thomas Rast <tr@thomasrast.ch>:
>> Note that this patch changes the usage strings for the following
>> subcommands:
>
> Differences of git-remote usages after applied your patch.
>
> diff -u before/git-remote-add-usage after/git-remote-add-usage
> --- before/git-remote-add-usage 2013-11-03 15:10:06.000000000 +0800
> +++ after/git-remote-add-usage 2013-11-03 15:11:32.000000000 +0800
> @@ -1,4 +1,4 @@
> -usage: git remote add [<options>] <name> <url>
> +usage: git remote add [-t <branch>] [-m <master>] [-f]
> [--tags|--no-tags] [--mirror=<fetch|push>] <name> <url>
>
>      -f, --fetch           fetch the remote branches
>      --tags                import all tags and associated objects when fetching
>
> diff -u before/git-remote-prune-usage after/git-remote-prune-usage
> --- before/git-remote-prune-usage 2013-11-03 15:10:06.000000000 +0800
> +++ after/git-remote-prune-usage 2013-11-03 15:11:32.000000000 +0800
> @@ -1,4 +1,4 @@
> -usage: git remote prune [<options>] <name>
> +usage: git remote prune [-n | --dry-run] <name>
>
>      -n, --dry-run         dry run
>
> diff -u before/git-remote-set-branches-usage after/git-remote-set-branches-usage
> --- before/git-remote-set-branches-usage 2013-11-03 15:10:06.000000000 +0800
> +++ after/git-remote-set-branches-usage 2013-11-03 15:11:32.000000000 +0800
> @@ -1,5 +1,4 @@
> -usage: git remote set-branches <name> <branch>...
> -   or: git remote set-branches --add <name> <branch>...
> +usage: git remote set-branches [--add] <name> <branch>...
>
>      --add                 add branch
>
> diff -u before/git-remote-show-usage after/git-remote-show-usage
> --- before/git-remote-show-usage 2013-11-03 15:10:06.000000000 +0800
> +++ after/git-remote-show-usage 2013-11-03 15:11:32.000000000 +0800
> @@ -1,4 +1,4 @@
> -usage: git remote show [<options>] <name>
> +usage: git remote [-v | --verbose] show [-n] <name>
>
>      -n                    do not query remotes
>
> diff -u before/git-remote-update-usage after/git-remote-update-usage
> --- before/git-remote-update-usage 2013-11-03 15:10:06.000000000 +0800
> +++ after/git-remote-update-usage 2013-11-03 15:11:32.000000000 +0800
> @@ -1,4 +1,4 @@
> -usage: git remote update [<options>] [<group> | <remote>]...
> +usage: git remote [-v | --verbose] update [-p | --prune] [(<group> |
> <remote>)...]
>
>      -p, --prune           prune remotes after fetching
>

In order to get the differences of git-remote usages, I write a script.

    # SCRIPT to save git remote usage in files.
    for cmd in add set-head show prune update set-branches set-url; do
            git remote $cmd -h > $DIR/git-remote-$cmd-usage
    done
    git remote -h > $DIR/git-remote-usage
    git remote remove  > $DIR/git-remote-remove-usage 2>&1
    git remote rename  > $DIR/git-remote-rename-usage  2>&1

Then I find two subcommands (remove and rename) are strange.

 * All other subcommands output usages to STDIN, but subcommands
    "remove" and "rename" send their usages to STDERR.

 * I can get the help message of all other subcommands using:
   "git remote <subcmd> -h", but not "git remote rm -h"

        $ git remote rm -h
        error: Could not remove config section 'remote.-h'

Later I know it's a side-effect that all other subcommands could
print usages if provide a unkown "-h/--help" option.

What if add a parse_options call for both "rm" and "mv" functions
in builtin/remote.c?

diff --git a/builtin/remote.c b/builtin/remote.c
index 2f6366a..171d1a8 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -651,6 +651,8 @@ static int mv(int argc, const char **argv)
        struct rename_info rename;
        int i, refspec_updated = 0;

+       argc = parse_options(argc, argv, NULL, options,
+                            builtin_remote_rename_usage, 0);
        if (argc != 3)
                usage_with_options(builtin_remote_rename_usage, options);

@@ -808,6 +810,8 @@ static int rm(int argc, const char **argv)
        cb_data.skipped = &skipped;
        cb_data.keep = &known_remotes;

+       argc = parse_options(argc, argv, NULL, options,
+                            builtin_remote_rm_usage,0);
        if (argc != 2)
                usage_with_options(builtin_remote_rm_usage, options);


-- 
Jiang Xin

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

end of thread, other threads:[~2013-11-03  8:03 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <CANYiYbFoU3oSvmXvHfZj3js==4CxsU-0UZs_WU+VvrCNVGYRKg@mail.gmail.com>
     [not found] ` <527457A0.9010600@gmail.com>
     [not found]   ` <CANYiYbGmiBK23=A=MbkfSWwe8X8q3vYSvYPZXhKTLUsbLbMDsw@mail.gmail.com>
2013-11-02  2:41     ` [L10N] Kick off for Git 1.8.5 l10n round 1 Trần Ngọc Quân
2013-11-02 16:11       ` [PATCH] remote: unify main and subcommand usage strings Thomas Rast
2013-11-03  7:17         ` Jiang Xin
2013-11-03  8:03           ` Jiang Xin

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