Philip Oakley <philipoak...@iee.org> writes:

> 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 <philipoak...@iee.org>
> ---
>  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.h
>
> diff --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.

> 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?

> 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,
--
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

Reply via email to