bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Wolfgang Scherer
Subject: bug#37189: 25.4.1: vc-hg-ignore implementation is missing
Date: Wed, 5 Feb 2020 06:18:08 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.1

>> Per-directory vs. per-tree
>> ==========================
>
> ...I don't feel I understand better your view of what you describe as
> fundamental design problems in this area.  I'm saying that after
> reading your description at least twice.  You make many assertions and
> tell what these features _should_ do, but there's little explanation
> of _why_ these assertions are true or why the commands should do this
> or that.

Usually a design does not explain why things should be the way they are 
specified. Also, having extracted the initial code for `vc` ignore files in the 
previous post, I did not think it was necessary to do it again. I also assume 
familiarity with the code in question and the mechanisms of ignore files for at 
least CVS, Git and Hg.

I think I have sufficiently shown that the original implemention of the ignore 
feature did not work correctly in a single case. If you disagree, just use 
emacs 26.3 and try to ignore files in `vc-dir-mode` or by calling `vc-ignore` 
interactively in some CVS, Git or Hg repositories. CVS will not work at all. As 
for Git, Hg, if you ignore anything below the root directory it will not work 
correctly. Even the entries on the root level are not properly anchored and/or 
escaped. I am just making this argument to show that an API change does not 
break anything, since it did not work to begin with.

I have further tried to fix the problem with the minimum amount of effort, 
which worked for some of the accepted corrections, but it did not work for 
`vc-hg-ignore`, which is unfortunately my main use case. However, in order to 
fix `vc-hg-ignore` without a hack (recognition of regular expressions as 
implemented in the patch of this bug report), it is necessary to expand the API 
of `vc-ignore` and the corresponding backend functions by at least adding a 
flag that indicates, whether the file parameter should be treated as a filepath 
or a pattern.

Other topics like per-subtree ignore files have not even been mentioned yet. 
But they do not need API changes and can therefore be implemented later as a 
change request for `vc-find-ignore-file`. Hint: the assumption that .gitignore 
or .hgignore are always relative to `vc-root-dir` is simply false.

Since I cannot comfortably explain the matters in the Email text format, I have 
prepared a HTML version at 
http://sw-amt.ws/README-emacs-vc-ignore-feature.html#vc-ignore-api-change-request.
 The description contains a discussion of wildcard pattern escaping/anchoring 
and two detailed (non-exhaustive) tables with use cases showing the 
implementation to be incorrect (even with the latest changes). The tables show, 
that the function `vc-ignore` should be able to produce different output for 
the same FILE / DIRECTORY input (e.g. "fil?.ext" nil), when

- submitted as filepath by `vc-dir-ignore`
- submitted as a wildcard specification by an interactive invocation of 
`vc-ignore`.

However, producing different output for the same input is inherently impossible 
for fully deterministic functions. Once that is understood, it becomes obvious, 
that the correct results can only be achieved, when the backend functions are 
supplied with additional information to determine whether a single file should 
be ignored (escaping the filepath properly) or whether a pattern should be 
added verbatim without escaping anything.

Since the ignore file sub-system did not even work at all before the last 
series of changes and the current request for `vc-hg-ignore` cannot be fixed 
cleanly without API changes and also because the ignore sub-system would still 
be broken, my proposition is to fix the design. Hence the draft design with 
some identified use cases for your convenience. Feel free to ignore it and try 
to fix the problem another way.

This is really all that is necessary to understand the problem. I have answered 
some further arguments, but they are really just repetitions.
>   Even the first step of your argument: that there's a
> fundamental difference between per-directory and per-tree ignore specs
> is left with almost no description of these fundamental differences.
> I don't think I have a clear idea of why you think so.

I am not sure, why such a description is needed. Although per-directory ignore 
files are simply a maximally reduced special case of per-tree ignore files, 
where all relative filepathes are identical to basenames, it is still not 
sufficient to implement only per-directory ignore files, since per-tree ignore 
files have additional requirements that are completely uncovered by the 
per-directory semantics of the original implementation. That is the same type 
of difference as for real numbers and complex numbers. Whether you want to call 
it "fundamental" or not, does not invalidate it. Real numbers are a limited 
subset of complex numbers and not the other way around.

The full specification how per-directory and per-tree ignore files work can be 
found in the man pages. If necessary, I can summarize those, but I would rather 
not.

Here is one fundamental difference between per-directory and per-tree ignore 
specs, which also shows that due to the conceptual lack of anchoring, 
per-directory algorithms cannot emulate per-tree algorithms. However, a 
per-tree algorithm with null anchors can perfectly well emulate per-directory 
algorithms.

- Per-directory ignore specifications do not need to be anchored.
- Per-tree ignore specifications must be properly anchored.

>> 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.
>
> This enumerates use cases and states their requirements, but doesn't
> explain those requirements.  I don't think I understand why sometimes
> the file specs need to be relative and sometimes absolute, because you
> just say so without any explanations.

Assuming familiarity with the code, it should be clear that `vc-dir-ignore` 
calls `vc-ignore` with the result from (vc-dir-current-file). In this case FILE 
is an absolute filepath.

