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

[Wolfgang Scherer]

Am 01.02.20 um 09:27 schrieb Eli Zaretskii:
>> From: Wolfgang Scherer <Wolfgang.Scherer@gmx.de>
>> Date: Sat, 1 Feb 2020 02:20:08 +0100
>>
>> I don't think, that the problem with ignore specs can be solved in
>> this (adhoc) manner. It should (finally) be properly designed.
>>
>> There is a fundamental difference between per-directory ignore specs
>> and per-tree ignore specs. Let me try to outline, where I see the
>> problem with the current implementation:
>>
>> 1. Per-tree VC support is (unecessarily) mapped onto per-directory
>>    semantics.
>> 2. `vc-ignore` is a quick and dirty afterthought, which does not
>>    properly model per-tree use cases.
>> 3. The rationale for my implementation of `vc-hg-ignore` covers
>>    several use cases, which should be considered for a proper design.
> Could you please elaborate on these 3 points?  I've read the rest of
> your message at least twice, and I still don't think I understand your
> POV on these 3 issues well enough to make up my mind about them.  I
> appreciate the overview of the history of these features -- it might
> very well help us understand your POV -- but the POV itself is not
> clearly presented, AFAICT, so I at least couldn't find a path from the
> history of these features to the problems you allude to.  And the
> history alone cannot explain the problems in the current design,
> because there's nothing wrong in evolving a design per se.
>
> IOW, I'm OK with discussing design issues of ignoring files in VC, but
> please let's begin by presenting clear and detailed descriptions of
> the problems in the current design, and why you think the current
> design cannot easily support valid use cases with modern VCSes.
>
> Thanks.

Well, I will try again.

Opening a can of worms
======================

Just to be clear that my original intention was only a simple bugfix
...

In order to phase out `dvc`, I tried to use `vc-dir-ignore` in a
Mercurial repository. At that time (2019-08-24) I was still using
emacs 24.5 and 25.4.1 ubuntu standard packages.

First I ran into an unrelated bug with marking files in `vc-dir-mode`.

So, I cloned the current emacs repository to see, whether that bug was
already fixed. Since it was not, I decided to report it (see bug
#37182 https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37182).

While implementing `vc-hg-ignore`, I found the implementations of
`vc--add-line`, `vc--remove-regexp` faulty, which lead to bug #37185
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37185.

Now I started using Emacs snapshot packages (v26.3).

After finishing `vc-hg-ignore` in bug Bug #37189
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37189, I checked the
implementation of other VCSs that I still use (CVS, SVN, GIT) and
found them non-functional:

- CVS bug #37215 https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37215

- SVN bug #37214 https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37214,
  bug #37216 https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37216

- GIT does not have a specialized backend version, so I circled back
  to `vc-default-ignore`, which ironically would have worked for CVS,
  but not for per-tree semantics. The resulting patch in bug #37217
  https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37217 implements
  minimal per-tree functionality, but should be specialized for Git
  ignore glob features (anchored glob patterns).

There was no provision in `vc-dir-ignore` to ignore all marked files,
which begat bug #37240
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37240.

The discussion about my adhoc implementation of `vc-hg-ignore` now
ends up in a design analysis. How funny :-). This discussion and the
latest discovery (bug #39380
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=39380) finally sparked
the historic research into the `vc` package.

The research does show, that the initial implementation started out
with per-directory semantics. Although the `vc-default-ignore`
function was designed to cover Git/Hg in an adhoc manner, it was never
properly implemented.

The amount of bugs further shows, that the ignore feature was never
properly integrated.

Per-directory vs. per-tree
==========================

Per-directory and per-tree ignore contexts are sufficently polymorphic
such that per-directory algorithms can easily be implemented on top of
per-tree algorithms. However, implementing per-tree behavior on top of
per-directory does not generally work.

While there is no **difference between filepath and file pattern** in
a per-directory glob context, the difference **becomes vital in a
per-tree context**. The difference is further acerbated, when patterns
are encoded as regular expressions.

Since there is no possible automatic detection of filepath vs. file
pattern, the current API of `vc-ignore` is insufficient.

Ignoring existing files leads to patterns with the least amount of
ambiguity, which requires regular expressions to be escaped. Whereas
user supplied patterns should not be encoded at all.

The use cases 1, 2, 3 below should therefore be covered by
`vc-ignore-file`, use case 4 should be covered by `vc-ignore-pattern`.

`vc-ignore` should be expanded to accept an optional flag `as-is`
which determines processing of the `file` argument. This flag can be
used by `vc-ignore-pattern` to specify its use case. `vc-ignore-file`
can be implemented as alias for `vc-ignore`.

Per-directory
-------------

Per-directory `vc-ignore` algorithm for CVS:

- One ignore file per directory.
- Each line is a file glob pattern relative to the directory of the ignore file.
- Pattern scope is limited to the directory.

Use cases

