Re: [PATCH 1/3 v2] add: introduce new --exclude option

[Eric Sunshine <sunshine@sunshineco.com>]

On Sun, Mar 15, 2015 at 3:06 PM, Alexander Kuleshov
<kuleshovmail@gmail.com> wrote:
> 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 specified in the ignore files.
>
> This option can be useful in a case when we have a directory with some *.ext
> files which have changes and we want to commit all files besides one for now.
> It can be too annoying to touch .gitignore for this.

Won't this lead to unintuitive behavior? The 'excludes' mechanism does
not unconditionally ignore; instead, it ignores _untracked_ files.
Consider file "foo" which is already tracked. Make a temporary change
to "foo" which you don't intend to commit. Since "foo" is tracked, the
command 'git add . --exclude=foo' will still add "foo" to the index,
despite the use of --exclude. Most people would probably find such
behavior surprising and undesirable.

The negative pathspec mentioned by Junio[1], on the other hand, does
not suffer this shortcoming.

More below.

[1]: http://thread.gmane.org/gmane.comp.version-control.git/265493/focus=265518

> Helped-by: Eric Sunshine <sunshine@sunshineco.com>
> Helped-by: Torsten Bögershausen <tboegi@web.de>
> Helped-by: Philip Oakley <philipoakley@iee.org>
> Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
> ---
> diff --git a/builtin/add.c b/builtin/add.c
> index 3390933..e165fbc 100644
> --- a/builtin/add.c
> +++ b/builtin/add.c
> @@ -244,6 +244,8 @@ static int ignore_removal_cb(const struct option *opt, const char *arg, int unse
>         return 0;
>  }
>
> +static struct string_list exclude_list = STRING_LIST_INIT_NODUP;
> +
>  static struct option builtin_add_options[] = {
>         OPT__DRY_RUN(&show_only, N_("dry run")),
>         OPT__VERBOSE(&verbose, N_("be verbose")),
> @@ -255,6 +257,8 @@ 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")),
> +       OPT_STRING_LIST(0, "exclude", &exclude_list, N_("pattern"),
> +                       N_("do not add files matching pattern to index")),
>         { OPTION_CALLBACK, 0, "ignore-removal", &addremove_explicit,
>           NULL /* takes no arguments */,
>           N_("ignore paths removed in the working tree (same as --no-all)"),
> @@ -305,6 +309,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
>         int add_new_files;
>         int require_pathspec;
>         char *seen = NULL;
> +       struct exclude_list *el;

This variable is only used within the 'if (!ignored_too)' block below,
so its declaration should be moved there.

>         git_config(add_config, NULL);
>
> @@ -379,8 +384,14 @@ int cmd_add(int argc, const char **argv, const char *prefix)
>                 /* Set up the default git porcelain excludes */
>                 memset(&dir, 0, sizeof(dir));
>                 if (!ignored_too) {
> +                       int i;
>                         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 +457,6 @@ finish:
>                         die(_("Unable to write new index file"));
>         }
>
> +       string_list_clear(&exclude_list, 0);
>         return exit_status;
>  }
> --
> 2.3.3.472.g20ceeac.dirty