* [PATCH RFC] git remote: Separate usage strings for subcommands
@ 2009-11-15 21:43 Tim Henigan
2009-11-17 1:02 ` Junio C Hamano
0 siblings, 1 reply; 3+ messages in thread
From: Tim Henigan @ 2009-11-15 21:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
When the usage string for a subcommand must be printed,
only print the information relevant to that command.
Signed-off-by: Tim Henigan <tim.henigan@gmail.com>
---
This patch is based on:
http://article.gmane.org/gmane.comp.version-control.git/132953
All usage strings are still only located at the top of file. However,
separate usage
string arrays have been created for each subcommand.
Does this look like a sane way to structure the code?
If anyone else should be added to the CC list, please let me know.
builtin-remote.c | 57 +++++++++++++++++++++++++++++++++--------------------
1 files changed, 35 insertions(+), 22 deletions(-)
diff --git a/builtin-remote.c b/builtin-remote.c
index 0777dd7..ec65a4b 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c
@@ -7,18 +7,35 @@
#include "run-command.h"
#include "refs.h"
+#define REMOTE_BARE_USAGE "git remote [-v | --verbose]"
+#define REMOTE_ADD_USAGE "git remote add [-t <branch>] [-m <master>]
[-f] [--mirror] <name> <url>"
+#define REMOTE_RENAME_USAGE "git remote rename <old> <new>"
+#define REMOTE_RM_USAGE "git remote rm <name>"
+#define REMOTE_SETHEAD_USAGE "git remote set-head <name> [-a | -d | <branch>]"
+#define REMOTE_SHOW_USAGE "git remote show [-n] <name>"
+#define REMOTE_PRUNE_USAGE "git remote prune [-n | --dry-run] <name>"
+#define REMOTE_UPDATE_USAGE "git remote [-v | --verbose] update [-p |
--prune] [group]"
+
static const char * const builtin_remote_usage[] = {
- "git remote [-v | --verbose]",
- "git remote add [-t <branch>] [-m <master>] [-f] [--mirror] <name> <url>",
- "git remote rename <old> <new>",
- "git remote rm <name>",
- "git remote set-head <name> [-a | -d | <branch>]",
- "git remote show [-n] <name>",
- "git remote prune [-n | --dry-run] <name>",
- "git remote [-v | --verbose] update [-p | --prune] [group]",
+ REMOTE_BARE_USAGE,
+ REMOTE_ADD_USAGE,
+ REMOTE_RENAME_USAGE,
+ REMOTE_RM_USAGE,
+ REMOTE_SETHEAD_USAGE,
+ REMOTE_SHOW_USAGE,
+ REMOTE_PRUNE_USAGE,
+ REMOTE_UPDATE_USAGE,
NULL
};
+static const char * const builtin_remote_add_usage[] = {
REMOTE_ADD_USAGE, NULL };
+static const char * const builtin_remote_rename_usage[] = {
REMOTE_RENAME_USAGE, NULL };
+static const char * const builtin_remote_rm_usage[] = {
REMOTE_RM_USAGE, NULL };
+static const char * const builtin_remote_sethead_usage[] = {
REMOTE_SETHEAD_USAGE, NULL };
+static const char * const builtin_remote_show_usage[] = {
REMOTE_SHOW_USAGE, NULL };
+static const char * const builtin_remote_prune_usage[] = {
REMOTE_PRUNE_USAGE, NULL };
+static const char * const builtin_remote_update_usage[] = {
REMOTE_UPDATE_USAGE, NULL };
+
#define GET_REF_STATES (1<<0)
#define GET_HEAD_NAMES (1<<1)
#define GET_PUSH_REF_STATES (1<<2)
@@ -70,7 +87,6 @@ static int add(int argc, const char **argv)
int i;
struct option options[] = {
- OPT_GROUP("add specific options"),
OPT_BOOLEAN('f', "fetch", &fetch, "fetch the remote branches"),
OPT_CALLBACK('t', "track", &track, "branch",
"branch(es) to track", opt_parse_track),
@@ -79,11 +95,11 @@ static int add(int argc, const char **argv)
OPT_END()
};
- argc = parse_options(argc, argv, NULL, options, builtin_remote_usage,
+ argc = parse_options(argc, argv, NULL, options, builtin_remote_add_usage,
0);
if (argc < 2)
- usage_with_options(builtin_remote_usage, options);
+ usage_with_options(builtin_remote_add_usage, options);
name = argv[0];
url = argv[1];
@@ -540,7 +556,7 @@ static int mv(int argc, const char **argv)
int i;
if (argc != 3)
- usage_with_options(builtin_remote_usage, options);
+ usage_with_options(builtin_remote_rename_usage, options);
rename.old = argv[1];
rename.new = argv[2];
@@ -681,7 +697,7 @@ static int rm(int argc, const char **argv)
int i, result;
if (argc != 2)
- usage_with_options(builtin_remote_usage, options);
+ usage_with_options(builtin_remote_rm_usage, options);
remote = remote_get(argv[1]);
if (!remote)
@@ -984,7 +1000,7 @@ static int show(int argc, const char **argv)
struct string_list info_list = { NULL, 0, 0, 0 };
struct show_info info;
- argc = parse_options(argc, argv, NULL, options, builtin_remote_usage,
+ argc = parse_options(argc, argv, NULL, options, builtin_remote_show_usage,
0);
if (argc < 1)
@@ -1088,7 +1104,7 @@ static int set_head(int argc, const char **argv)
"delete refs/remotes/<name>/HEAD"),
OPT_END()
};
- argc = parse_options(argc, argv, NULL, options, builtin_remote_usage,
+ argc = parse_options(argc, argv, NULL, options, builtin_remote_sethead_usage,
0);
if (argc)
strbuf_addf(&buf, "refs/remotes/%s/HEAD", argv[0]);
@@ -1114,7 +1130,7 @@ static int set_head(int argc, const char **argv)
if (delete_ref(buf.buf, NULL, REF_NODEREF))
result |= error("Could not delete %s", buf.buf);
} else
- usage_with_options(builtin_remote_usage, options);
+ usage_with_options(builtin_remote_sethead_usage, options);
if (head_name) {
unsigned char sha1[20];
@@ -1138,16 +1154,15 @@ static int prune(int argc, const char **argv)
{
int dry_run = 0, result = 0;
struct option options[] = {
- OPT_GROUP("prune specific options"),
OPT__DRY_RUN(&dry_run),
OPT_END()
};
- argc = parse_options(argc, argv, NULL, options, builtin_remote_usage,
+ argc = parse_options(argc, argv, NULL, options, builtin_remote_prune_usage,
0);
if (argc < 1)
- usage_with_options(builtin_remote_usage, options);
+ usage_with_options(builtin_remote_prune_usage, options);
for (; argc; argc--, argv++)
result |= prune_remote(*argv, dry_run);
@@ -1228,13 +1243,12 @@ static int update(int argc, const char **argv)
struct string_list list = { NULL, 0, 0, 0 };
static const char *default_argv[] = { NULL, "default", NULL };
struct option options[] = {
- OPT_GROUP("update specific options"),
OPT_BOOLEAN('p', "prune", &prune,
"prune remotes after fetching"),
OPT_END()
};
- argc = parse_options(argc, argv, NULL, options, builtin_remote_usage,
+ argc = parse_options(argc, argv, NULL, options, builtin_remote_update_usage,
PARSE_OPT_KEEP_ARGV0);
if (argc < 2) {
argc = 2;
@@ -1334,7 +1348,6 @@ static int show_all(void)
int cmd_remote(int argc, const char **argv, const char *prefix)
{
struct option options[] = {
- OPT__VERBOSE(&verbose),
OPT_END()
};
int result;
--
1.6.5.2.185.gb7fba.dirty
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH RFC] git remote: Separate usage strings for subcommands
2009-11-15 21:43 [PATCH RFC] git remote: Separate usage strings for subcommands Tim Henigan
@ 2009-11-17 1:02 ` Junio C Hamano
2009-11-17 1:39 ` Tim Henigan
0 siblings, 1 reply; 3+ messages in thread
From: Junio C Hamano @ 2009-11-17 1:02 UTC (permalink / raw)
To: Tim Henigan; +Cc: git
Tim Henigan <tim.henigan@gmail.com> writes:
> All usage strings are still only located at the top of file. However,
> separate usage string arrays have been created for each subcommand.
>
> Does this look like a sane way to structure the code?
>
> If anyone else should be added to the CC list, please let me know.
>
>
> builtin-remote.c | 57 +++++++++++++++++++++++++++++++++--------------------
> 1 files changed, 35 insertions(+), 22 deletions(-)
>
> diff --git a/builtin-remote.c b/builtin-remote.c
> index 0777dd7..ec65a4b 100644
> --- a/builtin-remote.c
> +++ b/builtin-remote.c
> @@ -7,18 +7,35 @@
> #include "run-command.h"
> #include "refs.h"
>
> +#define REMOTE_BARE_USAGE "git remote [-v | --verbose]"
> +#define REMOTE_ADD_USAGE "git remote add [-t <branch>] [-m <master>]
> [-f] [--mirror] <name> <url>"
> +#define REMOTE_RENAME_USAGE "git remote rename <old> <new>"
> +#define REMOTE_RM_USAGE "git remote rm <name>"
> +#define REMOTE_SETHEAD_USAGE "git remote set-head <name> [-a | -d | <branch>]"
> +#define REMOTE_SHOW_USAGE "git remote show [-n] <name>"
> +#define REMOTE_PRUNE_USAGE "git remote prune [-n | --dry-run] <name>"
> +#define REMOTE_UPDATE_USAGE "git remote [-v | --verbose] update [-p |
> --prune] [group]"
> +
> static const char * const builtin_remote_usage[] = {
> - "git remote [-v | --verbose]",
> - "git remote add [-t <branch>] [-m <master>] [-f] [--mirror] <name> <url>",
> - "git remote rename <old> <new>",
> - "git remote rm <name>",
> - "git remote set-head <name> [-a | -d | <branch>]",
> - "git remote show [-n] <name>",
> - "git remote prune [-n | --dry-run] <name>",
> - "git remote [-v | --verbose] update [-p | --prune] [group]",
> + REMOTE_BARE_USAGE,
> + REMOTE_ADD_USAGE,
> + REMOTE_RENAME_USAGE,
> + REMOTE_RM_USAGE,
> + REMOTE_SETHEAD_USAGE,
> + REMOTE_SHOW_USAGE,
> + REMOTE_PRUNE_USAGE,
> + REMOTE_UPDATE_USAGE,
> NULL
> };
> ...
> +static const char * const builtin_remote_add_usage[] = {
> REMOTE_ADD_USAGE, NULL };
> ...
I am not sure about the value of reusing option string like this, and for
all other subcommands the same comment applies. For example, in the case
of "remote add -h", you would use
"git remote add [-t <branch>] [-m <master>] [-f] [--mirror] <name> <url>"
from REMOTE_ADD_USAGE, but ...
> @@ -70,7 +87,6 @@ static int add(int argc, const char **argv)
> int i;
>
> struct option options[] = {
> - OPT_GROUP("add specific options"),
> OPT_BOOLEAN('f', "fetch", &fetch, "fetch the remote branches"),
> OPT_CALLBACK('t', "track", &track, "branch",
> "branch(es) to track", opt_parse_track),
> @@ -79,11 +95,11 @@ static int add(int argc, const char **argv)
> OPT_END()
> };
>
> - argc = parse_options(argc, argv, NULL, options, builtin_remote_usage,
> + argc = parse_options(argc, argv, NULL, options, builtin_remote_add_usage,
> 0);
... the options list is used to reproduce the information in a major part
of that string already. So I would prefer builtin_remote_add_usage[] to
be something like:
"git remote add [<options>...] <name> <url>"
It is in line with a discussion we had earlier:
http://thread.gmane.org/gmane.comp.version-control.git/129906/focus=130646
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH RFC] git remote: Separate usage strings for subcommands
2009-11-17 1:02 ` Junio C Hamano
@ 2009-11-17 1:39 ` Tim Henigan
0 siblings, 0 replies; 3+ messages in thread
From: Tim Henigan @ 2009-11-17 1:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
On Mon, Nov 16, 2009 at 8:02 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Tim Henigan <tim.henigan@gmail.com> writes:
>
>> ...
>> +static const char * const builtin_remote_add_usage[] = {
>> REMOTE_ADD_USAGE, NULL };
>> ...
>
> I am not sure about the value of reusing option string like this, and for
> all other subcommands the same comment applies. For example, in the case
> of "remote add -h", you would use
>
> "git remote add [-t <branch>] [-m <master>] [-f] [--mirror] <name> <url>"
> from REMOTE_ADD_USAGE, but ...
Do you object to the new #defines for the strings? I added them since we now
really need to construct the usage string for 2 separate uses:
(1) When 'git remote -h' is invoked, we need to return the usage
string showing
all subcommands.
(2) When 'git remote <subcommand> -h' is invoked, we need to return the
usage for just that command.
It doesn't make sense to me to maintain separate strings for the two use cases.
>> @@ -70,7 +87,6 @@ static int add(int argc, const char **argv)
>> int i;
>>
>> struct option options[] = {
>> - OPT_GROUP("add specific options"),
>> OPT_BOOLEAN('f', "fetch", &fetch, "fetch the remote branches"),
>> OPT_CALLBACK('t', "track", &track, "branch",
>> "branch(es) to track", opt_parse_track),
>> @@ -79,11 +95,11 @@ static int add(int argc, const char **argv)
>> OPT_END()
>> };
>>
>> - argc = parse_options(argc, argv, NULL, options, builtin_remote_usage,
>> + argc = parse_options(argc, argv, NULL, options, builtin_remote_add_usage,
>> 0);
>
> ... the options list is used to reproduce the information in a major part
> of that string already. So I would prefer builtin_remote_add_usage[] to
> be something like:
>
> "git remote add [<options>...] <name> <url>"
>
> It is in line with a discussion we had earlier:
>
> http://thread.gmane.org/gmane.comp.version-control.git/129906/focus=130646
I planned to make this change in a separate (or maybe just a later
version) patch,
but first I wanted to be sure my other changes were sound. I will send a new
version that includes this change.
Should this change be made to the man page as well?
>> @@ -1334,7 +1348,6 @@ static int show_all(void)
>> int cmd_remote(int argc, const char **argv, const char *prefix)
>> {
>> struct option options[] = {
>> - OPT__VERBOSE(&verbose),
>> OPT_END()
>> };
>> int result;
I did find one problem with the above portion of the patch. With this change
'-v' was no longer recognized as an option. I will remove this change in the
next version. Keeping it means that '-v' will still be reported in the
'git remote -h' usage string, but I don't see an easy way to remove the string
without removing the feature.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2009-11-17 1:39 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-11-15 21:43 [PATCH RFC] git remote: Separate usage strings for subcommands Tim Henigan
2009-11-17 1:02 ` Junio C Hamano
2009-11-17 1:39 ` Tim Henigan
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).