threads / rfc / 1335

Re: [RFC] extending git-ls-files --exclude.

Subject: Re: [RFC] extending git-ls-files --exclude.

## tl;dr

3 messages between Jul 25, 2005 and Jul 25, 2005.

replies: 2people: 2as markdown or json

Marco Costalba· Jul 25, 2005, 06:41 UTC · lore

Junio C Hamano wrote:

Show 14 quoted lines

>The list of patterns that is in effect at a given time is
>built and ordered in the following way:
>
> * --exclude=<pattern> and lines read from --exclude-from=<file>
>   come at the beginning of the list of patterns, in the order
>   given on the command line.  Patterns that come from the file
>   specified with --exclude-from are ordered in the same order
>   as they appear in the file.
>
> * When --exclude-per-directory=<name> is specified, upon
>   entering a directory that has such a file, its contents are
>   appended at the end of the current "list of patterns".  They
>   are popped off when leaving the directory.
>

Are really necessary to have both --exclude-from=<file> and --exclude-per-directory=<name> ?

Peraphs, if the file name of excluded list is the same for each directory, e.g. .gitignore or something similar, instead of --exclude-per-directory we can use a concept of file validity 'scope' and just use --exclude-from=<file>. If entering in a directory <file> is found its contents are appended and removed when leaving directory. A bad analogy can be with the use of recursive Makefile.

Show 7 quoted lines

>
>A pattern specified on the command line with --exclude or read
>from the file specified with --exclude-from is relative to the
>top of the directory tree.  A pattern read from a file specified
>by --exclude-per-directory is relative to the directory that the
>pattern file appears in.
>

If we use the 'scope' logic we can just prepend path when adding entries and serach with with FNM_PATHNAME flag.

Show 12 quoted lines

> - if it does not contain a slash '/', it is a shell glob
>   pattern and used to match against the filename without
>   leading directories (i.e. the same way as the current
>   implementation).
>
> - otherwise, it is a shell glob pattern, suitable for
>   consumption by fnmatch(3) with FNM_PATHNAME flag.  I.e. a
>   slash in the pattern must match a slash in the pathname.
>   "Documentation/*.html" matches "Documentation/git.html" but
>   not "ppc/ppc.html".  As a natural exception, "/*.c" matches
>   "cat-file.c" but not "mozilla-sha1/sha1.c".
>

Same comment as above, if prepending path when adding per directory contents we can simplify to always use FNM_PATHNAME flag.

We don't have even to special case base directory global scope <file> if we use realtive paths so that we prepend its contents with ./ and we have what we expect.

I am sorry to not be able to send a patch, better explaining what proposed but I am just leaving today and I will be off line for a couple of weeks.

Marco

__________________________________________________ Do You Yahoo!? Tired of spam? Yahoo! Mail has the best spam protection around http://mail.yahoo.com

Junio C Hamano· Jul 25, 2005, 07:32 UTC · re: Marco Costalba · lore

Marco Costalba <mcostalba@yahoo.it> writes:

Show 9 quoted lines

> Are really necessary to have both --exclude-from=<file> and 
> --exclude-per-directory=<name> ?
>
> Peraphs, if the file name of excluded list is the same for each directory, 
> e.g. .gitignore or something similar, instead of --exclude-per-directory
> we can use a concept of file validity 'scope' and just use --exclude-from=<file>.
> If entering in a directory <file> is found its contents are appended and 
> removed when leaving directory. A bad analogy can be with the use of 
> recursive Makefile.

Pasky wants to have one of the files in ~/.git-something if I understand correctly. I did not want to remove --exclude-from for that reason.

> If we use the 'scope' logic we can just prepend path when
> adding entries and serach with with FNM_PATHNAME flag.

And I wanted for people's script and existing ignore files to continue to work.

> Same comment as above, if prepending path when adding per 
> directory contents we can simplify to always use FNM_PATHNAME flag.

Yes, but that means you need to always prepend the current directory being looked at to get the original behaviour. The way it was originally done without FNM_PATHNAME was far easier to read, at least for me.

Junio C Hamano· Jul 25, 2005, 19:56 UTC · re: Marco Costalba · lore

Marco Costalba <mcostalba@yahoo.it> writes:

Show 6 quoted lines

> Peraphs, if the file name of excluded list is the same for each directory, 
> e.g. .gitignore or something similar, instead of --exclude-per-directory
> we can use a concept of file validity 'scope' and just use --exclude-from=<file>.
> If entering in a directory <file> is found its contents are appended and 
> removed when leaving directory. A bad analogy can be with the use of 
> recursive Makefile.

I do not know if you noticed it, but the patch already have the concept of "scope". If a pattern has a slash '/' it is scoped to the named directory by using FNM_PATHNAME. "The named directory" for command line --exclude and --exclude-from are relative to the project top while it is relative to the directory the file being used is in for --exclude-per-directory case.

> If we use the 'scope' logic we can just prepend path when adding entries
> and serach with with FNM_PATHNAME flag.

You suggest to always use FNM_PATHNAME by adjusting the pattern by prefixing the scope. While that can theoretically be made to work just as the posted patch does, you need to realize it is a lot more cumbersome to deal with metacharacters in pathnames your way. If the prefix you need to add (because you are now looking at a path for that directory which has a funny name) is "foo*bar", you need to add "foo\*bar" to the pattern in order to make sure that the asterisk is not used to match anything other than a literal asterisk, for example.

You could argue that nobody should use funny characters in pathnames, and you could further argue that current Porcelains do not handle certain characters in pathnames anyway so why bother. But ls-files being core, which is "the funny filesystem layer to build SCM on", I would like to be careful not to be unnecessarily restrictive.