Re: [PATCH v2] Add --exclude to git-clean.

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 19, 2010, 17:43 UTC
Message-ID: <7vbpa3pdtc.fsf@alter.siamese.dyndns.org>
In-Reply-To: <bf4ea4e83162ebf1d915b87413aad54a1162a637.1279556172.git.jaredhance@gmail.com>

Jared Hance <jaredhance@gmail.com> writes:

Show 10 quoted lines

> diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt
> index a81cb6c..2bf9cf2 100644
> --- a/Documentation/git-clean.txt
> +++ b/Documentation/git-clean.txt
> @@ -8,7 +8,7 @@ git-clean - Remove untracked files from the working tree
>  SYNOPSIS
>  --------
>  [verse]
> -'git clean' [-d] [-f] [-n] [-q] [-x | -X] [--] <path>...
> +'git clean' [-d] [-f] [-n] [-q] [-e] [-x | -X] [--] <path>...

This and ...

Show 8 quoted lines

>  DESCRIPTION
>  -----------
> @@ -45,6 +45,10 @@ OPTIONS
>  	Be quiet, only report errors, but not the files that are
>  	successfully removed.
>  
> +-e::
> +--exclude::

this both look wrong. They do not tell the readers that the option takes a mandatory argument that specifies the "exceptions". Worse yet,

> +	Specify special exceptions to not be cleaned. Separate with colon.

this does not tell _how_ exceptions are specified.

What should each element on the list look like?

Is it a dot-suffix without dot (e.g. "html") or with dot (e.g. ".html")? Or is it a glob (e.g. "*.html")? Or is it a full path relative to the worktree root (e.g. "Documentation/index.html")?

Using colon as an inter-element separator makes sense only if last one is true (i.e. "concrete path, not glob nor suffix"), so an intelligent reader could probably guess what you meant, but you shouldn't make readers guess in the first place.

If on the other hand you wanted to allow specifying the same kind of patterns used in the gitignore files from the command line:

 (1) A list separated with whitespace would be more natural, not a colon;
     and

 (2) I have to wonder why do we give such a command line exclude override
     to begin with.

     (2-a) wouldn't it be easier for the user to add such a local
           configuration to $GIT_DIR/info/exclude once and be done with
           it?

     (2-b) if command-line override has benefit, why is it limited to only
           _exclude_ and not include (iow, additional ignore patterns)?