Hi Dmitry,

Am 25.12.19 um 01:16 schrieb Dmitry Gutov:
> On 29.08.2019 18:52, Wolfgang Scherer wrote:
>> +  "Ignore FILE of DIRECTORY (default is `default-directory').
>
> IF this function needs a docstring at all (which is not obvious since it should be following vc-ignore), I think I'd rather this followed the latter's docstring. Where the clarification about the default is not in the first sentence.

vc-hg-ignore needs a docstring, since it exhibits specific behavior for the backend (e.g. glob/regex syntax), which is not and cannot be covered by the rather generic vc-ignore docstring.

Most of the description in  vc-ignore is not even applicable to the backend specific functions, e.g. there is no interactive functionality in the backend function and no backend is determined.

If a backend does not provide a vc-BACKEND-ignore function, vc-default-ignore is invoked. This function has a docstring of its own (which contains the clarification about the default in the first sentence). According to your argument, this function should also have no docstring of its own. However, I cannot see how both docstrings are equivalent.

vc-hg-ignore is modeled after vc-default-ignore, the docstring was copied from vc-default-ignore and modified to fit the implementation. I have modified the docstring further to match other backend specific ignore functions.

>
> Also, I think saying "Ignore FILE under DIRECTORY" would be better, if you intend to add this particular semantics to relative names.
All other ignore functions say "Ignore FILE under VCS". I have modified the docstring accordingly.
>
>> +Otherwise, FILE is either relative to DIRECTORY or absolute. FILE
>> +is converted to a path relative to the project root of DIRECTORY.
>
> Isn't it a bit odd that vc-ignore's docstring doesn't specify this distinction, and yet we're trying to implement it in vc-hg-ignore?

No. vc-ignore's semantic roots stem from SCCS/RCS/CVS/subversion with single directory ignore files. The ignore files for those VCS only contain file patterns.

For a VCS with subdirectories and a single ignore file at the root of the repository (Git, Mercurial), rooted and non-rooted expressions are handled differently and the single directory ignore file paradigm fails.

>
> Do you have a particular reason for that?
Yes. It is the standard use case for ignoring something from vc-dir mode. As mentioned, the vc-default-ignore produces utterly useless results for Git and Mercurial in vc-dir mode.
>
>> +                (concat pattern (and (file-directory-p file-path) "*"))))))
>
> I think it needs to asterisks for the glob to become recursive. At least according to https://stackoverflow.com/a/255094/615245.
No, according to https://stackoverflow.com/a/255094/2127439, all of "bin/", "bin/*", "bin/**" are equivalent under glob syntax. I also tested this specifically and extensivlely.
>
>> +    (lambda (_c)
>> +      (cons _c (concat "\\" (char-to-string _c)))))
>
> Our convention says that an argument whose name starts with underscore is unused. That's not the case here, so it shouldn't be named like that.
OK.
>
>> +    (lambda (_c)
>> +      (or (cdr (assq _c vc-hg--py-regexp-special-chars))
>> +          (char-to-string _c))))
>
> Same.

OK.

New patch attached.