Web lists-archives.com

Re: [GSoC][PATCH 08/13] submodule: port submodule subcommand 'summary' from shell to C




On Mon, Aug 7, 2017 at 11:18 PM, Prathamesh Chavan <pc44800@xxxxxxxxx> wrote:

> +static enum {
> +       DIFF_INDEX,
> +       DIFF_FILES
> +} diff_cmd = DIFF_INDEX;

Using an enum could be a good idea, but I am not sure about using a
static variable.

> +static int compute_summary_module_list(char *head, struct summary_cb *info)
> +{
> +       struct argv_array diff_args = ARGV_ARRAY_INIT;
> +       struct rev_info rev;
> +       struct module_cb_list list = MODULE_CB_LIST_INIT;
> +
> +       argv_array_push(&diff_args, diff_cmd ? "diff-files" : "diff-index");

Maybe diff_cmd could be an argument of compute_summary_module_list()
instead of a static variable, as it's only used in this function and
in module_summary() below which is calling it.

[...]

> +       if (files) {
> +               if (cached)
> +                       die(_("The --cached option cannot be used with the --files option"));
> +               diff_cmd++;

Couldn't this be:

               diff_cmd = DIFF_FILES;

?