Re: [PATCH V3 4/5] Help.c: add list_common_guides_help() function
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:56:37
Philip Oakley [off-list ref] writes:
quoted hunk
Re-use list_common_cmds_help but simply change the array name. Candidate for future refactoring to pass a pointer to the array. The common-guides.h list was generated with a simple variant of the generate-cmdlist.sh and command-list.txt. Do not list User-manual and Everday Git which not follow the naming convention, nor gitrepository-layout which doesn't fit within the name field size. Signed-off-by: Philip Oakley <redacted> --- builtin/help.c | 3 ++- common-guides.h | 11 +++++++++++ help.c | 18 ++++++++++++++++++ help.h | 1 + 4 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 common-guides.hdiff --git a/builtin/help.c b/builtin/help.c index 03d432b..91a6158 100644 --- a/builtin/help.c +++ b/builtin/help.c@@ -433,7 +433,8 @@ int cmd_help(int argc, const char **argv, const char *prefix) } if (show_guides) { - /* do action - next patch */ + list_common_guides_help(); + printf("\n"); }
This looks funny. If you look at list_commands() that this patch is mimicking, you will notice that the "trailing blank for clarity" is done as part of the function, not done by the caller. I think it is better done the same way.
quoted hunk
diff --git a/common-guides.h b/common-guides.h new file mode 100644 index 0000000..0e94fdc --- /dev/null +++ b/common-guides.h@@ -0,0 +1,11 @@ +/* re-use struct cmdname_help in common-commands.h */ + +static struct cmdname_help common_guides[] = { + {"attributes", "defining attributes per path"}, + {"glossary", "A GIT Glossary"}, + {"ignore", "Specifies intentionally untracked files to ignore"}, + {"modules", "defining submodule properties"}, + {"revisions", "specifying revisions and ranges for git"}, + {"tutorial", "A tutorial introduction to git (for version 1.5.1 or newer)"}, + {"workflows", "An overview of recommended workflows with git"}, +};
The _only_ reason we have common-cmds.h as a separat file even though it defines data (hence should not be included in more than one *.c file) is because it is a generated file. For this array, there is no reason to have it in a separate header file. Just define it immediately before list_common_guies_help() function that is the sole user of the array. The function can live in builtin/help.c as a static, without touching global help.c nor help.h, no? Is there a reason why it should be callable from other places?
quoted hunk
diff --git a/help.c b/help.c index 1dfa0b0..e0368ca 100644 --- a/help.c +++ b/help.c@@ -4,6 +4,7 @@ #include "levenshtein.h" #include "help.h" #include "common-cmds.h" +#include "common-guides.h" #include "string-list.h" #include "column.h" #include "version.h"@@ -240,6 +241,23 @@ void list_common_cmds_help(void) } } +void list_common_guides_help(void) +{ + int i, longest = 0; + + for (i = 0; i < ARRAY_SIZE(common_guides); i++) { + if (longest < strlen(common_guides[i].name)) + longest = strlen(common_guides[i].name); + } + + puts(_("The common Git guides are:\n")); + for (i = 0; i < ARRAY_SIZE(common_guides); i++) { + printf(" %s ", common_guides[i].name); + mput_char(' ', longest - strlen(common_guides[i].name)); + puts(_(common_guides[i].help)); + } +} + int is_in_cmdlist(struct cmdnames *c, const char *s) { int i;diff --git a/help.h b/help.h index 0ae5a12..4ae1fd7 100644 --- a/help.h +++ b/help.h@@ -17,6 +17,7 @@ static inline void mput_char(char c, unsigned int num) } extern void list_common_cmds_help(void); +extern void list_common_guides_help(void); extern const char *help_unknown_cmd(const char *cmd); extern void load_command_list(const char *prefix, struct cmdnames *main_cmds,