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

[Wolfgang Scherer]

Am 11.02.20 um 18:32 schrieb Eli Zaretskii:
>> Cc: dgutov@yandex.ru, 37189@debbugs.gnu.org
>> From: Wolfgang Scherer <Wolfgang.Scherer@gmx.de>
>> Date: Tue, 11 Feb 2020 02:45:41 +0100
>>
>> I suggest, we stop talking about the use case, where `vc-ignore`
>> is called interactively, I will let you have your opinion,
>> although I strongly disagree
> If you strongly disagree, then why do you give up so easily on your
> opinion?  I didn't yet make up my mind, so maybe you will be able to
> convince me.
I do not actually give up:-). I just think it is easier to
convince you that the "file path" use case is already supposed to
be supported by Emacs. In that case we are first talking about
fixing the broken functionality, before deciding how to implement
it exactly. Otherwise we will get lost in a "pattern" discussion,
which has no point, since I don't oppose it.
>
>> and the unsolved problem of interactively locating the correct
>> ignore file makes it ultimately hilarious:
>>
>> A command that locates an ignore file, but can only do so, if the
>> default-directory is already the one containing the ignore
>> file (always true for SRC, CVS, SVN)
> Is locating the ignore file a separate issue?  AFAICT, we currently
> always look for the ignore file in the repository root, is that right?
> If so, are you proposing a new feature here, where we would support
> ignore files in subdirectories?

I am not (yet) talking about a new feature, but about the "file path"
use case in *vc-dir-mode* for per-directory VCSs (CVS, SVN, SRC).

With a "pattern only" implementation it will become an issue, when
dealing with a recursive SVN repository in *vc-dir-mode*. Invoking
*vc-ignore* in a *vc-dir-mode* buffer will always use the current root
of the SVN tree. However, when a pattern shall be added to the ignore
spec in a subdirectory, a new buffer must be opened with the
subdirectory as root.

E.g., in this *vc-dir-mode* buffer it is not possible to use *vc-ignore*
in "pattern" mode:for adding an ignore pattern for SVN to `sub/`
or for SVN or CVS in `sub/sub2/`:

``` {.sourceCode .text}
VC backend : SVN
Working dir: /the/top

                  .svn
                  sub/CVS
   edited         sub/.cvsignore
                  sub/sub2/CVS
                  sub/sub2/RCS
   edited         sub/sub2/.cvsignore
```

In case of nested repositories with different VC backends (e.g. in
directories `sub` and `sub/sub2` above), it may even become necessary to
invoke *vc-dir* with a prefix argument to specify the appropriate VC
manually.

``` {.sourceCode .elisp}
>>> (let ((default-directory "/the/top/")) (vc-responsible-backend "./"))
SVN

>>> (let ((default-directory "/the/top/sub/")) (vc-responsible-backend "./"))
CVS

>>> (let ((default-directory "/the/top/sub/sub2/")) (vc-responsible-backend 
>>> "./"))
RCS
```

It is obvious, that it is quicker and safer in that case to just open
the ignore file directly and edit it. Ignoring a pattern in `sub` or
`sub/sub2/` leaves only the command
``svn propset svn:ignore pattern dir``, which overwrites previous
patterns making the whole operation complicated. This is a very good
point for implementing the "file path" use case.

With the "file path" algorithm none of these problems arise, because the
VC is determined by the current directory (SVN in `/the/top/`) and the
search for the ignore file starts in the appropriate subdirectory with
the correct backend.

``` {.sourceCode .elisp}
>>> (let ((default-directory "/the/top")) (vc-responsible-backend "./"))
SVN

>>> (let ((default-directory "/the/top")) (vc-responsible-backend "sub/"))
SVN

>>> (let ((default-directory "/the/top")) (vc-responsible-backend "sub/sub2/"))
SVN
```

