In addition to points raised by Philip and Torsten...

On Sun, Mar 15, 2015 at 9:49 AM, Alexander Kuleshov
<kuleshovm...@gmail.com> wrote:
> add: add new --exclude option to git add

No need for redundant "to git add", since you already have the "add:" prefix.

> This patch introduces new --exclude option for the git add
> command.

This line merely repeats the Subject: line, thus can be dropped.

> We already have core.excludesfile configuration variable which indicates
> a path to file which contains patterns to exclude. This patch provides
> ability to pass --exclude option to the git add command to exclude paths
> from command line in addition to which found in the ignore files.

The commit message is missing the important justification for why this
new option is desirable, and why only git-add needs it.

> Signed-off-by: Alexander Kuleshov <kuleshovm...@gmail.com>
> ---
> diff --git a/builtin/add.c b/builtin/add.c
> index 3390933..5c602a6 100644
> --- a/builtin/add.c
> +++ b/builtin/add.c
> @@ -244,6 +244,16 @@ static int ignore_removal_cb(const struct option *opt, 
> const char *arg, int unse
>         return 0;
>  }
>
> +struct string_list exclude_list = STRING_LIST_INIT_NODUP;

Shouldn't this be declared static?

> +struct exclude_list *el;

Why is this declared globally when it's only needed locally by cmd_add()?

> +static int exclude_cb(const struct option *opt, const char *arg, int unset)
> +{
> +       struct string_list *exclude_list = opt->value;
> +       string_list_append(exclude_list, arg);
> +       return 0;
> +}
> +
>  static struct option builtin_add_options[] = {
>         OPT__DRY_RUN(&show_only, N_("dry run")),
>         OPT__VERBOSE(&verbose, N_("be verbose")),
> @@ -255,6 +265,9 @@ static struct option builtin_add_options[] = {
>         OPT_BOOL('u', "update", &take_worktree_changes, N_("update tracked 
> files")),
>         OPT_BOOL('N', "intent-to-add", &intent_to_add, N_("record only the 
> fact that the path will be added later")),
>         OPT_BOOL('A', "all", &addremove_explicit, N_("add changes from all 
> tracked and untracked files")),
> +       { OPTION_CALLBACK, 0, "exclude", &exclude_list, N_("pattern"),
> +         N_("do no add files matching pattern to index"),
> +         0, exclude_cb },

Can this just be OPT_STRING_LIST instead of OPTION_CALLBACK?

>         { OPTION_CALLBACK, 0, "ignore-removal", &addremove_explicit,
>           NULL /* takes no arguments */,
>           N_("ignore paths removed in the working tree (same as --no-all)"),
> @@ -298,6 +311,7 @@ static int add_files(struct dir_struct *dir, int flags)
>
>  int cmd_add(int argc, const char **argv, const char *prefix)
>  {
> +       int i;
>         int exit_status = 0;
>         struct pathspec pathspec;
>         struct dir_struct dir;
> @@ -381,6 +395,11 @@ int cmd_add(int argc, const char **argv, const char 
> *prefix)
>                 if (!ignored_too) {
>                         dir.flags |= DIR_COLLECT_IGNORED;
>                         setup_standard_excludes(&dir);
> +
> +                       el = add_exclude_list(&dir, EXC_CMDL, "--exclude 
> option");
> +                       for (i = 0; i < exclude_list.nr; i++)
> +                               add_exclude(exclude_list.items[i].string, "", 
> 0, el, -(i+1));
> +
>                 }
>
>                 memset(&empty_pathspec, 0, sizeof(empty_pathspec));
> @@ -446,5 +465,6 @@ finish:
>                         die(_("Unable to write new index file"));
>         }
>
> +       string_list_clear(&exclude_list, 0);
>         return exit_status;
>  }
> --
> 2.3.3.472.g20ceeac
--
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