bug#37189: 25.4.1: vc-hg-ignore implementation is missing

[Dmitry Gutov]

On 14.02.2020 11:23, Eli Zaretskii wrote:
> > Cc: 37189@debbugs.gnu.org
> > From: Dmitry Gutov <dgutov@yandex.ru>
> > Date: Fri, 14 Feb 2020 01:40:31 +0200
> >
> > I think the first thing I'll have to do is revert a part of an earlier
> > patch to vc-default-ignore which changed its semantics a little, because
> > it doesn't look like this discussion is going to culminate in a patch
> > small enough for emacs-27 anyway.
>
> Which part?  Can you show a proposed patch?

See https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37217.

The patch fixed a legitimate scenario, but made an existing one work 
worse. Trying to vc-ignore, say, '*.c' from a subdirectory in a Git 
repo, for instance, will now prepend the intermediary directories to it.

(Mentioned this before: 
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37189#41)

Now, one could argue that one use case is more important than the other 
one, and that vc-ignore has for a while been geared toward entering 
plain file names rather than glob patterns. I have little opinion on 
this subject, however, since I mostly edit ignore files by hand, and do 
so very rarely. So as far as I'm concerned, we could remove this feature 
altogether and not lose much.

But maybe it's more valuable for SVN users? Where the ignore 
configuration is more tricky and the DIRECTORY argument is actually 
important.

So the options at hand are:

- Revert almost all of the patch from bug#37217, reverting to the 
previous, admittedly broken behavior, and continue to discuss a better 
improvement for Emacs 28.
- Try to resolve the ambiguity of purpose in favor either entering 
patterns on file names only. Probably the latter, later vc-ignore to 
vc-ignore-file.
- Try to sit on both chairs... Basically, that means using the user 
input unaltered. Allowing them to enter a file name as well, but treat 
it as a pattern, without escaping or the like. This would be close to 
the original intent behind vc-ignore, AFAICT.

To do the last one, read-file-name would need to be called with the 
second argument provided, the directory against which the file path 
should be relative. For most backend, we can reuse the find-ignore-file 
backend command, but SVN (and RCS, etc) don't have it defined. Roughly 
and handwavy, we can take this case to mean "use default-directory".

As you noted, the use of read-file-name at all in vc-ignore is somewhat 
problematic, but let's see if we can keep the function sane without 
removing it first.

Attaching a patch. Eli, Wolfgang, any objections?

> > Then, naturally, we'll have to look for small changes that improve the
> > situation but provide as little breakage as possible.
>
> Agreed.

Alas, the attached patch is probably not a "small" one. Option 1 would 
pass this criterion, though.

> 1) vc-dir-ignore:
>
> It calculates the file name for the entry into the ignore file, and
> thus must escape any characters in the file name that are special in
> ignore files -- which is backend-specific.
>
> The file name should also be "anchored", at least ideally, so that no
> other file is accidentally ignored.  This is also backend-specific.

I think we can add a new backend action that would escape and anchor a 
file name (and maybe turn it from an absolute into a relative one). That 
should take care of it.

> VC's support for directory-specific ignore files is inconsistent: they
> are supported for CVS (and actually mandatory there) and maybe SVN,
> but not for Git/Bazaar/Mercurial/Monotone -- for the latter we only
> support ignore files in the repository root.  This could be improved
> in the future, but for now I don't see an immediate need.

Agreed.

> For the same reason, using read-file-name here is too naïve, as well
> as yielding an absolute file name from what the user types.

For now, I kept the former but decided against the latter.

> If you agree with the above, we then need to decide what, if anything,
> of this should be fixed for Emacs 27.  E.g., vc-dir-ignore seems to be
> OK if the file name doesn't include special characters, does it?

With the attached patch, it needed a small adjustment, but with that 
should work okay-ish again.

vc-ignore.diff
Description: Text Data