So, if the "file path" use case remains broken or is removed entirely,
it will be a an issue which ultimately discourages users to apply the
function at all. While I appreciate the added functionality of the
"pattern" use case (which is most useful for modern VCs with a single
ignore file at the root), I would most certainly not bother with it,
should it be the only option.

I have been using the command line and Emacs **vc** package for more
than 25 years for almost all my VC needs, whenever support for a VC
became available. However, ignoring files I always had to do with other
packages, e.g. **pcl-cvs**, **dvc**, which all support the "file path"
use case, none of them support the "pattern" use case. Sigh, it is
finally really nice to phase out other vc packages.
>> Since the escaping must also be correct, I'm probably better off
>> locating and editing the ignore file manually. Inserting the output
>> of "ls -1" and editing away - as I sometimes do - is much more
>> convenient than calling `vc-ignore` multiple times and repeating
>> more complex editing of an absolute file path each time in the
>> minibuffer.
> vc-ignore is not only for adding ignored files/patterns, it is also
> for deleting entries in the ignore files.  And that functionality
> definitely needs the users to type the patterns exactly as they are in
> the ignore file.
I was making a case of editing the ignore file directly, so the REMOVE
case is obviously also covered.

But yes and no for the typing. The "file path" use case is covered,
since the escaped file path is removed. If the file path was added
automatically before, then that is exactly what is in the ignore
file. No typing required. Since there are many ways to specify the
same pattern, the manual approach is really less desirable than
automatic escaping.

This is where the completion list of active patterns comes in handy,
as those are the actual patterns that can be removed. As I said, I do
appreciate the "pattern" use case, it just needs a little bit of
enhancement to become more useful.
>> However, there is a use case that you keep avoiding to comment
>> on, namely pressing "G" in `vc-dir-mode`, which calls
>> `vc-dir-ignore`, which in turn calls `vc-ignore`
>> **programmatically** with an absolute file path. It has been
>> officially supported by Emacs since 2013. I did not invent it and
>> I did not alter its API.
>>
>> Since there is no interactive prompt whatsoever for a user to
>>
>> 1. properly construct an escaped pattern and
>> 2. specify the directory, where the search for the ignore file
>>    should start,
>>
>> the function called to ignore the file must consequently escape
>> and anchor it properly, just like any command that uses
>> `shell-quote-args` because the use case requires it and the
>> purpose of the argument is well-known.
>>
>> How do you plan to support this use case?
> Since vc-dir-ignore computes the file name(s) to add to the ignore
> file, it indeed will need to escape all the special characters in file
> names it will add, before it invokes vc-ignore.  You are right here.
In order to know, what part of the filename must be used, it is
necessary to know the location of the ignore file, which is only known
by the backend.

So, please have a look at *vc-default-get-ignore-file-and-pattern*,
which does exactly that, it returns the correct ignore file, and the
escaped pattern.

You want this function to be called from *vc-dir-ignore*, and then only
use the pattern for *vc-ignore* and voila: **it is impossible for
vc-ignore to find the correct ignore file** when it is in a
subdirectory, as I have shown previously.

Setting *default-directory* to the directory of the ignore file results
in *vc-ignore* being unable to determine the correct backend, as shown
above.

Since *vc-dir-ignore* already knows the backend, it can call the backend
function *vc-default-ignore* directly. And *vc-default-ignore* in turn
will call *vc-default-get-ignore-file-and-pattern* **again**, since it
must **again** find the ignore file.

Don't you think, that this is a waste of resources and possible cause of
many errors?

My implementation uses a single path for both *vc-ignore-file* and
*vc-ignore-pattern* eliminating that quite nicely. *vc-ignore-file* is
used for both interactive specification of files as well as
*vc-dir-mode* and *dired-mode* file sets, eliminating the function
*vc-dir-ignore*.
>> According to your opinion `vc-ignore` can only be used
>> **interactively** by a learned expert user, who wants to make
>> ignore file administration even more complicated by editing
>> random ignore files without any feedback.
> Given that vc-ignore can also delete a an entry from the ignore file,
> and given that we supported patterns there (albeit with bugs) for some
> time, I don't see how we can change that in a radical way like you
> suggest.
I think you misunderstand me. The pattern REMOVE functionality is
supported exactly the same way as before. There is no change besides the
missing bugs and the complete support of all backends (that *is*
probably a bit radical).

