Re: [PATCH] remote: unify main and subcommand usage strings

2013-11-03 Thread Jiang Xin
2013/11/3 Thomas Rast t...@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.0 +0800
+++ after/git-remote-add-usage 2013-11-03 15:11:32.0 +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
 --tagsimport 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.0 +0800
+++ after/git-remote-prune-usage 2013-11-03 15:11:32.0 +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.0 +0800
+++ after/git-remote-set-branches-usage 2013-11-03 15:11:32.0 +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.0 +0800
+++ after/git-remote-show-usage 2013-11-03 15:11:32.0 +0800
@@ -1,4 +1,4 @@
-usage: git remote show [options] name
+usage: git remote [-v | --verbose] show [-n] name

 -ndo 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.0 +0800
+++ after/git-remote-update-usage 2013-11-03 15:11:32.0 +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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] remote: unify main and subcommand usage strings

2013-11-03 Thread Jiang Xin
2013/11/3 Jiang Xin worldhello@gmail.com:
 2013/11/3 Thomas Rast t...@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.0 +0800
 +++ after/git-remote-add-usage 2013-11-03 15:11:32.0 +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
  --tagsimport 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.0 +0800
 +++ after/git-remote-prune-usage 2013-11-03 15:11:32.0 +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.0 +0800
 +++ after/git-remote-set-branches-usage 2013-11-03 15:11:32.0 +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.0 +0800
 +++ after/git-remote-show-usage 2013-11-03 15:11:32.0 +0800
 @@ -1,4 +1,4 @@
 -usage: git remote show [options] name
 +usage: git remote [-v | --verbose] show [-n] name

  -ndo 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.0 +0800
 +++ after/git-remote-update-usage 2013-11-03 15:11:32.0 +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 21
git remote rename   $DIR/git-remote-rename-usage  21

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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] remote: unify main and subcommand usage strings

2013-11-02 Thread Thomas Rast
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 vnwild...@gmail.com
Signed-off-by: Thomas Rast t...@thomasrast.ch
---

Trần Ngọc Quân vnwild...@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...),
+