Subject: Re: bug#37215: [PATCH] vc-cvs-ignore writes absolute filenames and duplicate strings

From: Eli Zaretskii <[email-masked]>
Date: Sat, 15 Feb 2020 09:44:23 +0200

> Cc: [email-masked], [email-masked], Dmitry Gutov <[email-masked]>
> From: Wolfgang Scherer <[email-masked]>
> Date: Sat, 15 Feb 2020 02:42:28 +0100
> 
> >> vc-dir-ignore with patch from #37240
> > OK, but then please document this use case and how DIRECTORY is used
> > in this function.  The various -ignore functions in vc.el and in
> > backends assign different semantics to their DIRECTORY argument, and I
> > think these (largely undocumented) differences are a source of some
> > confusion and bugs in this area.
> This is one of the first errors I ran into and I know a lot more now,
> after doing the research.
> 
> There is a problem with describing the use case, because
> *vc-cvs-append-to-ignore* has several of them. So I think it is best,
> that Dmitry comes up with an overall solution for *vc-ignore* - as he
> has planned - which would naturally solve this problem, too. Since CVS
> is a special case, because another package, PCL-CVS, is also involved,
> therefore increasing the danger that something breaks, I have put my
> findings at the end for your convenience.
> >
> >>>   Because if that happens, the file's name will be added to
> >>> .cvsignore not in DIRECTORY but in one of its subdirectories.  Would
> >>> that be surprising?
> >> Not for anybody familiar with CVS.
> > This should be documented, IMO.  The existing documentation of
> > .cvsignore is minimal, and doesn't mention several important aspects.
> > For example, the fact that only basenames are allowed is only hinted
> > upon, and there's no information whatsoever AFAICT whether characters
> > special to wildcards can be escaped.  So I think we should provide
> > this minimal information either in doc strings or at least in comments
> > in the code.
> 
> As for the pattern syntax for CVS, it is following POSIX and is fully
> described in the man page for glob(7) including the escape mechanism
> with backslash. There is one quirk in the .cvsignore file parser, which
> breaks patterns not only at lines but also at other whitespace on the
> same line. It is therefore better to match a filename containing spaces
> with a `?` for each space character.

I'd like to install your patch, and I'd like to do it soon, so it
makes it into Emacs 27.  Can you please propose a modified patch which
adds some minimal information about what's going on to the doc string
and/or the comments around the code?

IOW, I don't think we need any further discussions of this issue, we
need a somewhat improved patch that explains more why its code is
correct.  Do you think you could provide such an improved patch?  I
don't think it's wise to make this patch wait for untangling the more
general vc-ignore issues.

Thanks.