On Sat, May 2, 2015 at 8:23 PM, Sébastien Guimmara
<sebastien.guimmara@xxxxxxxxx> wrote:
> When 'git help' is called, a list of common commands are printed:
>
> The most commonly used git commands are:
> add Add file contents to the index
> bisect Find by binary search the change that introduced a bug
> branch List, create, or delete branches
> checkout Checkout a branch or paths to the working tree
> clone Clone a repository into a new directory
> commit Record changes to the repository
> [...]
>
> Instead of a less than optimal alphabetical order, print those
> commands in theme-related groups:
>
> The most commonly used git commands are:
> clone Clone a repository into a new directory
> init Create an empty Git repository or reinitialize an existing one
>
> log Show commit logs
> show Show various types of objects
> status Show the working tree status
>
> add Add file contents to the index
> mv Move or rename a file, a directory, or a symlink
> reset Reset current HEAD to the specified state
> rm Remove files from the working tree and from the index
> [...]
>
> To achieve this, qsort the common_cmds array by group name,
> then print those common commands, skipping a line between each group.
Just a minor observation regarding the final paragraph (feel free to ignore):
This level of detail is probably overkill. The overall intent of the
change has already been spelled out nicely via the examples, so
mentioning qsort() is unnecessary since it's a mere implementation
detail; one which a reader can easily discover by reading the patch
proper. Likewise, the example "new output" already shows clearly the
blank line between groups, so there is no need to mention it in prose.
If I was composing the commit message, I'd probably drop the final
paragraph altogether and rephrase something like this:
'git help' shows common commands in alphabetical order like this:
<current output>
without any indication of how commands relate to high-level
concepts or each other. Revise the output to group commands by
concept, like this:
<revised output>
More below.
> Signed-off-by: Sébastien Guimmara <sebastien.guimmara@xxxxxxxxx>
> ---
> diff --git a/help.c b/help.c
> index 2072a87..2169a59 100644
> --- a/help.c
> +++ b/help.c
> @@ -218,17 +218,38 @@ void list_commands(unsigned int colopts,
> }
> }
> +/* sort the command name struct by group name */
> +int cmd_group_cmp(const void *elem1, const void *elem2)
> +{
> + struct cmdname_help *cmd1 = (struct cmdname_help*) elem1;
> + struct cmdname_help *cmd2 = (struct cmdname_help*) elem2;
As this is C rather than C++, you can drop the cast.
Also, if you were to keep the cast, the prevailing style in git code
is to insert a space before the '*' and to drop the space after the
')'.
> + return strcmp(cmd1->group, cmd2->group);
> +}
> +
> void list_common_cmds_help(void)
> {
> int i, longest = 0;
> + char *current_grp = NULL;
> for (i = 0; i < ARRAY_SIZE(common_cmds); i++) {
> if (longest < strlen(common_cmds[i].name))
> longest = strlen(common_cmds[i].name);
> }
> + /* sort common commands by group (i.e, beginner's relevance) */
> + qsort(common_cmds, ARRAY_SIZE(common_cmds),
> + sizeof(struct cmdname_help), cmd_group_cmp);
Slightly more maintenance free would be sizeof(common_cmds[0]) instead
of sizeof(struct cmdname_help).
> puts(_("The most commonly used git commands are:"));
> for (i = 0; i < ARRAY_SIZE(common_cmds); i++) {
> +
Style: Drop this blank line.
> + /* skip a line each time we encounter a new command group */
> + if (current_grp != NULL && strcmp(common_cmds[i].group,
> current_grp))
> + printf("\n");
This chunk of code is pretty well self-explanatory, so the comment
isn't really adding anything. A comment which merely repeat what the
code itself already says is typically considered distracting rather
than illuminating, thus it could (and probably should) be dropped. The
same observation probably applies to the comment preceding the qsort()
invocation, as well.
Finally, elsewhere in this source file, a blank line is emitted with
the more idiomatic putchar('\n') rather than printf("\n").
> +
> + current_grp = common_cmds[i].group;
> +
> printf(" %s ", common_cmds[i].name);
> mput_char(' ', longest - strlen(common_cmds[i].name));
> puts(_(common_cmds[i].help));
> --
> 2.4.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html