1. Ignore a file in `vc-dir-mode`.

   Write basename of absolute filepath into ignore file for directory.

2. Ignore marked files in `vc-dir-mode`.

   Write basename of relative filepath into ignore file for directory.

3. Ignore absolute/relative filepath with `vc-ignore`.

   Write basename of filepath into ignore file for directory.

4. Ignore pattern with `vc-ignore`.

   Write pattern unaltered into ignore file of directory.

Per-tree
--------

Per-tree algorithms for Mercurial (Git is somewhat similiar, but only
supports glob patterns):

- Single ignore file at the root of the repository
- alternative syntax: regexp or glob
- Each line is a pattern for a filepath relative to the directory of the ignore 
file.
- Pattern scope extends into sub directories

1. Ignore a file in `vc-dir-mode`.

   Convert absolute filepath to filepath relative to `vc-root-dir` and write 
into ignore file.

2. Ignore marked files in `vc-dir-mode`.

   Convert filepath relative to default directory to filepath relative to 
`vc-root-dir` and write into ignore file.

3. Ignore absolute/relative filepath with `vc-ignore`.

   Convert filepath to filepath relative to `vc-root-dir` and write into ignore 
file.

4. Ignore pattern with `vc-ignore`.

   Write pattern unaltered into ignore file.

Faulty implementation of `vc-default-ignore`
============================================

Both sets of algorithms and use cases are currently covered by
`vc-ignore` being called interactively or by `vc-dir-ignore` from
`vc-dir-mode`.

The reference backend implementation is `vc-default-ignore`. Although
the documentation already specifies what the function should do in a
per-tree context, the original implementation strangely follows the
description of `vc-ignore` which is still per-directory.

Consequently the original implementation of `vc-default-ignore` only
works for per-directory VCSs. This would have been sufficient for CVS,
but ironically CVS has a specialized function `vc-cvs-ignore`,
imported from `pcvs`, which does not work correctly .

While the frontend command `vc-ignore` does not have inherent
per-directory or per-tree semantics, the description should use a
per-tree context, similar to `vc-default-ignore`.

A. Original vc-default-ignore
=============================

.. code-block:: elisp

   (defun vc-default-ignore (backend file &optional directory remove)
     "Ignore FILE under the VCS of DIRECTORY (default is `default-directory').
   FILE is a file wildcard, relative to the root directory of DIRECTORY.
   When called from Lisp code, if DIRECTORY is non-nil, the
   repository to use will be deduced by DIRECTORY; if REMOVE is
   non-nil, remove FILE from ignored files.
   Argument BACKEND is the backend you are using."
     (let ((ignore
            (vc-call-backend backend 'find-ignore-file (or directory 
default-directory)))
           (pattern (file-relative-name
                     (expand-file-name file) (file-name-directory file))))
       (if remove
           (vc--remove-regexp pattern ignore)
         (vc--add-line pattern ignore))))

B. Revised vc-default-ignore
============================

.. code-block:: elisp

   (defun vc-default-ignore (backend file &optional directory remove)
     "Ignore FILE under the VCS of DIRECTORY (default is `default-directory').
   FILE is a wildcard specification, either relative to
   DIRECTORY or absolute.
   When called from Lisp code, if DIRECTORY is non-nil, the
   repository to use will be deduced by DIRECTORY; if REMOVE is
   non-nil, remove FILE from ignored files.
   Argument BACKEND is the backend you are using."
     (let ((ignore
            (vc-call-backend backend 'find-ignore-file (or directory 
default-directory)))
           file-path root-dir pattern)
       (setq file-path (expand-file-name file directory))
       (setq root-dir (file-name-directory ignore))
       (when (not (string= (substring file-path 0 (length root-dir)) root-dir))
         (error "Ignore spec %s is not below project root %s" file-path 
root-dir))
       (setq pattern (substring file-path (length root-dir)))
       (if remove
           (vc--remove-regexp (concat "^" (regexp-quote pattern ) 
"\\(\n\\|$\\)") ignore)
         (vc--add-line pattern ignore))))

C. vc-ignore
============

.. code-block:: elisp

   (defun vc-ignore (file &optional directory remove)
     "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.

   DIRECTORY defaults to `default-directory' and is used to
   determine the responsible VC backend.

   When called interactively, prompt for a FILE to ignore, unless a
   prefix argument is given, in which case prompt for a file FILE to
   remove from the list of ignored files."
     (interactive
      (list
       (if (not current-prefix-arg)
           (read-file-name "File to ignore: ")
         (completing-read
          "File to remove: "
          (vc-call-backend
           (or (vc-responsible-backend default-directory)
               (error "Unknown backend"))
           'ignore-completion-table default-directory)))
       nil current-prefix-arg))
     (let* ((directory (or directory default-directory))
            (backend (or (vc-responsible-backend default-directory)
                         (error "Unknown backend"))))
       (vc-call-backend backend 'ignore file directory remove)))