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

[Wolfgang Scherer]

Am 28.08.19 um 08:16 schrieb Eli Zaretskii:
>> Cc: 37189@debbugs.gnu.org
>> From: Wolfgang Scherer <Wolfgang.Scherer@gmx.de>
>> Date: Wed, 28 Aug 2019 03:46:53 +0200
>>
>>>> +(defun vc-hg-ignore (file &optional directory remove)
>>>> +  "Ignore FILE of DIRECTORY (default is `default-directory').
>>>> +
>>>> +FILE is a file wildcard, relative to the root directory of DIRECTORY.
>>> I think instead of "root directory of DIRECTORY" this should say "the
>>> top-level directory of DIRECTORY's repository".
>> I disagree. This comment is a modified version of the comment for
>> `vc-default-ignore'. And the exact same phrase also pops up for
>> `vc-svn-ignore':
> But do you agree that the alternative text I proposed is more accurate
> and more clear?  If so, we might wish to change all those places to
> use the better wording.  It could be a separate commit, of course.


I agree, that the text should be corrected (and I already did so
for `vc-hg-ignore').  I do not agree to introducing the term
`top-level directory' as replacement for `project root'.

However, the problem here is not the wording, but the
incorrect/incomplete implementation of various
commands/functions. (Actually, not a single handler
implementation is correct. Not even CVS.)

1. `vc-ignore' provides the API specification for the
   backend-specific implementations:

     "Ignore FILE under the VCS of DIRECTORY.

     Normally, FILE is a wildcard specification that matches the files
     to be ignored.  When REMOVE is non-nil, remove FILE from the list
     of ignored files."
  
   Based on this API, mentioning the project root at all does not
   make any sense.

   If the wildcard specification is relative, it is relative to
   DIRECTORY (or default-directory).  It is definitely not
   relative to the project root.
  
2. `vc-cvs-ignore' implements only the appending part, but does
   not implement the `remove' option.

   When appending, the correct .cvsignore file is identified,
   but when called with an absolute filename (which is the
   default case), it writes the entire path into the .cvsignore
   file.

   `vc-cvs-ignore' also writes duplicate strings into .cvsignore.

   I submitted a patch for these errors as bug#37215.

   (And yes, I still have a bunch of old unconverted CVS repositories ;-)).

3. When `vc-svn-ignore' starts a new `svn:ignore' property, The
   function `vc-svn-ignore-completion-table' parses an error
   message as ignore patterns. The split also matches any
   whitespace instead of lines only. I have submitted a patch for
   these errors as bug##37214.

   `vc-svn-ignore' states:

      "Ignore FILE under Subversion.
      FILE is a file wildcard, relative to the root directory of DIRECTORY."

    This is just not true. The correct description is:

      "Ignore FILE under Subversion.
      FILE is a wildcard specification, either relative to
      DIRECTORY or absolute."

    `vc-svn-ignore' gets the relative filename right (in a manner
    suitable for Git), but then fails to add the ignore spec to
    the correct subdirectory, if the relative path has at least
    one level of directories. A patch is supplied as bug#37216.

4. This leaves `vc-default-ignore'.

   Both the description and implementation are wrong.

   Only the basename of FILE is written to the ignore file. This is
   wrong for all filenames relative to project root with one or
   more parent directories.

   The remove option is also implemented incorrectly.

   A patch is supplied as bug#37217.

The mentioned commits replace all occurences of the incorrect description.