For a set of marked files `vc-dir-ignore` calls `vc-ignore` repeatedly with the 
result from (vc-dir-fileinfo->name filearg). In this case FILE is a filepath 
relative to default-directory.

When calling `vc-ignore` interactively, the current directory is inserted at 
the prompt. In this case the FILE parameter is an absolute path unless the 
default directory is deleted first, then it becomes an undefined pattern.

When calling `vc-ignore` from lisp code any combination of FILE/DIRECTORY can 
be specified and should be processed sensibly.

>> 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.
>
> See, I don't even understand why you say vc-ignore is per-directory.

The **description** of `vc-ignore` is per-directory, while the **description** 
of `vc-default-ignore` is per-tree. That is obvious if you read both 
descriptions and do not confuse description and implementation.

The **implementation** of `vc-ignore` is inherently neither per-directory nor 
per-tree, because it only calls the appropriate backend function.

The original implementation of `vc-default-ignore` is per-directory only and 
therefore contradicts its description. The incorrect behavior is explained in 
bug #37217. While I partially corrected the implementation of 
`vc-default-ignore`, it is unsatisfactory in regard to a good abstraction of 
escaping and anchoring (incl. syntax detection), which would eliminate the need 
for specialized backend functions `vc-hg-ignore` and `vc-git-ignore`.

> That it accepts a directory as its argument doesn't yet mean it cannot
> support all of its subdirectories, recursively, which will make it
> per-tree.

The difference betwen per-directory and per-tree has nothing to do with a 
DIRECTORY parameter or recursion.
Per-tree is not a recursive per-directory algorithm and cannot be implemented 
as such.
However, per-directory is an automatic by-product of a per-tree algorithm (e.g. 
ignoring some file in the root directory).

A search of the `vc` source files for `per-\(dir\|tree\)` gives a hint that the 
concept of per-directory and per-tree is VCS specific:

vc-hooks.el\0110-(defcustom vc-handled-backends '(RCS CVS SVN SCCS SRC Bzr Git 
Hg Mtn)
vc-hooks.el\0111: ;; RCS, CVS, SVN, SCCS, and SRC come first because they are 
per-dir
vc-hooks.el\0112: ;; rather than per-tree. RCS comes first because of the 
multibackend

In regard to ignore file specifications, per-directory means that the file 
specification is a plain basename without directory parts, the scope of the 
specification is only the corresponding directory, no anchoring is necessary.
Per-tree means that the file specification may include directory pathes and 
unanchored file specifications are applied in all sub-directories, while 
anchored specs are only applied at the specified level.

Again, this is much better described in the respective man pages.

>   I'm probably missing something, but what?

The FILE parameter of `vc-ignore` is considered equivalent to a wildcard. This 
equivalence is false.

The design change is an additional parameter to `vc-ignore` and optionally new 
functions `vc-ignore-file`, `vc-ignore-pattern` to clarify the intended use.

Proposed shortcuts are:

`C-x v F` => `vc-ignore-file`
`C-x v G` => `vc-ignore-pattern`

in `vc-dir-mode`:

`F` => `vc-dir-ignore` => `vc-ignore-file`
`G` => `vc-ignore-pattern`.

> Once again, feel free to tell the discussion about the design is not
> useful and should be abandoned.

Are you saying that it is not useful to finally have a working ignore feature 
after more than 20 years?

The `vc` ignore feature implementation has not worked and still does not work 
correctly. But before adding some more adhoc patches without a final 
resolution, it is usually better to consider all problems and specify a working 
design.

Anyway, my contribution was not intended to be a final design but rather an 
incomplete collection of design criteria for the responsible maintainer. I also 
assumed, that the responsible maintainer has a general knowledge of the 
concepts for ignore files and specific knowledge of the `vc` ignore feature 
code.

>   I'm okay with discussing just the
> specific bug you have.

Unfortunately, the specific bug I encounter with `vc-hg-ignore` cannot be fixed 
without the API change.

>   But if we are to continue talking about design
> aspects, may I suggest that you explain your opinions more than you
> did until now?

The rationale for the design changes can be found in previous posts and other 
bug reports, where the problems with the implementation of `vc-hg-ignore` 
become apparent.

Among the differences between per-tree and per-directory semantics is the 
anchoring mechanism which is needed for per-tree and is unnecessary for 
per-directory ignore specifications.

Try to understand that although ignore specifications are always wildcard 
specifications, a generic frontend function like `vc-ignore` cannot ask other 
generic functions like `vc-dir-ignore` for properly escaped wildcard 
specifications, since the correct escape and anchoring mechanisms are unknown 
to frontend functions. Asking interactively for a raw pattern is feasible, 
since a user probably knows the appropriate syntax.

A frontend function should not be burdened with actually normalizing, escaping 
and anchoring filepath specifications. The high-level flag signaling either raw 
input or unescaped and unanchored filepath is sufficient and more readable.






reply via email to

[Prev in Thread] Current Thread [Next in Thread]