-   If you call *vc-ignore* programatically, you get the same result
    as before.
-   If you invoke `C-x v G`, you get the same result as before.
-   If you invoke `G` in *vc-dir-mode*, you get the same result
    as before.
-   **Only** if you invoke `M-x vc-ignore RET`, **you get an error**.
    This is intended to provide the incentive for noticing the
    modifications and also to have better doc strings.

    If you don't like it, **we can keep the old behavior** and make
    *vc-ignore-pattern* an alias for *vc-ignore*. In that case there is
    **no user visible change at all** for the "pattern" use case.

Just the previously broken functionality of ignoring "file pathes" (also
including the REMOVE feature) is now properly supported.

The only real API change is an additional flag AS-IS for *vc-ignore*.
>
>> So it is the wrong candidate for a noob user of `vc-dir-mode`
>> who just wants to ignore the selected files literally without
>> worrying about ignore files and escaping.
> I agree with you wrt vc-dir-ignore.
Good.
>
>> They also expect a visual feedback, that the operation had the
>> desired effect, as they have come to expect from all the other
>> commands in `vc-dir-mode`.
> AFAICT, the command does provide feedback.  Or maybe I misunderstand
> what feedback you had in mind.

vc-dir-ignore currently does not provide the normal feedback, I only just now 
put it in:

      (vc-dir-resynch-file file)

When ignoring files in vc-dir-mode, the status is now immediately checked and 
updated to "ignored".

>> If you are just worried, that somebody will use `vc-ignore` and
>> expect the old broken behavior
> The old behavior of vc-ignore was not broken for interactive
> invocations.  It was broken (in rare cases) for invocations from
> vc-dir-ignore, and that can IMO be fixed without affecting user-facing
> behavior.  So I see no backward-incompatible changes here.
Sorry, not *rare* but **all** cases! The invocation from vc-dir-ignore
placed an **absolute file path** into the ignore file. That is
**always** wrong.  The commands I fixed recently, now do no longer
fully support the "pattern" case anymore. The whole point of the
proposed implementation is, that both use cases, "pattern" and "file
path" are supported correctly and to the full extent.
>>> This concept is similar to what we do in commands such as "M-x grep",
>>> where we expect the users to escape special characters in the
>>> command-line arguments to be passed to Grep the program via the shell.
>> That is not at all comparable, since there is no use case "search
>> for file path in a bunch of files".
> My point is that Grep patterns can include characters special for the
> shell, but we never escape them ourselves, we rely on the user to
> escape them as needed.

The pattern is not prompted for separately! There is no way, that the
pattern argument for the *grep* command could be reliably parsed and
quoted. If the pattern was read separately from the rest of the
arguments, it would be quite alright to quote it for the shell.

If the vc-ignore supplied pattern was to be passed on to a shell
command, it would of course have to be properly quoted for the shell.
But nobody expects the user to do that.

In fact, I wrote myself a *grep-find* derived command to search for tags
in files, which prompts the user for an unquoted regular expression of
symbols. That expression is then augmented with the tag delimiters, it
is further quoted for the shell and inserted at the proper location for
the *grep-find* command which is then presented to the user for editing
the options. And why? Because I know in advance what the exact structure
of the pattern will be and that it has to be quoted for the shell ;-).

Provided with a pattern of "LOG\|BGX", the command prompt looks like this
(slightly edited):

``` {.sourceCode .sh}
/usr/bin/find . -name '*' -type f -exec /bin/grep --color -nH --null -e \
    \\\(\\\(\\\(\\\(\\\)\|\:\\\)\\\(LOG\\\|BGX\\\)\\\)\:\|\\\) \{\} +
```