help: use parseopt
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH 1/3 help: use parseopt
This patch converts cmd_help to use parseopt, along with a few style cleanups, including:
- enum constants are now ALL_CAPS
- parse_help_format returns an enum value rather than setting a global as a side effect
---
Sorry, the diff turned out quite messy because of the cleanups. It's probably easier to just read the result.
help.c | 123 ++++++++++++++++++++++++++++++--------------------------------- 1 files changed, 59 insertions(+), 64 deletions(-)
help.c +59 -64
@@ -7,40 +7,49 @@#include "builtin.h" #include "exec_cmd.h" #include "common-cmds.h" - -static const char *help_default_format; - -static enum help_format { - man_format, - info_format, - web_format, -} help_format = man_format; - -static void parse_help_format (const char *format) +#include "parse-options.h" + +enum help_format { +HELP_FORMAT_MAN , +HELP_FORMAT_INFO , +HELP_FORMAT_WEB , +}; + +static int show_all =0 ; +static enum help_format help_format =HELP_FORMAT_MAN ; +static struct option builtin_help_options[] = { +OPT_BOOLEAN ('a' ,"all" , &show_all,"print all available commands" ), +OPT_SET_INT ('m' ,"man" , &help_format,"show man page" ,HELP_FORMAT_MAN ), +OPT_SET_INT ('w' ,"web" , &help_format,"show manual in web browser" , +HELP_FORMAT_WEB ), +OPT_SET_INT ('i' ,"info" , &help_format,"show info page" , +HELP_FORMAT_INFO ), +}; + +static const char *const builtin_help_usage[] = { +"git-help [--all] [--man|--web|--info] [command]" , +NULL +}; + +static enum help_format parse_help_format (const char *format) { -if (!format) { - help_format = man_format; -return ; - } -if (!strcmp (format,"man" )) { - help_format = man_format; -return ; - } -if (!strcmp (format,"info" )) { - help_format = info_format; -return ; - } -if (!strcmp (format,"web" ) || !strcmp (format,"html" )) { - help_format = web_format; -return ; - } +if (!strcmp (format,"man" )) +return HELP_FORMAT_MAN ; +if (!strcmp (format,"info" )) +return HELP_FORMAT_INFO ; +if (!strcmp (format,"web" ) || !strcmp (format,"html" )) +return HELP_FORMAT_WEB ;die ("unrecognized help format '%s'" , format); }static int git_help_config (const char *var,const char *value) { -if (!strcmp (var,"help.format" )) -return git_config_string (&help_default_format, var, value); +if (!strcmp (var,"help.format" )) { +if (!value) +return config_error_nonbool (var); + help_format =parse_help_format (value); +return 0 ; + }return git_default_config (var, value); }
@@ -362,50 +371,36 @@ int cmd_version(int argc, const char **argv, const char *prefix)int cmd_help (int argc,const char **argv,const char *prefix) { -const char *help_cmd = argv[1 ]; +int nongit; -if (argc <2 ) { -printf ("usage: %s\n\n" , git_usage_string); -list_common_cmds_help (); -exit (0 ); - } +setup_git_directory_gently (&nongit); +git_config (git_help_config); -if (!strcmp (help_cmd,"--all" ) || !strcmp (help_cmd,"-a" )) { + argc =parse_options (argc, argv, builtin_help_options, + builtin_help_usage,0 ); + +if (show_all) {printf ("usage: %s\n\n" , git_usage_string);list_commands (); +return 0 ; } -else if (!strcmp (help_cmd,"--web" ) || !strcmp (help_cmd,"-w" )) { -show_html_page (argc >2 ? argv[2 ] :NULL ); - } - -else if (!strcmp (help_cmd,"--info" ) || !strcmp (help_cmd,"-i" )) { -show_info_page (argc >2 ? argv[2 ] :NULL ); - } - -else if (!strcmp (help_cmd,"--man" ) || !strcmp (help_cmd,"-m" )) { -show_man_page (argc >2 ? argv[2 ] :NULL ); +if (!argv[0 ]) { +printf ("usage: %s\n\n" , git_usage_string); +list_common_cmds_help (); +return 0 ; } -else { -int nongit; - -setup_git_directory_gently (&nongit); -git_config (git_help_config); -if (help_default_format) -parse_help_format (help_default_format); - -switch (help_format) { -case man_format: -show_man_page (help_cmd); -break ; -case info_format: -show_info_page (help_cmd); -break ; -case web_format: -show_html_page (help_cmd); -break ; - } +switch (help_format) { +case HELP_FORMAT_MAN : +show_man_page (argv[0 ]); +break ; +case HELP_FORMAT_INFO : +show_info_page (argv[0 ]); +break ; +case HELP_FORMAT_WEB : +show_html_page (argv[0 ]); +break ; }return 0 ;
Signature · 1 line
1.5.4.3.305.g073a4
Quoted 4 lines · Jeff King
This patch converts
cmd_helpto use parseopt, along with a few style cleanups, including:- enum constants are now
ALL_CAPS
Ok.
Quoted 2 lines
-
parse_help_formatreturns an enum value rather than setting a global as a side effect
Nice cleanup.
Quoted 4 lines
Signed-off-by Jeff King <[email protected]>---
Sorry, the diff turned out quite messy because of the cleanups. It's probably easier to just read the result.
Or could you send the cleanups in some separated patches ?
Thanks in advance,
Christian.
Quoted 4 lines · Christian Couder
Quoted 2 lines
Sorry, the diff turned out quite messy because of the cleanups. It's probably easier to just read the result.
Or could you send the cleanups in some separated patches ?
Looking at it again, it's actually not too bad except for the cmd_help() function itself, which isn't due to the cleanups at all, but because the flow changes quite a bit. So I don't think splitting it actually will make the diff any friendlier.
-Peff
Quoted 11 lines · Jeff King
Quoted 4 lines · Christian Couder
Quoted 2 lines
Sorry, the diff turned out quite messy because of the cleanups. It's probably easier to just read the result.
Or could you send the cleanups in some separated patches ?
Looking at it again, it's actually not too bad except for the
cmd_help()function itself, which isn't due to the cleanups at all, but because the flow changes quite a bit. So I don't think splitting it actually will make the diff any friendlier.
Your patch is fine as-is.
PATCH 2/3 make alias lookup a public, procedural function
In reply to a message not in the archiveThis converts git_config_alias to the public alias_lookup function. Because of the nature of our config parser, we still have to rely on setting static data. However, that interface is wrapped so that you can just say
value = alias_lookup(key);
--- Makefile | 3 ++- alias.c | 22 ++++++++++++++++++++++ cache.h | 2 ++ git.c | 17 +++-------------- 4 files changed, 29 insertions(+), 15 deletions(-) create mode 100644 alias.c
Makefile +2 -1
@@ -327,7 +327,8 @@ LIB_OBJS = \ alloc.o merge-file.o path-list.o help.o unpack-trees.o $(DIFF_OBJS) \ color.o wt-status.o archive-zip.o archive-tar.o shallow.o utf8.o \ convert.o attr.o decorate.o progress.o mailmap.o symlinks.o remote.o \ - transport.o bundle.o walker.o parse-options.o ws.o archive.o + transport.o bundle.o walker.o parse-options.o ws.o archive.o \ + alias.o BUILTIN_OBJS = \ builtin-add.o \
alias.c new file +22 -0
@@ -0,0 +1,22 @@ +#include "cache.h" + +static const char *alias_key; +static char *alias_val; +static int alias_lookup_cb (const char *k,const char *v) +{ +if (!prefixcmp (k,"alias." ) && !strcmp (k+6 , alias_key)) { +if (!v) +return config_error_nonbool (k); + alias_val =xstrdup (v); +return 0 ; + } +return 0 ; +} + +char *alias_lookup (const char *alias) +{ + alias_key = alias; + alias_val =NULL ; +git_config (alias_lookup_cb); +return alias_val; +}
cache.h +2 -0
@@ -765,4 +765,6 @@ int pathspec_match(const char **spec, char *matched, const char *filename, int sint report_path_error (const char *ps_matched,const char **pathspec,int prefix_offset);void overlay_tree_on_cache (const char *tree_name,const char *prefix); +char *alias_lookup (const char *alias); +#endif /* CACHE_H */
git.c +3 -14
@@ -87,19 +87,6 @@ static int handle_options(const char*** argv, int* argc, int* envchanged)return handled; } -static const char *alias_command; -static char *alias_string; - -static int git_alias_config (const char *var,const char *value) -{ -if (!prefixcmp (var,"alias." ) && !strcmp (var +6 , alias_command)) { -if (!value) -return config_error_nonbool (var); - alias_string =xstrdup (value); - } -return 0 ; -} -static int split_cmdline (char *cmdline,const char ***argv) {int src, dst, count =0 , size =16 ;
@@ -159,11 +146,13 @@ static int handle_alias(int *argcp, const char ***argv)const char *subdir;int count, option_count;const char ** new_argv; +const char *alias_command; +char *alias_string; subdir =setup_git_directory_gently (&nongit); alias_command = (*argv)[0 ]; -git_config (git_alias_config); + alias_string =alias_lookup (alias_command);if (alias_string) {if (alias_string[0 ] =='!' ) {if (*argcp >1 ) {
Signature · 1 line
1.5.4.3.305.g073a4
PATCH 3/3 help: respect aliases
In reply to a message not in the archiveIf we have an alias "foo" defined, then the help text for "foo" (via "git help foo" or "git foo --help") now shows the definition of the alias.
Before showing an alias definition, we make sure that there is no git command which would override the alias (so that even though you may have a "log" alias, even though it will not work, we don't want to it supersede "git help log").
--- help.c | 33 ++++++++++++++++++++++++++++++++- 1 files changed, 32 insertions(+), 1 deletions(-)
help.c +32 -1
@@ -210,7 +210,7 @@ static unsigned int list_commands_in_dir(struct cmdnames *cmds,return longest; } -static void list_commands (void ) +static unsigned int load_command_list (void ) {unsigned int longest =0 ;unsigned int len;
@@ -250,6 +250,14 @@ static void list_commands(void)uniq (&other_cmds);exclude_cmds (&other_cmds, &main_cmds); +return longest; +} + +static void list_commands (void ) +{ +unsigned int longest =load_command_list (); +const char *exec_path =git_exec_path (); +if (main_cmds.cnt) {printf ("available git commands in '%s'\n" , exec_path);printf ("----------------------------" );
@@ -284,6 +292,22 @@ void list_common_cmds_help(void) } } +static int is_in_cmdlist (struct cmdnames *c,const char *s) +{ +int i; +for (i =0 ; i < c->cnt; i++) +if (!strcmp (s, c->names[i]->name)) +return 1 ; +return 0 ; +} + +static int is_git_command (const char *s) +{ +load_command_list (); +return is_in_cmdlist (&main_cmds, s) || +is_in_cmdlist (&other_cmds, s); +} +static const char *cmd_to_page (const char *git_cmd) {if (!git_cmd)
@@ -372,6 +396,7 @@ int cmd_version(int argc, const char **argv, const char *prefix)int cmd_help (int argc,const char **argv,const char *prefix) {int nongit; +const char *alias;setup_git_directory_gently (&nongit);git_config (git_help_config);
@@ -391,6 +416,12 @@ int cmd_help(int argc, const char **argv, const char *prefix)return 0 ; } + alias =alias_lookup (argv[0 ]); +if (alias && !is_git_command (argv[0 ])) { +printf ("`git %s' is aliased to `%s'\n" , argv[0 ], alias); +return 0 ; + } +switch (help_format) {case HELP_FORMAT_MAN :show_man_page (argv[0 ]);
Signature · 1 line
1.5.4.3.305.g073a4
Re: [PATCH 3/3] help: respect aliases
Quoted 3 lines · Jeff King
If we have an alias "foo" defined, then the help text for
"foo" (via "git help foo" or "git foo --help") now shows the definition of the alias.
Heh, now I can kill this alias of mine:
h = "!sh -c 'git alias \"$1\" || git help \"$1\"' -"
:-)
This too would be less ugly as a built-in:
alias = "! sh -c 'if test -z \"$1\"; then git config --list \
| expand \
| sed \"/^alias\\./!d; s/^alias\\.//; s/=/ /; s/ */ /g\" \
| sort | while read n v; do \
printf \"%-16s\" \"$n\"; echo \"$v\" | gnused \
\"s/\\(.\\{68\\}\\) /\\1\\n\t\t/g\" | expand; \
done; else git config \"alias.$1\"; fi' -":-)
j.
Re: [PATCH 3/3] help: respect aliases
Hi,
Quoted 9 lines · Jay Soffian
This too would be less ugly as a built-in:
alias = "! sh -c 'if test -z \"$1\"; then git config --list \ | expand \ | sed \"/^alias\\./!d; s/^alias\\.//; s/=/ /; s/ */ /g\" \ | sort | while read n v; do \ printf \"%-16s\" \"$n\"; echo \"$v\" | gnused \ \"s/\\(.\\{68\\}\\) /\\1\\n\t\t/g\" | expand; \ done; else git config \"alias.$1\"; fi' -"
Wow. This would look less ugly as an alias like this, too:
alias = "!sh -c 'case $# in \
0) git config --get-regexp \"^alias\\.\" | sed \"s/^alias\\.//\";; \
*) git config \"alias.$0\" ;; \
esac'"which incidentally fixes a bug in your alias: you ignore $0 which is the first parameter when using the sh -c '' idiom.
Of course, you can change the sed call to your liking...
Ciao,
Dscho
Re: [PATCH 3/3] help: respect aliases
Quoted 9 lines · Johannes Schindelin
Quoted 3 lines · Jay Soffian
This too would be less ugly as a built-in:
[elided so as not to burn anyone's eyes out again :-) -- jay]
Wow. This would look less ugly as an alias like this, too:
Quoted 7 lines
alias = "!sh -c 'case $# in \ 0) git config --get-regexp \"^alias\\.\" | sed \"s/^alias\\.//\";; \ *) git config \"alias.$0\" ;; \ esac'"which incidentally fixes a bug in your alias: you ignore $0 which is the first parameter when using the sh -c '' idiom.
Test mine. Test yours. See which works. :-)
At least on my system, I had to use:
sh -c '...' -
And then refer to $1.
Quoted 1 line
Of course, you can change the sed call to your liking...
Most of the ugliness in mine is that I crammed the whitespace down and that I'm using a quite involved pipeline to reformat all my aliases so they fit into my terminal window.
But the --get-regexp is a good tip.
j.
Re: [PATCH 3/3] help: respect aliases
Hi,
Quoted 16 lines · Jay Soffian
Quoted 14 lines · Johannes Schindelin
Quoted 3 lines · Jay Soffian
This too would be less ugly as a built-in:
[elided so as not to burn anyone's eyes out again :-) -- jay]
Wow. This would look less ugly as an alias like this, too:
alias = "!sh -c 'case $# in \ 0) git config --get-regexp \"^alias\\.\" | sed \"s/^alias\\.//\";; \ *) git config \"alias.$0\" ;; \ esac'"
Quoted 4 lines
Quoted 2 lines
which incidentally fixes a bug in your alias: you ignore $0 which is the first parameter when using the sh -c '' idiom.
Test mine. Test yours. See which works. :-)
I tested only mine. It works.
Ciao,
Dscho
Re: [PATCH 3/3] help: respect aliases
In reply to Johannes SchindelinQuoted 16 lines · Johannes Schindelin
Hi,
Quoted 9 lines · Jay Soffian
This too would be less ugly as a built-in:
alias = "! sh -c 'if test -z \"$1\"; then git config --list \ | expand \ | sed \"/^alias\\./!d; s/^alias\\.//; s/=/ /; s/ */ /g\" \ | sort | while read n v; do \ printf \"%-16s\" \"$n\"; echo \"$v\" | gnused \ \"s/\\(.\\{68\\}\\) /\\1\\n\t\t/g\" | expand; \ done; else git config \"alias.$1\"; fi' -"Wow. This would look less ugly as an alias like this, too:
Quoted 7 lines
alias = "!sh -c 'case $# in \ 0) git config --get-regexp \"^alias\\.\" | sed \"s/^alias\\.//\";; \ *) git config \"alias.$0\" ;; \ esac'"which incidentally fixes a bug in your alias: you ignore $0 which is the first parameter when using the sh -c '' idiom.
I notice that he is feeding an exra '-' for that.
Quoted 1 line
Of course, you can change the sed call to your liking...
Sure. The combination of expand, sed, and gnused hurts my eyes ;-)
One good lesson to take home is that by ending your lines with '|', you can tell the shell that you are not done with the pipeline you are forming yet. This allows you to lose all these ugly backslashes, like this:
xyzzy |
frotz |
nitfol
instead of
xyzzy \
| frotz \
| nitfolRe: [PATCH 3/3] help: respect aliases
Quoted 7 lines · Junio C Hamano
Sure. The combination of expand, sed, and gnused hurts my eyes ;-)
One good lesson to take home is that by ending your lines with '|', you can tell the shell that you are not done with the pipeline you are forming yet.
The backslashes aren't there for the shell, they are there for the git config parser. Anyway, I reworked it a bit:
alias = "! sh -c 'case $# in \
0) git config --get-regexp \"^alias\\.\" \
| sort | sed \"s/^alias\\.//; s/=/ /\" \
| while read n v; do \
printf \"%-16s\" \"$n\"; \
echo \"$v\" | fmt -sw 63 | sed \"2,$ s/^/\t\t/\" ;\
done ;; \
1) git config \"alias.$1\" | fmt -sw 78 ;; esac' -"And yes, it's still ugly.
Always happy to have folks help me bikeshed my git config. :-)
j.