unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
@ 2019-08-26  0:21 Wolfgang Scherer
       [not found] ` <handler.37189.B.15667808855126.ack@debbugs.gnu.org>
  0 siblings, 1 reply; 66+ messages in thread
From: Wolfgang Scherer @ 2019-08-26  0:21 UTC (permalink / raw)
  To: 37189

Using `vc-ignore' in a *vc-dir* buffer for mercurial does not provide a correct entry in `.hgignore.'.

The function `vc-hg-ignore'' below autodetects the correct syntax and adds a correctly quoted entry to `.hgignore'.

Feel free to incorporate it in `vc-hg.el'.


(defvar vc-hg--py-regexp-special-chars
  (mapcar
   (function
    (lambda (_c)
      (cons _c (concat "\\" (char-to-string _c)))))
   (append "()[]{}?*+-|^$\\.&~# \t\n\r\v\f" nil))
  "Characters that have special meaning in Python regular expressions.")

(defun vc-hg--py-regexp-quote (string)
  "Return a Python regexp string which matches exactly STRING and nothing else.
Ported from Python v3.7"
  (mapconcat
   (function
    (lambda (_c)
      (or (cdr (assq _c vc-hg--py-regexp-special-chars))
          (char-to-string _c))))
   string ""))

(defvar vc-hg-ignore-detect-wildcard "[*^$]"
  "Regular expresssion to detect wildcards in an ignored file specification.")

(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.

If FILE matches the regular expression
`vc-hg-ignore-detect-wildcard', it is appended to .hgignore as
is. Otherwise, FILE is escaped/expanded according to the active
syntax in .hgignore. If the syntax is `regexp', FILE is quoted as
anchored literal Python regexp and if FILE is a directory, the
trailing `$' is omitted.  Otherwise, if the syntax is `glob',
FILE is used unquoted and if FILE is a directory, a `*' is
appended.

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."
  (let ((ignore (vc-hg-find-ignore-file (or directory default-directory)))
        (pattern file)
        root-dir file-path syntax)
    (unless (string-match vc-hg-ignore-detect-wildcard pattern)
      (setq root-dir (file-name-directory ignore))
      (setq file-path (expand-file-name file directory))
      (setq pattern (substring file-path (length root-dir)))
      (save-match-data
        (with-current-buffer (find-file-noselect ignore)
          (goto-char (point-max))
          (setq syntax
                (if (re-search-backward "^ *syntax: *\\(regexp\\|glob\\)$" nil t)
                    (match-string 1)
                  "regexp")))
        (setq pattern
              (if (string= syntax "regexp")
                  (concat "^" (vc-hg--py-regexp-quote pattern)
                          (and (not (file-directory-p file-path)) "$"))
                (concat pattern (and (file-directory-p file-path) "*"))))))
    (if remove
        (vc--remove-regexp pattern ignore)
      (vc--add-line pattern ignore))))


^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: Acknowledgement (25.4.1: vc-hg-ignore implementation is missing)
       [not found] ` <handler.37189.B.15667808855126.ack@debbugs.gnu.org>
@ 2019-08-26 23:25   ` Wolfgang Scherer
  2019-08-27  7:45     ` Eli Zaretskii
  0 siblings, 1 reply; 66+ messages in thread
From: Wolfgang Scherer @ 2019-08-26 23:25 UTC (permalink / raw)
  To: 37189

[-- Attachment #1: Type: text/plain, Size: 37 bytes --]

Patch with commit message attached.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Provides-vc-hg-ignore-to-make-vc-ignore-work-correct.patch --]
[-- Type: text/x-patch; name="0001-Provides-vc-hg-ignore-to-make-vc-ignore-work-correct.patch", Size: 3708 bytes --]

From 7a238036ba08e3b1359e06839ea38b944bd0f2db Mon Sep 17 00:00:00 2001
From: Wolfgang Scherer <wolfgang.scherer@gmx.de>
Date: Tue, 27 Aug 2019 01:22:46 +0200
Subject: [PATCH] Provides vc-hg-ignore to make vc-ignore work correctly

* lisp/vc/vc-hg.el: (vc-hg-ignore) Ignore file of directory.  Add
filepath relative to directory of Mercurial .hgignore file.  The
filepath is quoted according to the active ignore syntax (Bug#37189).
(vc-hg--py-regexp-quote) Quote string as regexp to match exactly
string.

Copyright-paperwork-exempt: yes
---
 lisp/vc/vc-hg.el | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/lisp/vc/vc-hg.el b/lisp/vc/vc-hg.el
index f287adf..f4bddd2 100644
--- a/lisp/vc/vc-hg.el
+++ b/lisp/vc/vc-hg.el
@@ -1153,6 +1153,46 @@ REV is ignored."
   (expand-file-name ".hgignore"
 		    (vc-hg-root file)))

+(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.
+
+If FILE matches the regular expression
+`vc-hg-ignore-detect-wildcard', it is appended to .hgignore as
+is. Otherwise, FILE is escaped/expanded according to the active
+syntax in .hgignore. If the syntax is `regexp', FILE is quoted as
+anchored literal Python regexp and if FILE is a directory, the
+trailing `$' is omitted.  Otherwise, if the syntax is `glob',
+FILE is used unquoted and if FILE is a directory, a `*' is
+appended.
+
+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."
+  (let ((ignore (vc-hg-find-ignore-file (or directory default-directory)))
+        (pattern file)
+        root-dir file-path syntax)
+    (unless (string-match vc-hg-ignore-detect-wildcard pattern)
+      (setq root-dir (file-name-directory ignore))
+      (setq file-path (expand-file-name file directory))
+      (setq pattern (substring file-path (length root-dir)))
+      (save-match-data
+        (with-current-buffer (find-file-noselect ignore)
+          (goto-char (point-max))
+          (setq syntax
+                (if (re-search-backward "^ *syntax: *\\(regexp\\|glob\\)$" nil t)
+                    (match-string 1)
+                  "regexp")))
+        (setq pattern
+              (if (string= syntax "regexp")
+                  (concat "^" (vc-hg--py-regexp-quote pattern)
+                          (and (not (file-directory-p file-path)) "$"))
+                (concat pattern (and (file-directory-p file-path) "*"))))))
+    (if remove
+        (vc--remove-regexp pattern ignore)
+      (vc--add-line pattern ignore))))
+
 ;; Modeled after the similar function in vc-bzr.el
 (defun vc-hg-checkout (file &optional rev)
   "Retrieve a revision of FILE.
@@ -1451,6 +1491,24 @@ This function differs from vc-do-command in that it invokes
 (defun vc-hg-root (file)
   (vc-find-root file ".hg"))

+(defvar vc-hg--py-regexp-special-chars
+  (mapcar
+   (function
+    (lambda (_c)
+      (cons _c (concat "\\" (char-to-string _c)))))
+   (append "()[]{}?*+-|^$\\.&~# \t\n\r\v\f" nil))
+  "Characters that have special meaning in Python regular expressions.")
+
+(defun vc-hg--py-regexp-quote (string)
+  "Return a Python regexp string which matches exactly STRING and nothing else.
+Ported from Python v3.7"
+  (mapconcat
+   (function
+    (lambda (_c)
+      (or (cdr (assq _c vc-hg--py-regexp-special-chars))
+          (char-to-string _c))))
+   string ""))
+
 (provide 'vc-hg)

 ;;; vc-hg.el ends here
--
2.7.4


^ permalink raw reply related	[flat|nested] 66+ messages in thread

* bug#37189: Acknowledgement (25.4.1: vc-hg-ignore implementation is missing)
  2019-08-26 23:25   ` bug#37189: Acknowledgement (25.4.1: vc-hg-ignore implementation is missing) Wolfgang Scherer
@ 2019-08-27  7:45     ` Eli Zaretskii
  2019-08-28  1:46       ` bug#37189: *** GMX Spamverdacht *** " Wolfgang Scherer
  0 siblings, 1 reply; 66+ messages in thread
From: Eli Zaretskii @ 2019-08-27  7:45 UTC (permalink / raw)
  To: Wolfgang Scherer; +Cc: 37189

> From: Wolfgang Scherer <Wolfgang.Scherer@gmx.de>
> Date: Tue, 27 Aug 2019 01:25:59 +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".

> +If FILE matches the regular expression
> +`vc-hg-ignore-detect-wildcard', it is appended to .hgignore as
> +is. Otherwise, FILE is escaped/expanded according to the active
> +syntax in .hgignore. If the syntax is `regexp', FILE is quoted as
> +anchored literal Python regexp and if FILE is a directory, the
> +trailing `$' is omitted.  Otherwise, if the syntax is `glob',
> +FILE is used unquoted and if FILE is a directory, a `*' is
> +appended.

Our convention is to leave 2 spaces between sentences in comments and
doc strings.

> +When called from Lisp code, if DIRECTORY is non-nil, the

"When called from Lisp" implies that this function can be called in
some other way, which is generally correct only with commands.  But
this function is not a command, so I'm unsure what this means here.

Thanks.





^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: *** GMX Spamverdacht *** Re: bug#37189: Acknowledgement (25.4.1: vc-hg-ignore implementation is missing)
  2019-08-27  7:45     ` Eli Zaretskii
@ 2019-08-28  1:46       ` Wolfgang Scherer
  2019-08-28  6:16         ` Eli Zaretskii
  2019-08-29  0:38         ` Wolfgang Scherer
  0 siblings, 2 replies; 66+ messages in thread
From: Wolfgang Scherer @ 2019-08-28  1:46 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 37189

[-- Attachment #1: Type: text/plain, Size: 2565 bytes --]

Am 27.08.19 um 09:45 schrieb Eli Zaretskii:
>> From: Wolfgang Scherer <Wolfgang.Scherer@gmx.de>
>> Date: Tue, 27 Aug 2019 01:25:59 +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':

vc.el:1420:FILE is a file wildcard, relative to the root directory of DIRECTORY.
vc-svn.el:356:FILE is a file wildcard, relative to the root directory of DIRECTORY.

Also `top level' only appears twice in the `vc-' files:

vc-bzr.el:1070: A merge has been performed.\nA commit from the top-level directory
vc-cvs.el:904:;; at the top level for CVS.

while the phrases `root', `root directory', `repository root'
appear 23 times in strings and comments alone.

The sentence is actually utterly false for `vc-default-ignore',
since FILE is usually an absolute file path when called from a
*vc-dir* buffer. With relative paths the code is still wrong and
ends up as plain basename for the pattern.

Since the implementation of `vc-hg-ignore' corresponds to the
actual situation, I have changed the wording to reflect that using
the term `project root'.

>
>> +If FILE matches the regular expression
>> +`vc-hg-ignore-detect-wildcard', it is appended to .hgignore as
>> +is. Otherwise, FILE is escaped/expanded according to the active
>> +syntax in .hgignore. If the syntax is `regexp', FILE is quoted as
>> +anchored literal Python regexp and if FILE is a directory, the
>> +trailing `$' is omitted.  Otherwise, if the syntax is `glob',
>> +FILE is used unquoted and if FILE is a directory, a `*' is
>> +appended.
> Our convention is to leave 2 spaces between sentences in comments and
> doc strings.
Done.
>> +When called from Lisp code, if DIRECTORY is non-nil, the
> "When called from Lisp" implies that this function can be called in
> some other way, which is generally correct only with commands.  But
> this function is not a command, so I'm unsure what this means here.
I agree, and I fixed the remove option, so it actually does what
it says ;-)
> Thanks.

You're welcome.

Oops, I had forgotten to include the variable
`vc-hg-ignore-detect-wildcard'. So here is an overhauled patch.



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Provides-vc-hg-ignore-to-make-vc-ignore-work-correct.patch --]
[-- Type: text/x-patch; name="0001-Provides-vc-hg-ignore-to-make-vc-ignore-work-correct.patch", Size: 3817 bytes --]

From ec04366f6f9ba813b66d62396b1cfa7f2a865a25 Mon Sep 17 00:00:00 2001
From: Wolfgang Scherer <wolfgang.scherer@gmx.de>
Date: Wed, 28 Aug 2019 03:42:22 +0200
Subject: [PATCH] Provides vc-hg-ignore to make vc-ignore work correctly

* lisp/vc/vc-hg.el: (vc-hg-ignore) Ignore file of directory.  Add
filepath relative to directory of Mercurial .hgignore file.  The
filepath is quoted according to the active ignore syntax (Bug#37189).
(vc-hg--py-regexp-quote) Quote string as regexp to match exactly
string.
---
 lisp/vc/vc-hg.el | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/lisp/vc/vc-hg.el b/lisp/vc/vc-hg.el
index f287adf..aad0bd3 100644
--- a/lisp/vc/vc-hg.el
+++ b/lisp/vc/vc-hg.el
@@ -1153,6 +1153,48 @@ REV is ignored."
   (expand-file-name ".hgignore"
 		    (vc-hg-root file)))
 
+(defvar vc-hg-ignore-detect-wildcard "[*^$]"
+  "Regular expresssion to detect wildcards in an ignored file
+  specification.")
+
+(defun vc-hg-ignore (file &optional directory remove)
+  "Ignore FILE of DIRECTORY (default is `default-directory').
+If FILE matches the regular expression
+`vc-hg-ignore-detect-wildcard', it is appended to .hgignore
+unmodified.
+Otherwise, FILE is assumed to be relative to DIRECTORY and is
+converted to a path relative to the project root of DIRECTORY.
+It is then further escaped/expanded according to the active
+syntax in .hgignore.  If the syntax is `regexp', FILE is quoted
+as anchored literal Python regexp and if FILE is a directory, the
+trailing `$' is omitted.  Otherwise, if the syntax is `glob',
+FILE is used unquoted and if FILE is a directory, a `*' is
+appended.
+If REMOVE is non-nil, remove the pattern derived from FILE from
+ignored files."
+  (let ((ignore (vc-hg-find-ignore-file (or directory default-directory)))
+        (pattern file)
+        root-dir file-path syntax)
+    (unless (string-match vc-hg-ignore-detect-wildcard pattern)
+      (setq root-dir (file-name-directory ignore))
+      (setq file-path (expand-file-name file directory))
+      (setq pattern (substring file-path (length root-dir)))
+      (save-match-data
+        (with-current-buffer (find-file-noselect ignore)
+          (goto-char (point-max))
+          (setq syntax
+                (if (re-search-backward "^ *syntax: *\\(regexp\\|glob\\)$" nil t)
+                    (match-string 1)
+                  "regexp")))
+        (setq pattern
+              (if (string= syntax "regexp")
+                  (concat "^" (vc-hg--py-regexp-quote pattern)
+                          (and (not (file-directory-p file-path)) "$"))
+                (concat pattern (and (file-directory-p file-path) "*"))))))
+    (if remove
+        (vc--remove-regexp (concat (regexp-quote pattern ) "\n?") ignore)
+      (vc--add-line pattern ignore))))
+
 ;; Modeled after the similar function in vc-bzr.el
 (defun vc-hg-checkout (file &optional rev)
   "Retrieve a revision of FILE.
@@ -1451,6 +1493,24 @@ This function differs from vc-do-command in that it invokes
 (defun vc-hg-root (file)
   (vc-find-root file ".hg"))
 
+(defvar vc-hg--py-regexp-special-chars
+  (mapcar
+   (function
+    (lambda (_c)
+      (cons _c (concat "\\" (char-to-string _c)))))
+   (append "()[]{}?*+-|^$\\.&~# \t\n\r\v\f" nil))
+  "Characters that have special meaning in Python regular expressions.")
+
+(defun vc-hg--py-regexp-quote (string)
+  "Return a Python regexp string which matches exactly STRING and nothing else.
+Ported from Python v3.7"
+  (mapconcat
+   (function
+    (lambda (_c)
+      (or (cdr (assq _c vc-hg--py-regexp-special-chars))
+          (char-to-string _c))))
+   string ""))
+
 (provide 'vc-hg)
 
 ;;; vc-hg.el ends here
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 66+ messages in thread

* bug#37189: *** GMX Spamverdacht *** Re: bug#37189: Acknowledgement (25.4.1: vc-hg-ignore implementation is missing)
  2019-08-28  1:46       ` bug#37189: *** GMX Spamverdacht *** " Wolfgang Scherer
@ 2019-08-28  6:16         ` Eli Zaretskii
  2019-08-29  1:23           ` bug#37189: 25.4.1: vc-hg-ignore implementation is missing Wolfgang Scherer
  2019-08-29  0:38         ` Wolfgang Scherer
  1 sibling, 1 reply; 66+ messages in thread
From: Eli Zaretskii @ 2019-08-28  6:16 UTC (permalink / raw)
  To: Wolfgang Scherer; +Cc: 37189

> 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.





^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2019-08-28  1:46       ` bug#37189: *** GMX Spamverdacht *** " Wolfgang Scherer
  2019-08-28  6:16         ` Eli Zaretskii
@ 2019-08-29  0:38         ` Wolfgang Scherer
  2019-08-29 15:52           ` Wolfgang Scherer
  1 sibling, 1 reply; 66+ messages in thread
From: Wolfgang Scherer @ 2019-08-29  0:38 UTC (permalink / raw)
  To: 37189

[-- Attachment #1: Type: text/plain, Size: 61 bytes --]

Small fix for option remove. New version of patch attached.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Provides-vc-hg-ignore-to-make-vc-ignore-work-correct.patch --]
[-- Type: text/x-patch; name="0001-Provides-vc-hg-ignore-to-make-vc-ignore-work-correct.patch", Size: 3982 bytes --]

From 4f95f6de3ec429dc2db632514b43e3773e048505 Mon Sep 17 00:00:00 2001
From: Wolfgang Scherer <wolfgang.scherer@gmx.de>
Date: Thu, 29 Aug 2019 02:34:17 +0200
Subject: [PATCH] Provides vc-hg-ignore to make vc-ignore work correctly

* lisp/vc/vc-hg.el: (vc-hg-ignore) Ignore file of directory.  Add
filepath relative to directory of Mercurial .hgignore file.  The
filepath is quoted according to the active ignore syntax (Bug#37189).
(vc-hg--py-regexp-quote) Quote string as regexp to match exactly
string.
---
 lisp/vc/vc-hg.el | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/lisp/vc/vc-hg.el b/lisp/vc/vc-hg.el
index f287adf..cf51833 100644
--- a/lisp/vc/vc-hg.el
+++ b/lisp/vc/vc-hg.el
@@ -1153,6 +1153,50 @@ REV is ignored."
   (expand-file-name ".hgignore"
 		    (vc-hg-root file)))

+(defvar vc-hg-ignore-detect-wildcard "[*^$]"
+  "Regular expresssion to detect wildcards in an ignored file
+  specification.")
+
+(defun vc-hg-ignore (file &optional directory remove)
+  "Ignore FILE of DIRECTORY (default is `default-directory').
+If FILE matches the regular expression
+`vc-hg-ignore-detect-wildcard', it is appended to .hgignore
+unmodified.
+Otherwise, FILE is either relative to DIRECTORY or absolute. FILE
+is converted to a path relative to the project root of DIRECTORY.
+It is then further escaped/expanded according to the active
+syntax in .hgignore.  If the syntax is `regexp', FILE is quoted
+as anchored literal Python regexp and if FILE is a directory, the
+trailing `$' is omitted.  Otherwise, if the syntax is `glob',
+FILE is used unquoted and if FILE is a directory, a `*' is
+appended.
+If REMOVE is non-nil, remove the pattern derived from FILE from
+ignored files."
+  (let ((ignore (vc-hg-find-ignore-file (or directory default-directory)))
+        (pattern file)
+        root-dir file-path syntax)
+    (unless (string-match vc-hg-ignore-detect-wildcard 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)))
+      (save-match-data
+        (with-current-buffer (find-file-noselect ignore)
+          (goto-char (point-max))
+          (setq syntax
+                (if (re-search-backward "^ *syntax: *\\(regexp\\|glob\\)$" nil t)
+                    (match-string 1)
+                  "regexp")))
+        (setq pattern
+              (if (string= syntax "regexp")
+                  (concat "^" (vc-hg--py-regexp-quote pattern)
+                          (and (not (file-directory-p file-path)) "$"))
+                (concat pattern (and (file-directory-p file-path) "*"))))))
+    (if remove
+        (vc--remove-regexp (concat "^" (regexp-quote pattern ) "\n?") ignore)
+      (vc--add-line pattern ignore))))
+
 ;; Modeled after the similar function in vc-bzr.el
 (defun vc-hg-checkout (file &optional rev)
   "Retrieve a revision of FILE.
@@ -1451,6 +1495,24 @@ This function differs from vc-do-command in that it invokes
 (defun vc-hg-root (file)
   (vc-find-root file ".hg"))

+(defvar vc-hg--py-regexp-special-chars
+  (mapcar
+   (function
+    (lambda (_c)
+      (cons _c (concat "\\" (char-to-string _c)))))
+   (append "()[]{}?*+-|^$\\.&~# \t\n\r\v\f" nil))
+  "Characters that have special meaning in Python regular expressions.")
+
+(defun vc-hg--py-regexp-quote (string)
+  "Return a Python regexp string which matches exactly STRING and nothing else.
+Ported from Python v3.7"
+  (mapconcat
+   (function
+    (lambda (_c)
+      (or (cdr (assq _c vc-hg--py-regexp-special-chars))
+          (char-to-string _c))))
+   string ""))
+
 (provide 'vc-hg)

 ;;; vc-hg.el ends here
--
2.7.4


^ permalink raw reply related	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2019-08-28  6:16         ` Eli Zaretskii
@ 2019-08-29  1:23           ` Wolfgang Scherer
  0 siblings, 0 replies; 66+ messages in thread
From: Wolfgang Scherer @ 2019-08-29  1:23 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 37189


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.







^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2019-08-29  0:38         ` Wolfgang Scherer
@ 2019-08-29 15:52           ` Wolfgang Scherer
  2019-12-25  0:16             ` Dmitry Gutov
  0 siblings, 1 reply; 66+ messages in thread
From: Wolfgang Scherer @ 2019-08-29 15:52 UTC (permalink / raw)
  To: 37189

[-- Attachment #1: Type: text/plain, Size: 88 bytes --]

A unit test showed, that the removal regexp was faulty. New version of patch attached.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Provide-vc-hg-ignore-to-make-vc-ignore-work-correctl.patch --]
[-- Type: text/x-patch; name="0001-Provide-vc-hg-ignore-to-make-vc-ignore-work-correctl.patch", Size: 3990 bytes --]

From 4fb25acadedc81a6d654bed89e816906aff07178 Mon Sep 17 00:00:00 2001
From: Wolfgang Scherer <wolfgang.scherer@gmx.de>
Date: Thu, 29 Aug 2019 17:49:54 +0200
Subject: [PATCH] Provide vc-hg-ignore to make vc-ignore work correctly

* lisp/vc/vc-hg.el: (vc-hg-ignore) Ignore file of directory.  Add
filepath relative to directory of Mercurial .hgignore file.  The
filepath is quoted according to the active ignore syntax (Bug#37189).
(vc-hg--py-regexp-quote) Quote string as regexp to match exactly
string.
---
 lisp/vc/vc-hg.el | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/lisp/vc/vc-hg.el b/lisp/vc/vc-hg.el
index f287adf..9cbd9d4 100644
--- a/lisp/vc/vc-hg.el
+++ b/lisp/vc/vc-hg.el
@@ -1153,6 +1153,50 @@ REV is ignored."
   (expand-file-name ".hgignore"
 		    (vc-hg-root file)))

+(defvar vc-hg-ignore-detect-wildcard "[*^$]"
+  "Regular expresssion to detect wildcards in an ignored file
+  specification.")
+
+(defun vc-hg-ignore (file &optional directory remove)
+  "Ignore FILE of DIRECTORY (default is `default-directory').
+If FILE matches the regular expression
+`vc-hg-ignore-detect-wildcard', it is appended to .hgignore
+unmodified.
+Otherwise, FILE is either relative to DIRECTORY or absolute. FILE
+is converted to a path relative to the project root of DIRECTORY.
+It is then further escaped/expanded according to the active
+syntax in .hgignore.  If the syntax is `regexp', FILE is quoted
+as anchored literal Python regexp and if FILE is a directory, the
+trailing `$' is omitted.  Otherwise, if the syntax is `glob',
+FILE is used unquoted and if FILE is a directory, a `*' is
+appended.
+If REMOVE is non-nil, remove the pattern derived from FILE from
+ignored files."
+  (let ((ignore (vc-hg-find-ignore-file (or directory default-directory)))
+        (pattern file)
+        root-dir file-path syntax)
+    (unless (string-match vc-hg-ignore-detect-wildcard 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)))
+      (save-match-data
+        (with-current-buffer (find-file-noselect ignore)
+          (goto-char (point-max))
+          (setq syntax
+                (if (re-search-backward "^ *syntax: *\\(regexp\\|glob\\)$" nil t)
+                    (match-string 1)
+                  "regexp")))
+        (setq pattern
+              (if (string= syntax "regexp")
+                  (concat "^" (vc-hg--py-regexp-quote pattern)
+                          (and (not (file-directory-p file-path)) "$"))
+                (concat pattern (and (file-directory-p file-path) "*"))))))
+    (if remove
+        (vc--remove-regexp (concat "^" (regexp-quote pattern ) "\\(\n\\|$\\)") ignore)
+      (vc--add-line pattern ignore))))
+
 ;; Modeled after the similar function in vc-bzr.el
 (defun vc-hg-checkout (file &optional rev)
   "Retrieve a revision of FILE.
@@ -1451,6 +1495,24 @@ This function differs from vc-do-command in that it invokes
 (defun vc-hg-root (file)
   (vc-find-root file ".hg"))

+(defvar vc-hg--py-regexp-special-chars
+  (mapcar
+   (function
+    (lambda (_c)
+      (cons _c (concat "\\" (char-to-string _c)))))
+   (append "()[]{}?*+-|^$\\.&~# \t\n\r\v\f" nil))
+  "Characters that have special meaning in Python regular expressions.")
+
+(defun vc-hg--py-regexp-quote (string)
+  "Return a Python regexp string which matches exactly STRING and nothing else.
+Ported from Python v3.7"
+  (mapconcat
+   (function
+    (lambda (_c)
+      (or (cdr (assq _c vc-hg--py-regexp-special-chars))
+          (char-to-string _c))))
+   string ""))
+
 (provide 'vc-hg)

 ;;; vc-hg.el ends here
--
2.7.4


^ permalink raw reply related	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2019-08-29 15:52           ` Wolfgang Scherer
@ 2019-12-25  0:16             ` Dmitry Gutov
  2020-01-05  3:46               ` Wolfgang Scherer
  0 siblings, 1 reply; 66+ messages in thread
From: Dmitry Gutov @ 2019-12-25  0:16 UTC (permalink / raw)
  To: Wolfgang Scherer, 37189

Hi Wolfgang,

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.

Also, I think saying "Ignore FILE under DIRECTORY" would be better, if 
you intend to add this particular semantics to relative names.

> +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?

Do you have a particular reason for that?

> +                (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.

> +    (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.

> +    (lambda (_c)
> +      (or (cdr (assq _c vc-hg--py-regexp-special-chars))
> +          (char-to-string _c))))

Same.





^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2019-12-25  0:16             ` Dmitry Gutov
@ 2020-01-05  3:46               ` Wolfgang Scherer
  2020-01-05  8:58                 ` Andreas Schwab
  2020-01-14  1:14                 ` Dmitry Gutov
  0 siblings, 2 replies; 66+ messages in thread
From: Wolfgang Scherer @ 2020-01-05  3:46 UTC (permalink / raw)
  To: Dmitry Gutov, 37189

[-- Attachment #1: Type: text/plain, Size: 3365 bytes --]

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.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Provide-vc-hg-ignore-to-make-vc-ignore-work-correctl.patch --]
[-- Type: text/x-patch; name="0001-Provide-vc-hg-ignore-to-make-vc-ignore-work-correctl.patch", Size: 16393 bytes --]

From 81672e2909bca4ea4299301bbf4450e392b3a4f8 Mon Sep 17 00:00:00 2001
From: Wolfgang Scherer <wolfgang.scherer@gmx.de>
Date: Sun, 5 Jan 2020 04:29:28 +0100
Subject: [PATCH] Provide vc-hg-ignore to make vc-ignore work correctly

* lisp/vc/vc-hg.el: (vc-hg-ignore) Ignore file of directory.  Add
filepath relative to directory of Mercurial .hgignore file.  The
filepath is quoted according to the active ignore syntax (Bug#37189).
(vc-hg--py-regexp-quote) Quote string as regexp to match exactly
string.
---
 lisp/vc/vc-hg.el | 229 +++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 145 insertions(+), 84 deletions(-)

diff --git a/lisp/vc/vc-hg.el b/lisp/vc/vc-hg.el
index eac9a6f..db84a28 100644
--- a/lisp/vc/vc-hg.el
+++ b/lisp/vc/vc-hg.el
@@ -142,9 +142,9 @@ If nil, use the value of `vc-diff-switches'.  If t, use no switches."
 If nil, use the value of `vc-annotate-switches'.  If t, use no
 switches."
   :type '(choice (const :tag "Unspecified" nil)
-		 (const :tag "None" t)
-		 (string :tag "Argument String")
-		 (repeat :tag "Argument List" :value ("") string))
+                 (const :tag "None" t)
+                 (string :tag "Argument String")
+                 (repeat :tag "Argument List" :value ("") string))
   :version "25.1"
   :group 'vc-hg)

@@ -152,8 +152,8 @@ switches."
   "String or list of strings specifying switches for hg revert
 under VC."
   :type '(choice (const :tag "None" nil)
-		 (string :tag "Argument String")
-		 (repeat :tag "Argument List" :value ("") string))
+                 (string :tag "Argument String")
+                 (repeat :tag "Argument List" :value ("") string))
   :version "27.1"
   :group 'vc-hg)

@@ -233,35 +233,35 @@ highlighting the Log View buffer."
             (setq status
                   (condition-case nil
                       ;; Ignore all errors.
-		      (let ((process-environment
-			     ;; Avoid localization of messages so we
-			     ;; can parse the output.  Disable pager.
-			     (append
-			      (list "TERM=dumb" "LANGUAGE=C" "HGPLAIN=1")
-			      process-environment)))
-			(process-file
-			 vc-hg-program nil t nil
+                      (let ((process-environment
+                             ;; Avoid localization of messages so we
+                             ;; can parse the output.  Disable pager.
+                             (append
+                              (list "TERM=dumb" "LANGUAGE=C" "HGPLAIN=1")
+                              process-environment)))
+                        (process-file
+                         vc-hg-program nil t nil
                          "--config" "ui.report_untrusted=0"
-			 "--config" "alias.status=status"
-			 "--config" "defaults.status="
-			 "status" "-A" (file-relative-name file)))
+                         "--config" "alias.status=status"
+                         "--config" "defaults.status="
+                         "status" "-A" (file-relative-name file)))
                     ;; Some problem happened.  E.g. We can't find an `hg'
                     ;; executable.
                     (error nil)))))))
     (when (and (eq 0 status)
-	       (> (length out) 0)
-	       (null (string-match ".*: No such file or directory$" out)))
+               (> (length out) 0)
+               (null (string-match ".*: No such file or directory$" out)))
       (let ((state (aref out 0)))
-	(cond
-	 ((eq state ?=) 'up-to-date)
-	 ((eq state ?A) 'added)
-	 ((eq state ?M) 'edited)
-	 ((eq state ?I) 'ignored)
-	 ((eq state ?R) 'removed)
-	 ((eq state ?!) 'missing)
-	 ((eq state ??) 'unregistered)
-	 ((eq state ?C) 'up-to-date) ;; Older mercurial versions use this.
-	 (t 'up-to-date))))))
+        (cond
+         ((eq state ?=) 'up-to-date)
+         ((eq state ?A) 'added)
+         ((eq state ?M) 'edited)
+         ((eq state ?I) 'ignored)
+         ((eq state ?R) 'removed)
+         ((eq state ?!) 'missing)
+         ((eq state ??) 'unregistered)
+         ((eq state ?C) 'up-to-date) ;; Older mercurial versions use this.
+         (t 'up-to-date))))))

 (defun vc-hg-working-revision (file)
   "Hg-specific version of `vc-working-revision'."
@@ -429,19 +429,19 @@ If LIMIT is non-nil, show no more than this many entries."
   ;; read-only.
   (let ((inhibit-read-only t))
     (with-current-buffer
-	buffer
+        buffer
       (apply 'vc-hg-command buffer 'async files "log"
-	     (nconc
-	      (when start-revision (list (format "-r%s:0" start-revision)))
-	      (when limit (list "-l" (format "%s" limit)))
+             (nconc
+              (when start-revision (list (format "-r%s:0" start-revision)))
+              (when limit (list "-l" (format "%s" limit)))
               (when (eq vc-log-view-type 'with-diff)
                 (list "-p"))
-	      (if shortlog
+              (if shortlog
                   `(,@(if vc-hg-log-graph '("--graph"))
                     "--template"
                     ,(car vc-hg-root-log-format))
                 `("--template" ,vc-hg-log-format))
-	      vc-hg-log-switches)))))
+              vc-hg-log-switches)))))

 (defvar log-view-message-re)
 (defvar log-view-file-re)
@@ -455,35 +455,35 @@ If LIMIT is non-nil, show no more than this many entries."
   (set (make-local-variable 'log-view-per-file-logs) nil)
   (set (make-local-variable 'log-view-message-re)
        (if (eq vc-log-view-type 'short)
-	   (cadr vc-hg-root-log-format)
+           (cadr vc-hg-root-log-format)
          "^changeset:[ \t]*\\([0-9]+\\):\\(.+\\)"))
   (set (make-local-variable 'tab-width) 2)
   ;; Allow expanding short log entries
   (when (eq vc-log-view-type 'short)
     (setq truncate-lines t)
     (set (make-local-variable 'log-view-expanded-log-entry-function)
-	 'vc-hg-expanded-log-entry))
+         'vc-hg-expanded-log-entry))
   (set (make-local-variable 'log-view-font-lock-keywords)
        (if (eq vc-log-view-type 'short)
-	   (list (cons (nth 1 vc-hg-root-log-format)
-		       (nth 2 vc-hg-root-log-format)))
-	 (append
-	  log-view-font-lock-keywords
-	  '(
-	    ;; Handle the case:
-	    ;; user: FirstName LastName <foo@bar>
-	    ("^user:[ \t]+\\([^<(]+?\\)[ \t]*[(<]\\([A-Za-z0-9_.+-]+@[A-Za-z0-9_.-]+\\)[>)]"
-	     (1 'change-log-name)
-	     (2 'change-log-email))
-	    ;; Handle the cases:
-	    ;; user: foo@bar
-	    ;; and
-	    ;; user: foo
-	    ("^user:[ \t]+\\([A-Za-z0-9_.+-]+\\(?:@[A-Za-z0-9_.-]+\\)?\\)"
-	     (1 'change-log-email))
-	    ("^date: \\(.+\\)" (1 'change-log-date))
-	    ("^tag: +\\([^ ]+\\)$" (1 'highlight))
-	    ("^summary:[ \t]+\\(.+\\)" (1 'log-view-message)))))))
+           (list (cons (nth 1 vc-hg-root-log-format)
+                       (nth 2 vc-hg-root-log-format)))
+         (append
+          log-view-font-lock-keywords
+          '(
+            ;; Handle the case:
+            ;; user: FirstName LastName <foo@bar>
+            ("^user:[ \t]+\\([^<(]+?\\)[ \t]*[(<]\\([A-Za-z0-9_.+-]+@[A-Za-z0-9_.-]+\\)[>)]"
+             (1 'change-log-name)
+             (2 'change-log-email))
+            ;; Handle the cases:
+            ;; user: foo@bar
+            ;; and
+            ;; user: foo
+            ("^user:[ \t]+\\([A-Za-z0-9_.+-]+\\(?:@[A-Za-z0-9_.-]+\\)?\\)"
+             (1 'change-log-email))
+            ("^date: \\(.+\\)" (1 'change-log-date))
+            ("^tag: +\\([^ ]+\\)$" (1 'highlight))
+            ("^summary:[ \t]+\\(.+\\)" (1 'log-view-message)))))))

 (autoload 'vc-switches "vc")

@@ -545,7 +545,7 @@ This requires hg 4.4 or later, for the \"-L\" option of \"hg log\"."
     (when (and (not oldvers) newvers)
       (setq oldvers working))
     (apply #'vc-hg-command
-	   (or buffer "*vc-diff*")
+           (or buffer "*vc-diff*")
            nil ; bug#21969
            files "diff"
            (append
@@ -584,7 +584,7 @@ This requires hg 4.4 or later, for the \"-L\" option of \"hg log\"."
   "Execute \"hg annotate\" on FILE, inserting the contents in BUFFER.
 Optional arg REVISION is a revision to annotate from."
   (apply #'vc-hg-command buffer 0 file "annotate" "-dq" "-n"
-	 (append (vc-switches 'hg 'annotate)
+         (append (vc-switches 'hg 'annotate)
                  (if revision (list (concat "-r" revision))))))

 (declare-function vc-annotate-convert-time "vc-annotate" (&optional time))
@@ -1102,9 +1102,9 @@ hg binary."
                (let ((vc-hg-size (nth 2 dirstate-entry))
                      (vc-hg-mtime (nth 3 dirstate-entry))
                      (fs-size (file-attribute-size stat))
-		     (fs-mtime (time-convert
-				(file-attribute-modification-time stat)
-				'integer)))
+                     (fs-mtime (time-convert
+                                (file-attribute-modification-time stat)
+                                'integer)))
                  (if (and (eql vc-hg-size fs-size) (eql vc-hg-mtime fs-mtime))
                      'up-to-date
                    'edited)))
@@ -1210,7 +1210,51 @@ REV is ignored."
 (defun vc-hg-find-ignore-file (file)
   "Return the root directory of the repository of FILE."
   (expand-file-name ".hgignore"
-		    (vc-hg-root file)))
+                    (vc-hg-root file)))
+
+(defvar vc-hg-ignore-detect-wildcard "[*^$]"
+  "Regular expresssion to detect wildcards in an ignored file
+  specification.")
+
+(defun vc-hg-ignore (file &optional directory remove)
+  "Ignore FILE under Mercurial.
+FILE is either absolute or relative to DIRECTORY (default is
+`default-directory').
+If FILE matches the regular expression
+`vc-hg-ignore-detect-wildcard', it is processed unmodified.
+Otherwise, FILE is converted to a path relative to the project
+root of DIRECTORY.  It is then further escaped/expanded according
+to the active syntax in .hgignore.  If the syntax is `regexp',
+FILE is quoted as anchored literal Python regexp and if FILE is a
+directory, the trailing `$' is omitted.  Otherwise, if the syntax
+is `glob', FILE is used unquoted and if FILE is a directory, a
+`*' is appended.
+If REMOVE is non-nil, remove the pattern derived from FILE from
+ignored files."
+  (let ((ignore (vc-hg-find-ignore-file (or directory default-directory)))
+        (pattern file)
+        root-dir file-path syntax)
+    (unless (string-match vc-hg-ignore-detect-wildcard 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)))
+      (save-match-data
+        (with-current-buffer (find-file-noselect ignore)
+          (goto-char (point-max))
+          (setq syntax
+                (if (re-search-backward "^ *syntax: *\\(regexp\\|glob\\)$" nil t)
+                    (match-string 1)
+                  "regexp")))
+        (setq pattern
+              (if (string= syntax "regexp")
+                  (concat "^" (vc-hg--py-regexp-quote pattern)
+                          (and (not (file-directory-p file-path)) "$"))
+                (concat pattern (and (file-directory-p file-path) "*"))))))
+    (if remove
+        (vc--remove-regexp (concat "^" (regexp-quote pattern ) "\\(\n\\|$\\)") ignore)
+      (vc--add-line pattern ignore))))

 ;; Modeled after the similar function in vc-bzr.el
 (defun vc-hg-checkout (file &optional rev)
@@ -1250,7 +1294,6 @@ REV is the revision to check out into WORKFILE."
     (add-hook 'after-save-hook 'vc-hg-resolve-when-done nil t)
     (vc-message-unresolved-conflicts buffer-file-name)))

-
 ;; Modeled after the similar function in vc-bzr.el
 (defun vc-hg-revert (file &optional contents-done)
   (unless contents-done
@@ -1386,12 +1429,12 @@ REV is the revision to check out into WORKFILE."
 (defun vc-hg-log-incoming (buffer remote-location)
   (vc-setup-buffer buffer)
   (vc-hg-command buffer 1 nil "incoming" "-n" (unless (string= remote-location "")
-						remote-location)))
+                                                remote-location)))

 (defun vc-hg-log-outgoing (buffer remote-location)
   (vc-setup-buffer buffer)
   (vc-hg-command buffer 1 nil "outgoing" "-n" (unless (string= remote-location "")
-						remote-location)))
+                                                remote-location)))

 (defvar vc-hg-error-regexp-alist
   '(("^M \\(.+\\)" 1 nil nil 0))
@@ -1413,30 +1456,30 @@ commands, which only operated on marked files."
     ;; `pull'/`push' VC actions were implemented.
     ;; The following is for backwards compatibility.
     (if (and obsolete (setq marked-list (log-view-get-marked)))
-	(apply #'vc-hg-command
-	       nil 0 nil
-	       command
-	       (apply 'nconc
-		      (mapcar (lambda (arg) (list "-r" arg)) marked-list)))
+        (apply #'vc-hg-command
+               nil 0 nil
+               command
+               (apply 'nconc
+                      (mapcar (lambda (arg) (list "-r" arg)) marked-list)))
       (let* ((root (vc-hg-root default-directory))
-	     (buffer (format "*vc-hg : %s*" (expand-file-name root)))
-	      ;; Disable pager.
+             (buffer (format "*vc-hg : %s*" (expand-file-name root)))
+              ;; Disable pager.
              (process-environment (cons "HGPLAIN=1" process-environment))
-	     (hg-program vc-hg-program)
-	     args)
-	;; If necessary, prompt for the exact command.
+             (hg-program vc-hg-program)
+             args)
+        ;; If necessary, prompt for the exact command.
         ;; TODO if pushing, prompt if no default push location - cf bzr.
-	(when prompt
-	  (setq args (split-string
-		      (read-shell-command
+        (when prompt
+          (setq args (split-string
+                      (read-shell-command
                        (format "Hg %s command: " command)
                        (format "%s %s" hg-program command)
                        'vc-hg-history)
-		      " " t))
-	  (setq hg-program (car  args)
-		command    (cadr args)
-		args       (cddr args)))
-	(apply 'vc-do-async-command buffer root hg-program command args)
+                      " " t))
+          (setq hg-program (car  args)
+                command    (cadr args)
+                args       (cddr args)))
+        (apply 'vc-do-async-command buffer root hg-program command args)
         (with-current-buffer buffer
           (vc-run-delayed
             (dolist (cmd post-processing)
@@ -1458,7 +1501,7 @@ commands, which only operated on marked files."
                         (list compile-command nil
                               (lambda (_name-of-mode) buffer)
                               nil))))
-	(vc-set-async-update buffer)))))
+        (vc-set-async-update buffer)))))

 (defun vc-hg-pull (prompt)
   "Issue a Mercurial pull command.
@@ -1494,7 +1537,7 @@ call \"hg push -r REVS\" to push the specified revisions REVS."
   "Prompt for revision and merge it into working directory.
 This runs the command \"hg merge\"."
   (let* ((root (vc-hg-root default-directory))
-	 (buffer (format "*vc-hg : %s*" (expand-file-name root)))
+         (buffer (format "*vc-hg : %s*" (expand-file-name root)))
          ;; Disable pager.
          (process-environment (cons "HGPLAIN=1" process-environment))
          (branch (vc-read-revision "Revision to merge: ")))
@@ -1522,6 +1565,24 @@ This function differs from vc-do-command in that it invokes
 (defun vc-hg-root (file)
   (vc-find-root file ".hg"))

+(defvar vc-hg--py-regexp-special-chars
+  (mapcar
+   (function
+    (lambda (ch)
+      (cons ch (concat "\\" (char-to-string ch)))))
+   (append "()[]{}?*+-|^$\\.&~# \t\n\r\v\f" nil))
+  "Characters that have special meaning in Python regular expressions.")
+
+(defun vc-hg--py-regexp-quote (string)
+  "Return a Python regexp string which matches exactly STRING and nothing else.
+Ported from Python v3.7"
+  (mapconcat
+   (function
+    (lambda (ch)
+      (or (cdr (assq ch vc-hg--py-regexp-special-chars))
+          (char-to-string ch))))
+   string ""))
+
 (provide 'vc-hg)

 ;;; vc-hg.el ends here
--
2.7.4


^ permalink raw reply related	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-01-05  3:46               ` Wolfgang Scherer
@ 2020-01-05  8:58                 ` Andreas Schwab
  2020-01-05 17:25                   ` Wolfgang Scherer
  2020-01-14  1:14                 ` Dmitry Gutov
  1 sibling, 1 reply; 66+ messages in thread
From: Andreas Schwab @ 2020-01-05  8:58 UTC (permalink / raw)
  To: Wolfgang Scherer; +Cc: 37189, Dmitry Gutov

On Jan 05 2020, Wolfgang Scherer wrote:

> diff --git a/lisp/vc/vc-hg.el b/lisp/vc/vc-hg.el
> index eac9a6f..db84a28 100644
> --- a/lisp/vc/vc-hg.el
> +++ b/lisp/vc/vc-hg.el
> @@ -142,9 +142,9 @@ If nil, use the value of `vc-diff-switches'.  If t, use no switches."
>  If nil, use the value of `vc-annotate-switches'.  If t, use no
>  switches."
>    :type '(choice (const :tag "Unspecified" nil)
> -		 (const :tag "None" t)
> -		 (string :tag "Argument String")
> -		 (repeat :tag "Argument List" :value ("") string))
> +                 (const :tag "None" t)
> +                 (string :tag "Argument String")
> +                 (repeat :tag "Argument List" :value ("") string))
>    :version "25.1"
>    :group 'vc-hg)
>

That contains a lot of spurious whitespace differences.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."





^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-01-05  8:58                 ` Andreas Schwab
@ 2020-01-05 17:25                   ` Wolfgang Scherer
  0 siblings, 0 replies; 66+ messages in thread
From: Wolfgang Scherer @ 2020-01-05 17:25 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: 37189, Dmitry Gutov

[-- Attachment #1: Type: text/plain, Size: 229 bytes --]

Am 05.01.20 um 09:58 schrieb Andreas Schwab:
> That contains a lot of spurious whitespace differences.

You are right, that is not intended. Sometimes muscle memory just
invokes my whitespace cleaner.

Corrected patch attached.


[-- Attachment #2: 0001-Provide-vc-hg-ignore-to-make-vc-ignore-work-correctl.patch --]
[-- Type: text/x-patch, Size: 3988 bytes --]

From f01b089eda1be46e65a2d68b3c3ed389a61c5084 Mon Sep 17 00:00:00 2001
From: Wolfgang Scherer <wolfgang.scherer@gmx.de>
Date: Sun, 5 Jan 2020 18:09:42 +0100
Subject: [PATCH] Provide vc-hg-ignore to make vc-ignore work correctly

* lisp/vc/vc-hg.el: (vc-hg-ignore) Ignore file of directory.  Add
filepath relative to directory of Mercurial .hgignore file.  The
filepath is quoted according to the active ignore syntax (Bug#37189).
(vc-hg--py-regexp-quote) Quote string as regexp to match exactly
string.
---
 lisp/vc/vc-hg.el | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/lisp/vc/vc-hg.el b/lisp/vc/vc-hg.el
index eac9a6f..633da22 100644
--- a/lisp/vc/vc-hg.el
+++ b/lisp/vc/vc-hg.el
@@ -1212,6 +1212,50 @@ REV is ignored."
   (expand-file-name ".hgignore"
 		    (vc-hg-root file)))

+(defvar vc-hg-ignore-detect-wildcard "[*^$]"
+  "Regular expresssion to detect wildcards in an ignored file
+  specification.")
+
+(defun vc-hg-ignore (file &optional directory remove)
+  "Ignore FILE under Mercurial.
+FILE is either absolute or relative to DIRECTORY (default is
+`default-directory').
+If FILE matches the regular expression
+`vc-hg-ignore-detect-wildcard', it is processed unmodified.
+Otherwise, FILE is converted to a path relative to the project
+root of DIRECTORY.  It is then further escaped/expanded according
+to the active syntax in .hgignore.  If the syntax is `regexp',
+FILE is quoted as anchored literal Python regexp and if FILE is a
+directory, the trailing `$' is omitted.  Otherwise, if the syntax
+is `glob', FILE is used unquoted and if FILE is a directory, a
+`*' is appended.
+If REMOVE is non-nil, remove the pattern derived from FILE from
+ignored files."
+  (let ((ignore (vc-hg-find-ignore-file (or directory default-directory)))
+        (pattern file)
+        root-dir file-path syntax)
+    (unless (string-match vc-hg-ignore-detect-wildcard 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)))
+      (save-match-data
+        (with-current-buffer (find-file-noselect ignore)
+          (goto-char (point-max))
+          (setq syntax
+                (if (re-search-backward "^ *syntax: *\\(regexp\\|glob\\)$" nil t)
+                    (match-string 1)
+                  "regexp")))
+        (setq pattern
+              (if (string= syntax "regexp")
+                  (concat "^" (vc-hg--py-regexp-quote pattern)
+                          (and (not (file-directory-p file-path)) "$"))
+                (concat pattern (and (file-directory-p file-path) "*"))))))
+    (if remove
+        (vc--remove-regexp (concat "^" (regexp-quote pattern ) "\\(\n\\|$\\)") ignore)
+      (vc--add-line pattern ignore))))
+
 ;; Modeled after the similar function in vc-bzr.el
 (defun vc-hg-checkout (file &optional rev)
   "Retrieve a revision of FILE.
@@ -1522,6 +1566,24 @@ This function differs from vc-do-command in that it invokes
 (defun vc-hg-root (file)
   (vc-find-root file ".hg"))

+(defvar vc-hg--py-regexp-special-chars
+  (mapcar
+   (function
+    (lambda (ch)
+      (cons ch (concat "\\" (char-to-string ch)))))
+   (append "()[]{}?*+-|^$\\.&~# \t\n\r\v\f" nil))
+  "Characters that have special meaning in Python regular expressions.")
+
+(defun vc-hg--py-regexp-quote (string)
+  "Return a Python regexp string which matches exactly STRING and nothing else.
+Ported from Python v3.7"
+  (mapconcat
+   (function
+    (lambda (ch)
+      (or (cdr (assq ch vc-hg--py-regexp-special-chars))
+          (char-to-string ch))))
+   string ""))
+
 (provide 'vc-hg)

 ;;; vc-hg.el ends here
--
2.7.4


^ permalink raw reply related	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-01-05  3:46               ` Wolfgang Scherer
  2020-01-05  8:58                 ` Andreas Schwab
@ 2020-01-14  1:14                 ` Dmitry Gutov
  2020-02-01  1:20                   ` Wolfgang Scherer
  1 sibling, 1 reply; 66+ messages in thread
From: Dmitry Gutov @ 2020-01-14  1:14 UTC (permalink / raw)
  To: Wolfgang Scherer, 37189

Hi Wolfgang,

Sorry for the wait.

On 05.01.2020 6:46, Wolfgang Scherer wrote:
>> 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.

OK, I'm fine with that. But user-facing one is vc-ignore, so it can have 
some generic language to that effect (saying something like "wildcard or 
other syntax supported by the backend").

> 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.

OK, true, but otherwise the description should be closer. And we also 
have the description of this backend command in the header commentary of 
vc.el.

> 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.

Actually, I'd have more doubt in its necessity, yes.

This one really should be covered by the docstring of vc-ignore, as well 
as the description of the 'ignore' VC command in the header commentary. 
I mean, it can have a docstring, but it would be a plain copy of the 
existing descriptions elsewhere.

> 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.

Actually, let's talk about vc-default-ignore.

In a recent patch you submitted in debbugs#37217 you changed its 
docstring to say "either relative to DIRECTORY or absolute" whereas it 
originally said "either relative to the root directory of DIRECTORY or 
absolute. That sounds like a change in documented behavior. And it could 
be a problem if it were in a docstring users are actually likely to see.

In any case, I think the docstrings should really be in accord, and the 
"more private" functions shouldn't have crucial information about 
command's behavior that the users won't be able to see.

>> 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.

vc-ignore is still the command the user calls, isn't it? Or are you 
using some other command?

(This seems like the key question of this email at this point).

If yes, I also wonder what are the cases when the DIRECTORY argument is 
important. I.e., when is it not the same as default-directory?

vc-ignore, when called interactively, sets this argument to nil (so it 
is set to default-directory). vc-dir-ignore doesn't pass the second 
argument either.

Importantly, when do we really expect FILE to be something other than 
absolute, relative to the repository root, or a simple glob (e.g. *.c) 
which is going to apply to files regardless of the directory? BTW, it 
seems to me like f144c87f92b broke the last case. Or at least made it 
work worse (calling vc-ignore with '*.c' somewhere deep inside the tree 
would prepend its subdirectory to it).

There is some *valuable* code in this patch, but let's resolve this 
relative-names confusion first.

 From what I see, when vc-ignore receives the FILE argument 
interactively, it's either absolute (as a product of read-file-name when 
the user choose a file with completion), or a free-form glob when the 
user just went ahead and wrote one anyway.

So instead of the current always-relativizing logic, I think we should 
choose what to do by calling file-name-absolute-p. And if it's not 
absolute, just take the value as-is, because there is no telling if the 
user meant "this file in the current dir" or "all files with that name". 
If it is absolute, on the other hand, yes, make it relative to the 
repository root.

>>> +                (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.

Very well.





^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-01-14  1:14                 ` Dmitry Gutov
@ 2020-02-01  1:20                   ` Wolfgang Scherer
  2020-02-01  8:27                     ` Eli Zaretskii
  0 siblings, 1 reply; 66+ messages in thread
From: Wolfgang Scherer @ 2020-02-01  1:20 UTC (permalink / raw)
  To: Dmitry Gutov, 37189

Hi Dmitry,

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.

Until the `vc-ignore` system is fully streamlined, the patches I provided
can be used to deliver something other than nonsense for the
`vc-dir-mode` cases.

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

Originally, `vc` only supoprted SCCS, RCS, CVS, which all have
per-directory semantics (see A. VC Backend History). It is not until
2004-03-15, when Arch is added, that per-tree semantics is mentioned.

Starting 2007-07-30 GIT, HG, BZR, Mtn per-tree VCS are added. Finally
2014-11-20 SRC is added to the current list of supported backends:

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

However, the per-directory paradigm still bleeds through the UI. With
changes as late as 2015, when `vc-root` became the default for
`vc-dir`. And Bug#39380, which I just discovered.

vc-ignore is a quick and dirty afterthought
===========================================

Since `vc` started out with RCS and SCCS support only, there was no
need for ignore file support. Since `pcl-cvs` was more popular than
`vc` for CVS, the ignore file support was obviously never missed
enough.

The first version of `cvs-ignore` in `vc` appeared in 2013
(see B. Initial revision of vc-ignore). It imported
`cvs-append-to-ignore` from `pcl-cvs` and added some quick and dirty
backend implementations for other VCS. Especially, ignoring files in
`vc-dir-mode` is seriously broken.

My standard use case for ignore files under per-tree VCS
========================================================

My most frequent use case is ignoring a couple of marked files in
`vc-dir-mode` under Mercurial.  I almost never add (or remove)
patterns with `M-x vc-ignore`.

The following use cases are implemented in `vc-ignore`,
`vc-hg-ignore`:

1. `vc-dir-ignore` without a prefix argument shall call `vc-ignore`
   with the result of (vc-dir-current-file), which is an absolute path
   name.

2. `vc-dir-ignore` with a prefix argument shall call `vc-ignore` for
   each marked file with the result of (vc-dir-fileinfo->name
   filearg), which is a path relative to (vc-root).

3. The argument FILE of `vc-ignore` is either a pattern, an absolute
   file path or a relative file path.

4. The argument FILE of `vc-hg-ignore` is either a pattern or a file name.

   a. If FILE matches the regular expression
      `vc-hg-ignore-detect-wildcard` ("[*^$]"), it is considered a
      pattern an is written unmodified into the ignore file,

   b. Otherwise, FILE is expanded to an absolute file name, which is
      then made relative to the ignore file directory. The relative
      file path is then escaped according to the active ignore syntax
      and written into the ignore file.

Examples (Mercurial repository):

* `M-x vc-ignore` in sub directory "below1/below2":
  "/home/repo/below1/below2/file.name" => "^below1/below2/file\.name$"

* `M-x vc-ignore` in sub directory "below1/below2":
  "file.name" => "^below1/below2/file\.name$"

* `M-x vc-ignore` in sub directory "some/where/in":
  "\.ext$" => "\.ext$"

* `G` in `vc-dir-mode` on "below1/below2/file.name":
  "/home/repo/below1/below2/file.name" => "^below1/below2/file\.name$"

* `G` in `vc-dir-mode` marked "below1/below2/file.name":
  "below1/below2/file.name" => "^below1/below2/file\.name$"

A. VC backend history
=====================

2015-01-19: (vc-root) instead of default-directory as default for `vc-dir`

2014-12-08: Arch removed
-(defcustom vc-handled-backends '(RCS CVS SVN SCCS SRC Bzr Git Hg Mtn Arch)
+(defcustom vc-handled-backends '(RCS CVS SVN SCCS SRC Bzr Git Hg Mtn)

2014-11-20: SRC added
-(defcustom vc-handled-backends '(RCS CVS SVN SCCS Bzr Git Hg Mtn Arch)
-  ;; RCS, CVS, SVN and SCCS come first because they are per-dir
+(defcustom vc-handled-backends '(RCS CVS SVN SCCS SRC Bzr Git Hg Mtn Arch)
+  ;; RCS, CVS, SVN, SCCS, and SRC come first because they are per-dir

2008-05-07: MCVS removed
-(defcustom vc-handled-backends '(RCS CVS SVN SCCS Bzr Git Hg Mtn Arch MCVS)
+(defcustom vc-handled-backends '(RCS CVS SVN SCCS Bzr Git Hg Mtn Arch)

2007-09-14: Mtn added
-(defcustom vc-handled-backends '(RCS CVS SVN SCCS Bzr Git Hg Arch MCVS)
+(defcustom vc-handled-backends '(RCS CVS SVN SCCS Bzr Git Hg Mtn Arch MCVS)

2007-07-31: BZR added
-(defcustom vc-handled-backends '(RCS CVS SVN SCCS GIT HG Arch MCVS)
+(defcustom vc-handled-backends '(RCS CVS SVN SCCS GIT HG BZR Arch MCVS)

2007-07-30: GIT, HG added
-(defcustom vc-handled-backends '(RCS CVS SVN SCCS Arch MCVS)
+(defcustom vc-handled-backends '(RCS CVS SVN SCCS GIT HG Arch MCVS)

2004-03-15: Arch added, first mention of per-tree
-(defcustom vc-handled-backends '(RCS CVS SVN MCVS SCCS)
+(defcustom vc-handled-backends '(RCS CVS SVN SCCS Arch MCVS)
+  ;; Arch and MCVS come last because they are per-tree rather than per-dir.

2003-05-07: SVN MCVS added
-(defcustom vc-handled-backends '(RCS CVS SCCS)
+(defcustom vc-handled-backends '(RCS CVS SVN MCVS SCCS)

2000-09-04: vc-handled-backends introduced
+(defcustom vc-handled-backends '(RCS CVS SCCS)

1994 CVS support '(RCS SCCS CVS)

1992 first version of vc.el '(RCS SCCS)

B. Initial revision of vc-ignore
================================

Note: `cvs-append-to-ignore` imported from pcvs.el

commit 7aa7fff0c8860b72a2c7cdc7d4d0845245754d43
Author: Xue Fuqiao <xfq.free@gmail.com>
Date:   Tue Jul 30 08:25:31 2013 +0800

    Add vc-ignore.
    * lisp/vc/vc.el (vc-ignore): New function.
    * lisp/vc/vc-svn.el (vc-svn-ignore): New function.
    * lisp/vc/vc-hg.el (vc-hg-ignore): New function.
    * lisp/vc/vc-git.el (vc-git-ignore): New function.
    * lisp/vc/vc-dir.el (vc-dir-mode-map): Add key binding for vc-dir-ignore
    (vc-dir-ignore): New function.
    * lisp/vc/vc-cvs.el (vc-cvs-ignore): New function.
    (cvs-append-to-ignore): Moved from pcvs.el.
    * lisp/vc/vc-bzr.el (vc-bzr-ignore): New function.
    * lisp/vc/pcvs.el (vc-cvs): Require 'vc-cvs.

+(defun vc-bzr-ignore (file)
+  "Ignore FILE under Bazaar."
+  (interactive)
+  (vc-bzr-command "ignore" (get-buffer-create "*vc-ignore*") 0
+                 file))

+(defun vc-cvs-ignore (file)
+  "Ignore FILE under CVS."
+  (interactive)
+  (cvs-append-to-ignore (file-name-directory file) file))
+
+(defun cvs-append-to-ignore (dir str &optional old-dir)
+  "In DIR, add STR to the .cvsignore file.
+If OLD-DIR is non-nil, then this is a directory that we don't want
+to hear about anymore."
+  (with-current-buffer
+      (find-file-noselect (expand-file-name ".cvsignore" dir))
+    (when (ignore-errors
+           (and buffer-read-only
+                (eq 'CVS (vc-backend buffer-file-name))
+                (not (vc-editable-p buffer-file-name))))
+      ;; CVSREAD=on special case
+      (vc-checkout buffer-file-name t))
+    (goto-char (point-max))
+    (unless (bolp) (insert "\n"))
+    (insert str (if old-dir "/\n" "\n"))
+    (if cvs-sort-ignore-file (sort-lines nil (point-min) (point-max)))
+    (save-buffer)))

+    (define-key map "I" 'vc-dir-ignore)

+(defun vc-dir-ignore ()
+  "Ignore the current file."
+  (interactive)
+  (vc-ignore (vc-dir-current-file)))

+(defun vc-git-ignore (file)
+  "Ignore FILE under Git."
+  (interactive)
+  (with-temp-buffer
+    (insert-file-contents
+     (let (gitignore (concat (file-name-as-directory (vc-git-root
+                                                     default-directory)) ".gitignore"))
+       (unless (search-forward file nil t)
+        (goto-char (point-max))
+        (insert (concat "\n" file "\n"))
+        (write-region 1 (point-max) gitignore))))))

+(defun vc-hg-ignore (file)
+  "Ignore FILE under Mercurial."
+  (interactive)
+  (with-temp-buffer
+    (insert-file-contents
+     (let (hgignore (concat (file-name-as-directory (vc-hg-root
+                                                    default-directory)) ".hgignore"))
+       (unless (search-forward file nil t)
+        (goto-char (point-max))
+        (insert (concat "\n" file "\n"))
+        (write-region 1 (point-max) hgignore))))))

+(defun vc-svn-ignore (file)
+  "Ignore FILE under Subversion."
+  (interactive)
+  (vc-svn-command (get-buffer-create "*vc-ignore*") 0
+                 file "propedit" "svn:ignore"))
+

+(defun vc-ignore (file)
+  "Ignore FILE under the current VCS."
+  (interactive "fIgnore file: ")
+  (let ((backend (vc-backend file)))
+    (vc-call-backend backend 'ignore file)))

C. My personal experience with Emacs version control
====================================================

I have been using `vc` for RCS and `pcl-cvs` (and later `vc`) for CVS
since the very beginning in the 90's.

`pcl-cvs` already had support for ignoring files (`cvs-mode-ignore`),
which I always missed in `vc`.

I stopped using CVS in favour of Mercurial for new projects
around 2007.  Due to the lack of support for Mercurial in `vc`, I used
the `dvc` package until recently. The `dvc` package always presents
the entire repository tree and does not map the per-tree semantics
into an inapplicable per-directory UI.

Marking some files in the `*xhg-status*` buffer and invoking the
ignore function properly escapes the filenames relative to the
repository root and puts them into the `.hgignore` file at the
repository root.

Both, `pcl-cvs` and `dvc` work the same way, with `pcl-cvs` placing
the ignored files into the appropriate per-directory ignore files and
`dvc` adding them properly escaped to the per-tree ignore file.

I recently started using `vc` again for Mercurial and Git. When trying
to phase out `dvc` by covering all use cases, I stumbled over the
`vc-ignore` problems. First Mercurial, then CVS and SVN.





^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-01  1:20                   ` Wolfgang Scherer
@ 2020-02-01  8:27                     ` Eli Zaretskii
  2020-02-03  1:16                       ` Wolfgang Scherer
  0 siblings, 1 reply; 66+ messages in thread
From: Eli Zaretskii @ 2020-02-01  8:27 UTC (permalink / raw)
  To: Wolfgang Scherer; +Cc: 37189, dgutov

> 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.





^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-01  8:27                     ` Eli Zaretskii
@ 2020-02-03  1:16                       ` Wolfgang Scherer
  2020-02-04 18:55                         ` Eli Zaretskii
  0 siblings, 1 reply; 66+ messages in thread
From: Wolfgang Scherer @ 2020-02-03  1:16 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 37189, dgutov


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)))






^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-03  1:16                       ` Wolfgang Scherer
@ 2020-02-04 18:55                         ` Eli Zaretskii
  2020-02-05  5:18                           ` Wolfgang Scherer
  2020-02-05 19:06                           ` Wolfgang Scherer
  0 siblings, 2 replies; 66+ messages in thread
From: Eli Zaretskii @ 2020-02-04 18:55 UTC (permalink / raw)
  To: Wolfgang Scherer; +Cc: 37189, dgutov

> Cc: dgutov@yandex.ru, 37189@debbugs.gnu.org
> From: Wolfgang Scherer <Wolfgang.Scherer@gmx.de>
> Date: Mon, 3 Feb 2020 02:16:43 +0100
> 
> Just to be clear that my original intention was only a simple bugfix
> ...

I'm okay with limiting these discussions only to a particular bug.  I
believe it was you who introduced the design issue into the
discussion.  We can drop that part if you don't think it's important
enough.

Having said that...

> 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.  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.

> 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.

> 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.
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.  I'm probably missing something, but what?

Once again, feel free to tell the discussion about the design is not
useful and should be abandoned.  I'm okay with discussing just the
specific bug you have.  But if we are to continue talking about design
aspects, may I suggest that you explain your opinions more than you
did until now?

Thanks.





^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-04 18:55                         ` Eli Zaretskii
@ 2020-02-05  5:18                           ` Wolfgang Scherer
  2020-02-05 19:06                           ` Wolfgang Scherer
  1 sibling, 0 replies; 66+ messages in thread
From: Wolfgang Scherer @ 2020-02-05  5:18 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 37189, dgutov

>> 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.






^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-04 18:55                         ` Eli Zaretskii
  2020-02-05  5:18                           ` Wolfgang Scherer
@ 2020-02-05 19:06                           ` Wolfgang Scherer
  2020-02-07  9:57                             ` Eli Zaretskii
  1 sibling, 1 reply; 66+ messages in thread
From: Wolfgang Scherer @ 2020-02-05 19:06 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 37189, dgutov

Hi Eli,

coming to think of it, I realized that there probably is no maintainer for the vc ignore feature. I.e., there is no use in explaining the design with many words, since it will not be implemented anyway.

So I went ahead and implemented it myself. The CVS, GIt and Hg subsystems are tested and work as expected. The code and a short description is available for review at http://sw-amt.ws/README-emacs-vc-ignore-feature.html#vc-ignore-parameter-extension

If you agree that this is a good way to proceed, I will integrate it.

Wolfgang






^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-05 19:06                           ` Wolfgang Scherer
@ 2020-02-07  9:57                             ` Eli Zaretskii
  2020-02-08  9:57                               ` Dmitry Gutov
  2020-02-09 21:06                               ` Wolfgang Scherer
  0 siblings, 2 replies; 66+ messages in thread
From: Eli Zaretskii @ 2020-02-07  9:57 UTC (permalink / raw)
  To: Wolfgang Scherer; +Cc: 37189, dgutov

> Cc: dgutov@yandex.ru, 37189@debbugs.gnu.org
> From: Wolfgang Scherer <Wolfgang.Scherer@gmx.de>
> Date: Wed, 5 Feb 2020 20:06:15 +0100
> 
> coming to think of it, I realized that there probably is no maintainer for the vc ignore feature. I.e., there is no use in explaining the design with many words, since it will not be implemented anyway.

AFAIU, Dmitry oversees the VC development and maintenance.  That
includes the issues you raised here.

> So I went ahead and implemented it myself. The CVS, GIt and Hg subsystems are tested and work as expected. The code and a short description is available for review at http://sw-amt.ws/README-emacs-vc-ignore-feature.html#vc-ignore-parameter-extension
> 
> If you agree that this is a good way to proceed, I will integrate it.

It's up to Dmitry, really.  If he feels my attempt to understand the
design critique you present is not helping, then I'll gladly leave it
to him to continue with this report.

Thanks.





^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-07  9:57                             ` Eli Zaretskii
@ 2020-02-08  9:57                               ` Dmitry Gutov
  2020-02-08 19:45                                 ` Wolfgang Scherer
  2020-02-09 21:06                               ` Wolfgang Scherer
  1 sibling, 1 reply; 66+ messages in thread
From: Dmitry Gutov @ 2020-02-08  9:57 UTC (permalink / raw)
  To: Eli Zaretskii, Wolfgang Scherer; +Cc: 37189

On 07.02.2020 12:57, Eli Zaretskii wrote:
> AFAIU, Dmitry oversees the VC development and maintenance.  That
> includes the issues you raised here.

That's right. And I asked some questions before in this report and 
didn't get direct answers.

To be clear, I'm quite happy with the meat of the patch presented here, 
just suspicious of the not well-enough documented change in vc-ignore's 
semantics. So yeah, we need a better understanding of the problem and 
maybe a better design. Hopefully one that's not too different from the 
current one.

I'll read the whole of the discussion as soon as I can, but it's going 
to take some time to digest, sorry.

> It's up to Dmitry, really.  If he feels my attempt to understand the
> design critique you present is not helping, then I'll gladly leave it
> to him to continue with this report.

Quite the opposite. If you reach some mutual understanding, it will 
surely help me when reading this thread a bit later.





^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-08  9:57                               ` Dmitry Gutov
@ 2020-02-08 19:45                                 ` Wolfgang Scherer
  2020-02-08 20:05                                   ` Eli Zaretskii
  0 siblings, 1 reply; 66+ messages in thread
From: Wolfgang Scherer @ 2020-02-08 19:45 UTC (permalink / raw)
  To: Dmitry Gutov, Eli Zaretskii; +Cc: 37189


Am 08.02.20 um 10:57 schrieb Dmitry Gutov:
> On 07.02.2020 12:57, Eli Zaretskii wrote:
>> AFAIU, Dmitry oversees the VC development and maintenance.  That
>> includes the issues you raised here.
>
> That's right. And I asked some questions before in this report and didn't get direct answers.

I answered as best as I could up to the point, where it became clear to me that the entire ignore feature has fatal design flaws which need to be addressed, if silly hacks (like this one) are to be avoided.

The status quo before Emacs 27 is:

1. The argument FILE of `vc-ignore` is documented to accept a wildcard specification. This is the use case "pattern".

2. `vc-ignore` is called from `vc-dir-ignore` with either an absolute or relative filename. This is the use case "file path".

3. Some backends expect a file path, some backends expect a pattern. This cannot be fixed without adding a parameter to `vc-ignore`, `vc-<backend>-ignore`.

+-----------------------+-------------+-----------+
| function              | file path   | pattern   |
+=======================+=============+===========+
| :func:`vc-ignore`     | strong hint | yes       |
+-----------------------+-------------+-----------+
| :func:`vc-dir-ignore` | mandatory   | no        |
+-----------------------+-------------+-----------+
| :func:`vc-cvs-ignore` | no          | mandatory |
+-----------------------+-------------+-----------+
| :func:`vc-svn-ignore` | mandatory   | no        |
+-----------------------+-------------+-----------+
| :func:`vc-src-ignore` | --          | --        |
+-----------------------+-------------+-----------+
| :func:`vc-bzr-ignore` | no          | mandatory |
+-----------------------+-------------+-----------+
| :func:`vc-git-ignore` | no          | mandatory |
+-----------------------+-------------+-----------+
| :func:`vc-hg-ignore`  | no          | mandatory |
+-----------------------+-------------+-----------+
| :func:`vc-mtn-ignore` | --          | --        |
+-----------------------+-------------+-----------+

>
> To be clear, I'm quite happy with the meat of the patch presented here,
I am not. The attempt to support a dual API with file/pattern pseudo-detection is just shameful.
> just suspicious of the not well-enough documented change in vc-ignore's semantics. So yeah, we need a better understanding of the problem and maybe a better design.

I have documented the environment, the rationale and the ongoing implementation extensively at http://sw-amt.ws/emacs/doc/_build/html/emacs-vc-ignore-feature.html. The core topic is "File pathes are NOT patterns", so they must be separated in the API.

> Hopefully one that's not too different from the current one.

The current one is completely broken but the necessary changes are only few. Since it hasn't worked to begin with, there is really nothing to preserve.

I have already implemented the core handler `vc-default-ignore` replacing the defunct handlers for CVS, Git, Hg, Bzr by parameter sets. The additional parameter set for SRC is also available. I am planning on implementing Mtn. I do not plan on implementing or fixing SVN (or maybe I will).

Search on page http://sw-amt.ws/emacs/doc/_build/html/emacs-vc-ignore-feature.html for x-vc-repair.el,which is a link to the ongoing implementation. With `eval-buffer` under Emacs 24 - 26, the feature is activated.

The user interface for ignoring files is`C-x v F` or `F` in `vc-dir-mode`. It behaves like other functions, e.g. `C-x v i`, `C-x v u`, `C-x v =`. I.e., it is possible to select multiple files to ignore in `vc-dir-mode` and `dired-mode`.

>
> I'll read the whole of the discussion as soon as I can, but it's going to take some time to digest, sorry.
Please, do not read this discussion, as it has became chaotic before I realized what is required to understand the problem.
>
>> It's up to Dmitry, really.  If he feels my attempt to understand the
>> design critique you present is not helping, then I'll gladly leave it
>> to him to continue with this report.
>
> Quite the opposite. If you reach some mutual understanding, it will surely help me when reading this thread a bit later.

I would really like to close this thread and open one about the correct implementation of `vc-ignore`.






^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-08 19:45                                 ` Wolfgang Scherer
@ 2020-02-08 20:05                                   ` Eli Zaretskii
  2020-02-08 23:12                                     ` Wolfgang Scherer
  2020-02-08 23:59                                     ` Wolfgang Scherer
  0 siblings, 2 replies; 66+ messages in thread
From: Eli Zaretskii @ 2020-02-08 20:05 UTC (permalink / raw)
  To: Wolfgang Scherer; +Cc: 37189, dgutov

> Cc: 37189@debbugs.gnu.org
> From: Wolfgang Scherer <Wolfgang.Scherer@gmx.de>
> Date: Sat, 8 Feb 2020 20:45:34 +0100
> 
> The status quo before Emacs 27 is:
> 
> 1. The argument FILE of `vc-ignore` is documented to accept a wildcard specification. This is the use case "pattern".
> 
> 2. `vc-ignore` is called from `vc-dir-ignore` with either an absolute or relative filename. This is the use case "file path".
> 
> 3. Some backends expect a file path, some backends expect a pattern. This cannot be fixed without adding a parameter to `vc-ignore`, `vc-<backend>-ignore`.
> 
> +-----------------------+-------------+-----------+
> | function              | file path   | pattern   |
> +=======================+=============+===========+
> | :func:`vc-ignore`     | strong hint | yes       |
> +-----------------------+-------------+-----------+
> | :func:`vc-dir-ignore` | mandatory   | no        |
> +-----------------------+-------------+-----------+
> | :func:`vc-cvs-ignore` | no          | mandatory |
> +-----------------------+-------------+-----------+
> | :func:`vc-svn-ignore` | mandatory   | no        |
> +-----------------------+-------------+-----------+
> | :func:`vc-src-ignore` | --          | --        |
> +-----------------------+-------------+-----------+
> | :func:`vc-bzr-ignore` | no          | mandatory |
> +-----------------------+-------------+-----------+
> | :func:`vc-git-ignore` | no          | mandatory |
> +-----------------------+-------------+-----------+
> | :func:`vc-hg-ignore`  | no          | mandatory |
> +-----------------------+-------------+-----------+
> | :func:`vc-mtn-ignore` | --          | --        |
> +-----------------------+-------------+-----------+

This shows that (ignoring mtn for now) all of the functions support
the "pattern" case, except vc-svn-ignore.  However, the doc string of
vc-svn-ignore says

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

So it looks like it, too, supports the "pattern" use case, or what am
I missing?

Now, vc-dir-ignore indeed ignores only one file, but since a file name
is a special case of a wildcard, I wonder why you say there's a need
in an additional argument.  Can you elaborate?

> I have already implemented the core handler `vc-default-ignore` replacing the defunct handlers for CVS, Git, Hg, Bzr by parameter sets. The additional parameter set for SRC is also available. I am planning on implementing Mtn. I do not plan on implementing or fixing SVN (or maybe I will).

From my POV, it is much more important to support SVN than to support
Monotone.  But that's me.

> I would really like to close this thread and open one about the correct implementation of `vc-ignore`.

Feel free to start a new thread, but I really don't see how that could
be of any help.  In particular, this thread discusses a specific bug
(or several related ones), and the new thread will discuss those same
bugs, right?  Then it makes little sense to start a new thread about
the same bug.

Thanks.





^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-08 20:05                                   ` Eli Zaretskii
@ 2020-02-08 23:12                                     ` Wolfgang Scherer
  2020-02-09 13:57                                       ` Wolfgang Scherer
                                                         ` (2 more replies)
  2020-02-08 23:59                                     ` Wolfgang Scherer
  1 sibling, 3 replies; 66+ messages in thread
From: Wolfgang Scherer @ 2020-02-08 23:12 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 37189, dgutov


Am 08.02.20 um 21:05 schrieb Eli Zaretskii:
>> Cc: 37189@debbugs.gnu.org
>> From: Wolfgang Scherer <Wolfgang.Scherer@gmx.de>
>> Date: Sat, 8 Feb 2020 20:45:34 +0100
>>
>> The status quo before Emacs 27 is:
>>
>> 1. The argument FILE of `vc-ignore` is documented to accept a wildcard specification. This is the use case "pattern".
>>
>> 2. `vc-ignore` is called from `vc-dir-ignore` with either an absolute or relative filename. This is the use case "file path".
>>
>> 3. Some backends expect a file path, some backends expect a pattern. This cannot be fixed without adding a parameter to `vc-ignore`, `vc-<backend>-ignore`.
>>
>> +-----------------------+-------------+-----------+
>> | function              | file path   | pattern   |
>> +=======================+=============+===========+
>> | :func:`vc-ignore`     | strong hint | yes       |
>> +-----------------------+-------------+-----------+
>> | :func:`vc-dir-ignore` | mandatory   | no        |
>> +-----------------------+-------------+-----------+
>> | :func:`vc-cvs-ignore` | no          | mandatory |
>> +-----------------------+-------------+-----------+
>> | :func:`vc-svn-ignore` | mandatory   | no        |
>> +-----------------------+-------------+-----------+
>> | :func:`vc-src-ignore` | --          | --        |
>> +-----------------------+-------------+-----------+
>> | :func:`vc-bzr-ignore` | no          | mandatory |
>> +-----------------------+-------------+-----------+
>> | :func:`vc-git-ignore` | no          | mandatory |
>> +-----------------------+-------------+-----------+
>> | :func:`vc-hg-ignore`  | no          | mandatory |
>> +-----------------------+-------------+-----------+
>> | :func:`vc-mtn-ignore` | --          | --        |
>> +-----------------------+-------------+-----------+
> This shows that (ignoring mtn for now) all of the functions support
> the "pattern" case, except vc-svn-ignore.  However, the doc string of
> vc-svn-ignore says
>
>     "Ignore FILE under Subversion.
>   FILE is a wildcard specification, either relative to
>   DIRECTORY or absolute."
>
> So it looks like it, too, supports the "pattern" use case, or what am
> I missing?

Well, FILE is used to construct a path, which is then split into file and directory parts

  (let* ((path (directory-file-name (expand-file-name file directory)))
         (directory (file-name-directory path))
         (file (file-name-nondirectory path))

The "pattern" use case is not the "wildcard" use case. "pattern" is an unspecified string, while "wildcard" is backend specific. For SVN it is a glob(7) expression without subdirectories (otherwise it does not match anything).

Besides the point, that it may not serve a true purpose, It is not possible to add a pattern containing a slash "/" to a SVN directory with `vc-svn-ignore`).

While the command:

   >>> svn propset svn:ignore 'not-here/123' '/srv/install/linux/emacs/check-svn' 

works perfectly well and stores the property as given:

   >>> svn propget svn:ignore '/srv/install/linux/emacs/check-svn'
   not-here/123

The `vc` equivalent fails:

   >>> (vc-svn-ignore 'not-here/123' '/srv/install/linux/emacs/check-svn/')
   'not-here' is not under version control
   svn: E155010: The node '/srv/install/linux/emacs/check-svn/not-here' was not found.

because the svn command issued was:

   >>> svn propset svn:ignore '123' '/srv/install/linux/emacs/check-svn/not-here'

> Now, vc-dir-ignore indeed ignores only one file, but since a file name
> is a special case of a wildcard,

No, it is not, which is the entire point ;-)

Assume you have three files named

  test5.xx
  test6.xx
  test[56].xx

Right now, when I move to the line showing "test[56].xx" and press "G", The function `vc-svn-ignore` is invoked with the FILE argument "test[56].xx", what do you expect to be ignored?

If you say anything else but "test[56].xx", we have a different opinion how the dir-mode UI should work.

Currently the result is, that the files "test5.xx" and "test6.xx" are ignored and the file "test[56].xx" still appears as "unregistered".

In order to ignore "test[56].xx", the function call:

   >>> (vc-svn-ignore "test\\[56].xx")

must be issued.

Unfortunately, critisizing use cases does not make such problems go away. Therefore, the protocol specification should be followed verbatim to implement the ignore function.

While glob syntax may actually be an esoteric use case, Hg and its regex syntax is not, since the "." is quite common in file names:

  myfile.jpg
  myfile+jpg

Ignoring "myfile.jpg" without proper escaping will also ignore "myfile+jpg". While ignoring "myfile+jpg" will not ignore anything.

So strictily speaking, yes, a file name is a special case of a wildcard, but a file name as a wildcard does not necessarily match itself.

Do you see the problem now?

>  I wonder why you say there's a need
> in an additional argument.  Can you elaborate?

The additional argument AS-IS is used to write the FILE argument unmodified to the svn:ignore poperty for DIRECTORY, so that a command:

   >>> (vc-svn-ignore 'not-here/123' '/srv/install/linux/emacs/check-svn/' t) ;; the t is the additional AS-IS argument

would succeed by issuing the appropriate shell command:

   >>> svn propset svn:ignore 'not-here/123' '/srv/install/linux/emacs/check-svn'

If you agree, that for the "file path" use case, the file path should be properly escaped, then the difference would be, e.g:

   >>> (vc-svn-ignore 'good/test[56].xx' '/srv/install/linux/emacs/check-svn/'
   svn propset svn:ignore 'test\[56].xx' '/srv/install/linux/emacs/check-svn/good'

versus:

   >>> (vc-svn-ignore 'good/test[56].xx' '/srv/install/linux/emacs/check-svn/' t) ;; the t is the additional AS-IS argument
   svn propset svn:ignore 'good/test[56].xx' '/srv/install/linux/emacs/check-svn/good'

>> I have already implemented the core handler `vc-default-ignore` replacing the defunct handlers for CVS, Git, Hg, Bzr by parameter sets. The additional parameter set for SRC is also available. I am planning on implementing Mtn. I do not plan on implementing or fixing SVN (or maybe I will).
> From my POV, it is much more important to support SVN than to support
> Monotone.  But that's me.
I already decided to adapt the algorithm/modularization to allow a simple implementation for SVN.
>> I would really like to close this thread and open one about the correct implementation of `vc-ignore`.
> Feel free to start a new thread, but I really don't see how that could
> be of any help.  In particular, this thread discusses a specific bug
> (or several related ones), and the new thread will discuss those same
> bugs, right?  Then it makes little sense to start a new thread about
> the same bug.
The title suggests that this is about Hg, while the problem affects the entire subsystem.

But I'm fine either way.

>
> Thanks.
Thank you





^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-08 20:05                                   ` Eli Zaretskii
  2020-02-08 23:12                                     ` Wolfgang Scherer
@ 2020-02-08 23:59                                     ` Wolfgang Scherer
  1 sibling, 0 replies; 66+ messages in thread
From: Wolfgang Scherer @ 2020-02-08 23:59 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 37189, dgutov

Maybe this helps.

My partial current test tree of mainly glob anomalies (this is also how I discovered Git built-in pathspecs #39452):

  VC backend : SRC
  Working dir: /srv/install/linux/emacs/check-src/

                           ./
      edited               data
      unregistered         sub-file
  *   unregistered         test-?.xx
      unregistered         test-x.xx
  *   unregistered         test2*.xx
      unregistered         test2.xx
      unregistered         test5.xx
      unregistered         test6.xx
      unregistered         test9.xx
  *   unregistered         test9\.xx
  *   unregistered         test[56].xx
      unregistered         with $pe~ial
      unregistered         with spa ces
                           sub/
      edited               sub/data
      unregistered         sub/sub-file
  *   unregistered         sub/test-?.xx
      unregistered         sub/test-x.xx
  *   unregistered         sub/test2*.xx
      unregistered         sub/test2.xx
      unregistered         sub/test5.xx
      unregistered         sub/test6.xx
      unregistered         sub/test9.xx
  *   unregistered         sub/test9\.xx
  *   unregistered         sub/test[56].xx
      unregistered         sub/with $pe~ial
      unregistered         sub/with spa ces

When press "F" with my new implementation, the marked files are ignored, the `vc-dir-mode` display updates itself correctly to:

  VC backend : SRC
  Working dir: /srv/install/linux/emacs/check-src/

                           ./
      unregistered         .srcignore
      edited               data
      unregistered         sub-file
  *   ignored              test-?.xx
      unregistered         test-x.xx
  *   ignored              test2*.xx
      unregistered         test2.xx
      unregistered         test5.xx
      unregistered         test6.xx
      unregistered         test9.xx
  *   ignored              test9\.xx
  *   ignored              test[56].xx
      unregistered         with $pe~ial
      unregistered         with spa ces
                           sub/
      unregistered         sub/.srcignore
      edited               sub/data
      unregistered         sub/sub-file
  *   ignored              sub/test-?.xx
      unregistered         sub/test-x.xx
  *   ignored              sub/test2*.xx
      unregistered         sub/test2.xx
      unregistered         sub/test5.xx
      unregistered         sub/test6.xx
      unregistered         sub/test9.xx
  *   ignored              sub/test9\.xx
  *   ignored              sub/test[56].xx
      unregistered         sub/with $pe~ial
      unregistered         sub/with spa ces

The new .srcignore files' contents are (SRC has a strange escape syntax, which does not follow glob(7)):

  .srcignore: echo
    test-[?].xx
    test2[*].xx
    test9\.xx
    test[[]56].xx

  sub/.srcignore: echo
    test-[?].xx
    test2[*].xx
    test9\.xx
    test[[]56].xx

Refreshing the display and removing the ignored files shows, that the ignore patterns are actually working:

  VC backend : SRC
  Working dir: /srv/install/linux/emacs/check-src/

                           ./
      unregistered         .srcignore
      edited               data
      unregistered         sub-file
      unregistered         test-x.xx
      unregistered         test2.xx
      unregistered         test5.xx
      unregistered         test6.xx
      unregistered         test9.xx
      unregistered         with $pe~ial
      unregistered         with spa ces
                           sub/
      unregistered         sub/.srcignore
      edited               sub/data
      unregistered         sub/sub-file
      unregistered         sub/test-x.xx
      unregistered         sub/test2.xx
      unregistered         sub/test5.xx
      unregistered         sub/test6.xx
      unregistered         sub/test9.xx
      unregistered         sub/with $pe~ial
      unregistered         sub/with spa ces


^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-08 23:12                                     ` Wolfgang Scherer
@ 2020-02-09 13:57                                       ` Wolfgang Scherer
  2020-02-09 13:57                                       ` Wolfgang Scherer
  2020-02-09 13:57                                       ` Wolfgang Scherer
  2 siblings, 0 replies; 66+ messages in thread
From: Wolfgang Scherer @ 2020-02-09 13:57 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 37189, dgutov

Let me clarify this one last time.

> The "pattern" use case is not the "wildcard" use case. "pattern" is an unspecified string, while "wildcard" is backend specific. For SVN it is a glob(7) expression without subdirectories (otherwise it does not match anything).
>
"pattern" is a generic term, which does not imply a specific syntax.

"wildcard specification" is a pattern following the rules of a glob(7)
syntax variant.

"regexp pattern" implies one of the regular expression syntaxes
(regex(7), Emacs, Perl, Python, ...).

Citing from 4.2.1. file pathes are not patterns (http://sw-amt.ws/emacs/doc/_build/html/emacs-vc-ignore-feature.html#file-pathes-are-not-patterns):

    Assuming that a file path is always a pattern that matches the
    file path unambiguously is simply wrong.

    When ignoring a specific file path and nothing but that file
    path, a pattern must be constructed that matches the file path
    exactly. The syntax for this pattern is backend specific and the
    pattern is generally not the file path itself. In other words: it
    is an exception that a file path and the exactly matching pattern
    are identical strings. Specifically, if a file path contains a
    character with special meaning in the pattern syntax, then the
    matching pattern cannot be identical to the file path since the
    special character must be escaped.

    +-------------+-----------------+---------------+-----------------+--------------------+
    | `file path` | glob(7)         | anchored glob | Hg `regex`      | Bzr `regex`        |
    +=============+=================+===============+=================+====================+
    | test[56].xx |   test\[56].xx  | /test\[56].xx | ^test\[56]\.xx$ | RE:^test\[56]\.xx$ |
    |             |   test[[]56].xx |               |                 |                    |
    +-------------+-----------------+---------------+-----------------+--------------------+
    | simple.txt  |   simple.txt    | /simple.txt   | ^simple\.txt$   | RE:^simple\.txt$   |
    |             |   simple[.]txt  |               |                 |                    |
    +-------------+-----------------+---------------+-----------------+--------------------+

    The correct escaping of FILE can only be determined by the
    backend. Therefore neither vc-dir-ignore nor lisp code calling
    vc-ignore can escape the FILE parameter correctly without support
    from the backend. This makes pattern input for FILE only useful
    during interactive calls.

Even, if it was magically possible to determine the correct
pattern in the frontend, submitting an anchored
glob "/some-sub/file.txt" to `vc-ignore` would be interpreted as
an absolute path.

In other words, the API specificaton

  [...] FILE is a wildcard specification, either relative to
  DIRECTORY or absolute.

which asks for implementing the pattern use case inextricably
mixed with the file path use case, is nonsense.

It also means, that all of the backend functions which currently
demand a pattern are absolutely useless.

Therefore the API change request is specifically:

    (defun vc-ignore (file-or-pattern &optional directory remove as-is)
      "Ignore FILE-OR-PATTERN under VCS of DIRECTORY.

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

    When REMOVE is non-nil, remove FILE-OR-PATTERN from the list of
    ignored files.

    If AS-IS is nil, FILE-OR-PATTERN is considered a file path that
    must be escaped and anchored. The directory name of
    FILE-OR-PATTERN expanded against DIRECTORY is used to determine
    the ignore file.  The effective pattern consists of the file path
    relative to the directory of the ignore file, properly escaped
    and anchored by the VC backend.

    If AS-IS is non-nil, FILE-OR-PATTERN is considered a pattern that
    should not be modified.  DIRECTORY is used to determine the
    ignore file."






^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-08 23:12                                     ` Wolfgang Scherer
  2020-02-09 13:57                                       ` Wolfgang Scherer
  2020-02-09 13:57                                       ` Wolfgang Scherer
@ 2020-02-09 13:57                                       ` Wolfgang Scherer
  2020-02-09 14:07                                         ` Wolfgang Scherer
  2 siblings, 1 reply; 66+ messages in thread
From: Wolfgang Scherer @ 2020-02-09 13:57 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 37189, dgutov

Let me clarify this one last time.

> The "pattern" use case is not the "wildcard" use case. "pattern" is an unspecified string, while "wildcard" is backend specific. For SVN it is a glob(7) expression without subdirectories (otherwise it does not match anything).
>
"pattern" is a generic term, which does not imply a specific syntax.

"wildcard specification" is a pattern following the rules of a glob(7)
syntax variant.

"regexp pattern" implies one of the regular expression syntaxes
(regex(7), Emacs, Perl, Python, ...).

Citing from 4.2.1. file pathes are not patterns (http://sw-amt.ws/emacs/doc/_build/html/emacs-vc-ignore-feature.html#file-pathes-are-not-patterns):

    Assuming that a file path is always a pattern that matches the
    file path unambiguously is simply wrong.

    When ignoring a specific file path and nothing but that file
    path, a pattern must be constructed that matches the file path
    exactly. The syntax for this pattern is backend specific and the
    pattern is generally not the file path itself. In other words: it
    is an exception that a file path and the exactly matching pattern
    are identical strings. Specifically, if a file path contains a
    character with special meaning in the pattern syntax, then the
    matching pattern cannot be identical to the file path since the
    special character must be escaped.

    +-------------+-----------------+---------------+-----------------+--------------------+
    | `file path` | glob(7)         | anchored glob | Hg `regex`      | Bzr `regex`        |
    +=============+=================+===============+=================+====================+
    | test[56].xx |   test\[56].xx  | /test\[56].xx | ^test\[56]\.xx$ | RE:^test\[56]\.xx$ |
    |             |   test[[]56].xx |               |                 |                    |
    +-------------+-----------------+---------------+-----------------+--------------------+
    | simple.txt  |   simple.txt    | /simple.txt   | ^simple\.txt$   | RE:^simple\.txt$   |
    |             |   simple[.]txt  |               |                 |                    |
    +-------------+-----------------+---------------+-----------------+--------------------+

    The correct escaping of FILE can only be determined by the
    backend. Therefore neither vc-dir-ignore nor lisp code calling
    vc-ignore can escape the FILE parameter correctly without support
    from the backend. This makes pattern input for FILE only useful
    during interactive calls.

Even, if it was magically possible to determine the correct
pattern in the frontend, submitting an anchored
glob "/some-sub/file.txt" to `vc-ignore` would be interpreted as
an absolute path.

In other words, the API specificaton

  [...] FILE is a wildcard specification, either relative to
  DIRECTORY or absolute.

which asks for implementing the pattern use case inextricably
mixed with the file path use case, is nonsense.

It also means, that all of the backend functions which currently
demand a pattern are absolutely useless.

Therefore the API change request is specifically:

    (defun vc-ignore (file-or-pattern &optional directory remove as-is)
      "Ignore FILE-OR-PATTERN under VCS of DIRECTORY.

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

    When REMOVE is non-nil, remove FILE-OR-PATTERN from the list of
    ignored files.

    If AS-IS is nil, FILE-OR-PATTERN is considered a file path that
    must be escaped and anchored. The directory name of
    FILE-OR-PATTERN expanded against DIRECTORY is used to determine
    the ignore file.  The effective pattern consists of the file path
    relative to the directory of the ignore file, properly escaped
    and anchored by the VC backend.

    If AS-IS is non-nil, FILE-OR-PATTERN is considered a pattern that
    should not be modified.  DIRECTORY is used to determine the
    ignore file."






^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-08 23:12                                     ` Wolfgang Scherer
  2020-02-09 13:57                                       ` Wolfgang Scherer
@ 2020-02-09 13:57                                       ` Wolfgang Scherer
  2020-02-10 16:02                                         ` Eli Zaretskii
  2020-02-09 13:57                                       ` Wolfgang Scherer
  2 siblings, 1 reply; 66+ messages in thread
From: Wolfgang Scherer @ 2020-02-09 13:57 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 37189, dgutov

Let me clarify this one last time.

> The "pattern" use case is not the "wildcard" use case. "pattern" is an unspecified string, while "wildcard" is backend specific. For SVN it is a glob(7) expression without subdirectories (otherwise it does not match anything).
>
"pattern" is a generic term, which does not imply a specific syntax.

"wildcard specification" is a pattern following the rules of a glob(7)
syntax variant.

"regexp pattern" implies one of the regular expression syntaxes
(regex(7), Emacs, Perl, Python, ...).

Citing from 4.2.1. file pathes are not patterns (http://sw-amt.ws/emacs/doc/_build/html/emacs-vc-ignore-feature.html#file-pathes-are-not-patterns):

    Assuming that a file path is always a pattern that matches the
    file path unambiguously is simply wrong.

    When ignoring a specific file path and nothing but that file
    path, a pattern must be constructed that matches the file path
    exactly. The syntax for this pattern is backend specific and the
    pattern is generally not the file path itself. In other words: it
    is an exception that a file path and the exactly matching pattern
    are identical strings. Specifically, if a file path contains a
    character with special meaning in the pattern syntax, then the
    matching pattern cannot be identical to the file path since the
    special character must be escaped.

    +-------------+-----------------+---------------+-----------------+--------------------+
    | `file path` | glob(7)         | anchored glob | Hg `regex`      | Bzr `regex`        |
    +=============+=================+===============+=================+====================+
    | test[56].xx |   test\[56].xx  | /test\[56].xx | ^test\[56]\.xx$ | RE:^test\[56]\.xx$ |
    |             |   test[[]56].xx |               |                 |                    |
    +-------------+-----------------+---------------+-----------------+--------------------+
    | simple.txt  |   simple.txt    | /simple.txt   | ^simple\.txt$   | RE:^simple\.txt$   |
    |             |   simple[.]txt  |               |                 |                    |
    +-------------+-----------------+---------------+-----------------+--------------------+

    The correct escaping of FILE can only be determined by the
    backend. Therefore neither vc-dir-ignore nor lisp code calling
    vc-ignore can escape the FILE parameter correctly without support
    from the backend. This makes pattern input for FILE only useful
    during interactive calls.

Even, if it was magically possible to determine the correct
pattern in the frontend, submitting an anchored
glob "/some-sub/file.txt" to `vc-ignore` would be interpreted as
an absolute path.

In other words, the API specificaton

  [...] FILE is a wildcard specification, either relative to
  DIRECTORY or absolute.

which asks for implementing the pattern use case inextricably
mixed with the file path use case, is nonsense.

It also means, that all of the backend functions which currently
demand a pattern are absolutely useless.

Therefore the API change request is specifically:

    (defun vc-ignore (file-or-pattern &optional directory remove as-is)
      "Ignore FILE-OR-PATTERN under VCS of DIRECTORY.

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

    When REMOVE is non-nil, remove FILE-OR-PATTERN from the list of
    ignored files.

    If AS-IS is nil, FILE-OR-PATTERN is considered a file path that
    must be escaped and anchored. The directory name of
    FILE-OR-PATTERN expanded against DIRECTORY is used to determine
    the ignore file.  The effective pattern consists of the file path
    relative to the directory of the ignore file, properly escaped
    and anchored by the VC backend.

    If AS-IS is non-nil, FILE-OR-PATTERN is considered a pattern that
    should not be modified.  DIRECTORY is used to determine the
    ignore file."






^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-09 13:57                                       ` Wolfgang Scherer
@ 2020-02-09 14:07                                         ` Wolfgang Scherer
  0 siblings, 0 replies; 66+ messages in thread
From: Wolfgang Scherer @ 2020-02-09 14:07 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 37189, dgutov

Sorry, my mail client had problems sending the mail, so it went out three times.





^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-07  9:57                             ` Eli Zaretskii
  2020-02-08  9:57                               ` Dmitry Gutov
@ 2020-02-09 21:06                               ` Wolfgang Scherer
  1 sibling, 0 replies; 66+ messages in thread
From: Wolfgang Scherer @ 2020-02-09 21:06 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 37189, dgutov

[-- Attachment #1: Type: text/plain, Size: 1168 bytes --]

Am 07.02.20 um 10:57 schrieb Eli Zaretskii:
>> Cc: dgutov@yandex.ru, 37189@debbugs.gnu.org
>> From: Wolfgang Scherer <Wolfgang.Scherer@gmx.de>
>> Date: Wed, 5 Feb 2020 20:06:15 +0100
>>
>> coming to think of it, I realized that there probably is no maintainer for the vc ignore feature. I.e., there is no use in explaining the design with many words, since it will not be implemented anyway.
> AFAIU, Dmitry oversees the VC development and maintenance.  That
> includes the issues you raised here.

I have finished the implementation of the vc ignore feature. See attached patch.

The standalone vc extension for old Emacsen is available at http://sw-amt.ws/emacs/doc/_build/html/_static/x-vc-repair.el

This obsoletes #37215 <https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37215>vc-cvs-ignore writes absolute filenames and duplicate strings, <https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37215>

------

Subject: [PATCH] vc ignore feature repair

Complete implementation of ignore feature with proper filename
escaping and anchoring for all applicable supported backends:

  CVS, SVN, SRC, Bzr, Git, Hg, Mtn.

Going back to my day job ...


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-vc-ignore-feature-repair.patch --]
[-- Type: text/x-patch; name="0001-vc-ignore-feature-repair.patch", Size: 25825 bytes --]

From 08d888a903796ef65fa0fe733ecfd71e3b367c26 Mon Sep 17 00:00:00 2001
From: Wolfgang Scherer <wolfgang.scherer@gmx.de>
Date: Sun, 9 Feb 2020 21:50:34 +0100
Subject: [PATCH] vc ignore feature repair
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Complete implementation of ignore feature with proper filename
escaping and anchoring for all applicable supported backends:

  CVS, SVN, SRC, Bzr, Git, Hg, Mtn.

* lisp/vc/vc.el: (vc-ignore-param-none, vc-ignore-param-glob)
  (vc-ignore-param-glob-anchored, vc-ignore-param-regexp)
  (vc-default-ignore-param, vc-glob-escape)
  (vc--py-regexp-special-chars, vc-py-regexp-quote): new vc ignore
  parameters
  (vc-ignore-pattern, vc-ignore-file): new user interface commands
  (vc-ignore-fileset): new frontend command
  (vc-ignore): thin wrapper around ‘vc-default-ignore’
  (vc-default-ignore): thin wrapper around
  ‘vc-default-get-ignore-file-and-pattern’ and
  ‘vc-default-modify-ignores’
  (vc-default-get-ignore-file-and-pattern): workhorse for preparing
  pattern and file ignore parameters
  (vc-default-modify-ignores): default ignore ignore file manipulator
  (vc-file-name-directory, vc-file-relative-name):
  (vc-no-final-slash, vc-has-final-slash): utilities

* lisp/vc/vc-bzr.el: (vc-bzr-ignore-param-regexp)
  (vc-bzr-ignore-param): new vc ignore parameters

* lisp/vc/vc-cvs.el: (vc-cvs-find-ignore-file)
  (vc-cvs-ignore-param-glob, vc-cvs-ignore-param)
  (vc-cvs-glob-escape): new vc ignore parameters
  (vc-cvs-ignore, vc-cvs-append-to-ignore): removed

* lisp/vc/vc-git.el: (vc-git-ignore-param): new vc ignore parameters

* lisp/vc/vc-hg.el: (vc-hg-ignore-param-regexp)
  (vc-hg-ignore-param-glob, vc-hg-ignore-param): new vc ignore
  parameters

* lisp/vc/vc-mtn.el: (vc-mtn-ignore-param-regexp)
  (vc-mtn-ignore-param): new vc ignore parameters

* lisp/vc/vc-src.el: (vc-src-find-ignore-file, vc-src-glob-escape)
  (vc-src-ignore-param-glob, vc-src-ignore-param): new vc ignore
  parameters

* lisp/vc/vc-svn.el: (vc-svn-find-ignore-file)
  (vc-svn-ignore-param-glob, vc-svn-ignore-param)
  (vc-svn-modify-ignores): new vc ignore parameters,
  (vc-svn-ignore): removed

* lisp/vc/vc-dir.el: (vc-dir-mode-map)
  new binding "F" => 'vc-ignore-file
  new binding "G" => 'vc-ignore-pattern

* lisp/vc/vc-hooks.el: (vc-prefix-map)
  new binding "F" => 'vc-ignore-file
  new binding "G" => 'vc-ignore-pattern
  (vc-menu-map): new menu item "Ignore Pattern..."
---
 lisp/vc/vc-bzr.el   |   8 ++
 lisp/vc/vc-cvs.el   |  38 +++----
 lisp/vc/vc-dir.el   |   3 +-
 lisp/vc/vc-git.el   |   4 +
 lisp/vc/vc-hg.el    |  23 +++++
 lisp/vc/vc-hooks.el |   8 +-
 lisp/vc/vc-mtn.el   |   8 ++
 lisp/vc/vc-src.el   |  26 +++++
 lisp/vc/vc-svn.el   |  26 +++--
 lisp/vc/vc.el       | 278 +++++++++++++++++++++++++++++++++++++++++++++-------
 10 files changed, 352 insertions(+), 70 deletions(-)

diff --git a/lisp/vc/vc-bzr.el b/lisp/vc/vc-bzr.el
index e5d307e..1546aba 100644
--- a/lisp/vc/vc-bzr.el
+++ b/lisp/vc/vc-bzr.el
@@ -683,6 +683,14 @@ or a superior directory.")
   (expand-file-name ".bzrignore"
 		    (vc-bzr-root file)))

+(defvar vc-bzr-ignore-param-regexp
+  '(:escape: vc-py-regexp-quote :anchor: "RE:^" :trailer: "$" :dir-trailer: "/.*")
+  "Ignore parameters for Bzr anchored regular expressions.")
+
+(defun vc-bzr-ignore-param (&optional _ignore-file)
+  "Appropriate Bzr ignore parameters for IGNORE-FILE."
+        vc-bzr-ignore-param-regexp)
+
 (defun vc-bzr-checkout (_file &optional rev)
   (if rev (error "Operation not supported")
     ;; Else, there's nothing to do.
diff --git a/lisp/vc/vc-cvs.el b/lisp/vc/vc-cvs.el
index 16566a8..ae60c6c 100644
--- a/lisp/vc/vc-cvs.el
+++ b/lisp/vc/vc-cvs.el
@@ -1220,29 +1220,21 @@ is non-nil."
   "Return the administrative directory of FILE."
   (vc-find-root file "CVS"))

-(defun vc-cvs-ignore (file &optional _directory _remove)
-  "Ignore FILE under CVS."
-  (vc-cvs-append-to-ignore (file-name-directory file) file))
-
-(defun vc-cvs-append-to-ignore (dir str &optional old-dir)
-  "In DIR, add STR to the .cvsignore file.
-If OLD-DIR is non-nil, then this is a directory that we don't want
-to hear about anymore."
-  (with-current-buffer
-      (find-file-noselect (expand-file-name ".cvsignore" dir))
-    (when (ignore-errors
-	    (and buffer-read-only
-		 (eq 'CVS (vc-backend buffer-file-name))
-		 (not (vc-editable-p buffer-file-name))))
-      ;; CVSREAD=on special case
-      (vc-checkout buffer-file-name t))
-    (goto-char (point-max))
-    (unless (bolp) (insert "\n"))
-    (insert str (if old-dir "/\n" "\n"))
-    ;; FIXME this is a pcvs variable.
-    (if (bound-and-true-p cvs-sort-ignore-file)
-        (sort-lines nil (point-min) (point-max)))
-    (save-buffer)))
+(defun vc-cvs-find-ignore-file (file)
+  "Return the ignore file for FILE."
+  (expand-file-name ".cvsignore" (if file (file-name-directory file))))
+
+(defvar vc-cvs-ignore-param-glob
+  '(:escape: vc-cvs-glob-escape :anchor: "" :trailer: "" :dir-trailer: "/")
+  "Ignore parameters for CVS partially anchored glob wildcards.")
+
+(defun vc-cvs-ignore-param (&optional _ignore-file)
+  "Appropriate CVS ignore parameters for IGNORE-FILE."
+  vc-cvs-ignore-param-glob)
+
+(defun vc-cvs-glob-escape (string)
+  "Escape special glob characters and spaces in STRING."
+  (replace-regexp-in-string " " "?" (vc-glob-escape string) t))

 (provide 'vc-cvs)

diff --git a/lisp/vc/vc-dir.el b/lisp/vc/vc-dir.el
index 0c29352..0a1deeb 100644
--- a/lisp/vc/vc-dir.el
+++ b/lisp/vc/vc-dir.el
@@ -302,7 +302,8 @@ See `run-hooks'."
     (define-key map "Q" 'vc-dir-query-replace-regexp)
     (define-key map (kbd "M-s a C-s")   'vc-dir-isearch)
     (define-key map (kbd "M-s a M-C-s") 'vc-dir-isearch-regexp)
-    (define-key map "G" 'vc-dir-ignore)
+    (define-key map "F" 'vc-ignore-file)
+    (define-key map "G" 'vc-ignore-pattern)

     (let ((branch-map (make-sparse-keymap)))
       (define-key map "B" branch-map)
diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index 2caa287..5292f21 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -976,6 +976,10 @@ It is based on `log-edit-mode', and has Git-specific extensions.")
   (expand-file-name ".gitignore"
 		    (vc-git-root file)))

+(defun vc-git-ignore-param (&optional _ignore-file)
+  "Appropriate Git ignore parameters for IGNORE-FILE."
+  vc-ignore-param-glob-anchored)
+
 (defun vc-git-checkout (file &optional rev)
   (vc-git-command nil 0 file "checkout" (or rev "HEAD")))

diff --git a/lisp/vc/vc-hg.el b/lisp/vc/vc-hg.el
index eac9a6f..3d22ae2 100644
--- a/lisp/vc/vc-hg.el
+++ b/lisp/vc/vc-hg.el
@@ -1212,6 +1212,29 @@ REV is ignored."
   (expand-file-name ".hgignore"
 		    (vc-hg-root file)))

+(defvar vc-hg-ignore-param-regexp
+  '(:escape: vc-py-regexp-quote :anchor: "^" :trailer: "$" :dir-trailer: "/")
+  "Ignore parameters for Hg anchored regular expressions.")
+
+(defvar vc-hg-ignore-param-glob
+  '(:escape: vc-glob-escape :anchor: "" :trailer: "" :dir-trailer: "/*")
+  "Ignore parameters for Hg anchored regular expressions.")
+
+(defun vc-hg-ignore-param (&optional ignore-file)
+  "Appropriate Hg ignore parameters for IGNORE-FILE."
+  (let ((syntax "regexp"))
+    (if (not ignore-file)
+        (setq ignore-file (vc-hg-find-ignore-file default-directory)))
+    (if (file-exists-p ignore-file)
+        (with-current-buffer (find-file-noselect ignore-file)
+          (save-match-data
+            (goto-char (point-max))
+            (if (re-search-backward "^ *syntax: *\\(regexp\\|glob\\)$" nil t)
+                (setq syntax (match-string 1))))))
+    (if (string= syntax "regexp")
+        vc-hg-ignore-param-regexp
+      vc-hg-ignore-param-glob)))
+
 ;; Modeled after the similar function in vc-bzr.el
 (defun vc-hg-checkout (file &optional rev)
   "Retrieve a revision of FILE.
diff --git a/lisp/vc/vc-hooks.el b/lisp/vc/vc-hooks.el
index 345a28d..3492dc1 100644
--- a/lisp/vc/vc-hooks.el
+++ b/lisp/vc/vc-hooks.el
@@ -883,7 +883,8 @@ In the latter case, VC mode is deactivated for this buffer."
     (define-key map "b" 'vc-switch-backend)
     (define-key map "d" 'vc-dir)
     (define-key map "g" 'vc-annotate)
-    (define-key map "G" 'vc-ignore)
+    (define-key map "F" 'vc-ignore-file)
+    (define-key map "G" 'vc-ignore-pattern)
     (define-key map "h" 'vc-region-history)
     (define-key map "i" 'vc-register)
     (define-key map "l" 'vc-print-log)
@@ -970,8 +971,11 @@ In the latter case, VC mode is deactivated for this buffer."
       '(menu-item "Register" vc-register
 		  :help "Register file set into a version control system"))
     (bindings--define-key map [vc-ignore]
-      '(menu-item "Ignore File..." vc-ignore
+      '(menu-item "Ignore File..." vc-ignore-file
 		  :help "Ignore a file under current version control system"))
+    (bindings--define-key vc-menu-map [vc-ignore-pattern]
+      '(menu-item "Ignore Pattern..." vc-ignore-pattern
+                  :help "Ignore a pattern under current version control system"))
     (bindings--define-key map [vc-dir]
       '(menu-item "VC Dir"  vc-dir
 		  :help "Show the VC status of files in a directory"))
diff --git a/lisp/vc/vc-mtn.el b/lisp/vc/vc-mtn.el
index 092d8b5..af42d3f 100644
--- a/lisp/vc/vc-mtn.el
+++ b/lisp/vc/vc-mtn.el
@@ -106,6 +106,14 @@ switches."
   "Return the mtn ignore file that controls FILE."
   (expand-file-name ".mtnignore" (vc-mtn-root file)))

+(defvar vc-mtn-ignore-param-regexp
+  '(:escape: vc-py-regexp-quote :anchor: "^" :trailer: "$" :dir-trailer: "/")
+  "Ignore parameters for Mtn anchored regular expressions.")
+
+(defun vc-mtn-ignore-param (&optional _ignore-file)
+  "Appropriate Mtn ignore parameters for IGNORE-FILE."
+  vc-mtn-ignore-param-regexp)
+
 (defun vc-mtn-registered (file)
   (let ((root (vc-mtn-root file)))
     (when root
diff --git a/lisp/vc/vc-src.el b/lisp/vc/vc-src.el
index db127ee..cd9f032 100644
--- a/lisp/vc/vc-src.el
+++ b/lisp/vc/vc-src.el
@@ -310,6 +310,32 @@ If LIMIT is non-nil, show no more than this many entries."
   "Rename file from OLD to NEW using `src mv'."
   (vc-src-command nil 0 new "mv" old))

+(defun vc-src-find-ignore-file (file)
+  "Return the ignore file for FILE."
+  (expand-file-name ".srcignore" (if file (file-name-directory file))))
+
+(defun vc-src-glob-escape (string)
+  "Escape special glob characters in STRING."
+  (save-match-data
+    (if (string-match "[?*[]" string)
+        (mapconcat (lambda (c)
+                     (pcase c
+                       (?? "[?]")
+                       (?* "[*]")
+                       (?\[ "[[]")
+                       (_ (char-to-string c))))
+                   string "")
+      string)))
+;; (vc-src-glob-escape "full[glo]?\\b*")
+
+(defvar vc-src-ignore-param-glob
+  '(:escape: vc-src-glob-escape :anchor: "" :trailer: "" :dir-trailer: "")
+  "Ignore parameters for SRC unanchored glob wildcards.")
+
+(defun vc-src-ignore-param (&optional _ignore-file)
+  "Appropriate SRC ignore parameters for IGNORE-FILE."
+  vc-src-ignore-param-glob)
+
 (provide 'vc-src)

 ;;; vc-src.el ends here
diff --git a/lisp/vc/vc-svn.el b/lisp/vc/vc-svn.el
index d039bf3..9d35b75 100644
--- a/lisp/vc/vc-svn.el
+++ b/lisp/vc/vc-svn.el
@@ -352,17 +352,25 @@ to the SVN command."
 		(concat "-r" rev))
 	   (vc-switches 'SVN 'checkout))))

-(defun vc-svn-ignore (file &optional directory remove)
-  "Ignore FILE under Subversion.
-FILE is a wildcard specification, either relative to
-DIRECTORY or absolute."
-  (let* ((path (directory-file-name (expand-file-name file directory)))
-         (directory (file-name-directory path))
-         (file (file-name-nondirectory path))
+(defun vc-svn-find-ignore-file (file)
+  "Return the virtual ignore file for FILE."
+  (expand-file-name ".svnignore" (if file (file-name-directory file))))
+
+(defvar vc-svn-ignore-param-glob
+  '(:escape: vc-glob-escape :anchor: "" :trailer: "" :dir-trailer: "")
+  "Ignore parameters for SVN unanchored glob wildcards.")
+
+(defun vc-svn-ignore-param (&optional _ignore-file)
+  "Appropriate SVN ignore parameters for IGNORE-FILE."
+  vc-svn-ignore-param-glob)
+
+(defun vc-svn-modify-ignores (pattern ignore-file remove)
+  ;; implements ‘vc-default-modify-ignores’ for SVN
+  (let* ((directory (file-name-directory ignore-file))
          (ignores (vc-svn-ignore-completion-table directory))
          (ignores (if remove
-                      (delete file ignores)
-                    (push file ignores))))
+                      (delete pattern ignores)
+                    (push pattern ignores))))
     (vc-svn-command nil 0 nil nil "propset" "svn:ignore"
                     (mapconcat #'identity ignores "\n")
                     directory)))
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index ec252b7..d3e5537 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -1392,54 +1392,262 @@ first backend that could register the file is used."
   (let ((vc-handled-backends (list backend)))
     (call-interactively 'vc-register)))

-(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.
+(defvar vc-ignore-param-none
+  '(:escape: identity :anchor: "" :trailer: "" :dir-trailer: "")
+  "Property list of ignore parameters for plain strings.
+
+All properties are optional.
+
+Property :escape: is a function that takes a pattern string as parameter
+and returns an escaped pattern (default is ‘identity’).
+
+Property :anchor: is a string that is prepended to the ignore
+pattern (default is an empty string).
+
+Property :trailer: is a string that is appended to non-directory
+ignore patterns (default is an empty string).
+
+Property :dir-trailer: is a string that is appended to directory
+ignore patterns (default is an empty string).")
+
+(defvar vc-ignore-param-glob
+  '(:escape: vc-glob-escape :anchor: "" :trailer: "" :dir-trailer: "")
+  "Ignore parameters for unanchored glob wildcards.")
+
+(defvar vc-ignore-param-glob-anchored
+  '(:escape: vc-glob-escape :anchor: "/" :trailer: "" :dir-trailer: "/")
+  "Ignore parameters for anchored glob wildcards.")
+
+(defvar vc-ignore-param-regexp
+  '(:escape: regexp-quote :anchor: "^" :trailer: "$" :dir-trailer: "/")
+  "Ignore parameters for anchored regular expressions.")
+
+(defun vc-default-ignore-param (_backend &optional _ignore-file)
+  "Default ignore parameters for IGNORE-FILE."
+  vc-ignore-param-glob)
+
+(defun vc-glob-escape (string)
+  "Escape special glob characters in STRING."
+  (save-match-data
+    (if (string-match "[\\?*[]" string)
+        (mapconcat (lambda (c)
+                     (pcase c
+                       (?\\ "\\\\")
+                       (?? "\\?")
+                       (?* "\\*")
+                       (?\[ "\\[")
+                       (_ (char-to-string c))))
+                   string "")
+      string)))
+
+(defvar vc--py-regexp-special-chars
+  (mapcar
+   (function
+    (lambda (c)
+      (cons c (concat "\\" (char-to-string c)))))
+   (append "()[]{}?*+-|^$\\.&~# \t\n\r\v\f" nil))
+  "Characters that have special meaning in Python regular expressions.")
+
+(defun vc-py-regexp-quote (string)
+  "Python regexp to match exactly STRING and nothing else.
+Ported from Python v3.7"
+  (mapconcat
+   (function
+    (lambda (c)
+      (or (cdr (assq c vc--py-regexp-special-chars))
+          (char-to-string c))))
+   string ""))
+
+(defun vc-ignore-pattern (pattern &optional directory remove)
+  "Ignore PATTERN under VCS of DIRECTORY.

 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."
+PATTERN is an expression following the rules of the backend
+pattern syntax, matching the files to be ignored.  When REMOVE is
+non-nil, remove PATTERN from the list of ignored files.
+
+When called interactively, prompt for a PATTERN to ignore, unless
+a prefix argument is given, in which case prompt for a PATTERN to
+remove from the list of ignored files offering currently defined
+patterns for completion."
   (interactive
    (list
     (if (not current-prefix-arg)
-        (read-file-name "File to ignore: ")
+        (read-string "Pattern to ignore: ")
       (completing-read
-       "File to remove: "
+       "Pattern 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)))
-
-(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))))
+  (vc-ignore pattern directory remove t))
+
+(defun vc-ignore-file (file &optional directory remove)
+  "Ignore FILE under VCS of DIRECTORY.
+
+DIRECTORY defaults to `default-directory' and is used to
+determine the responsible VC backend.
+
+If FILE is nil, ‘vc-ignore-fileset’ is called.
+
+Otherwise, FILE is a file path that must be escaped and anchored.
+The directory name of FILE expanded against DIRECTORY is used to
+determine the ignore file.  The effective pattern consists of the
+file path relative to the directory of the ignore file, properly
+escaped and anchored by the VC backend.
+
+The effective pattern is added to the list of ignored files,
+unless REMOVE is non-nil, in which case it is removed.
+
+When called interactively and the mode is neither ‘vc-dir-mode’
+nor ‘dired-mode’, prompt for a FILE to ignore, unless a prefix
+argument is given, in which case prompt for a FILE to remove from
+the list of ignored files."
+  (interactive
+   (list
+    (unless (or (derived-mode-p 'vc-dir-mode) (derived-mode-p 'dired-mode))
+      (read-file-name
+       (concat "File to "
+               (if (not current-prefix-arg) "ignore" "remove") ": ")))
+    nil current-prefix-arg))
+  (if file
+      (vc-ignore file directory remove nil)
+    (vc-ignore-fileset nil remove)))
+
+(defun vc-ignore-fileset (&optional vc-fileset remove)
+  "Ignore file set under a version control system..
+
+If VC-FILESET is not given, it is deduced with
+‘vc-deduce-fileset’.
+
+When REMOVE is non-nil (prefix arg, if interactive), remove the
+files from the list of ignored files."
+  (interactive (list nil current-prefix-arg))
+  (let* ((fileset-arg (or vc-fileset (vc-deduce-fileset t t)))
+         (backend (car fileset-arg))
+         (files (delq nil (nth 1 fileset-arg))))
+    (when files
+      (message "Ignoring %s... " files)
+      (mapc
+       (lambda (file)
+         (vc-call-backend backend 'ignore file nil remove nil)
+         (vc-dir-resynch-file file))
+       files))
+    (when (derived-mode-p 'vc-dir-mode)
+      (vc-dir-move-to-goal-column))
+    (when files (message "Ignoring %s... done" files))))
+
+(defun vc-ignore (file-or-pattern &optional directory remove as-is)
+  "Ignore FILE-OR-PATTERN under VCS of DIRECTORY.
+
+DIRECTORY defaults to `default-directory' and is used to
+determine the responsible VC backend.
+
+When REMOVE is non-nil, remove FILE-OR-PATTERN from the list of
+ignored files.
+
+If AS-IS is nil, FILE-OR-PATTERN is considered a file path that
+must be escaped and anchored.  The directory name of
+FILE-OR-PATTERN expanded against DIRECTORY is used to determine
+the ignore file.  The effective pattern consists of the file path
+relative to the directory of the ignore file, properly escaped
+and anchored by the VC backend.
+
+If AS-IS is non-nil, FILE-OR-PATTERN is considered a pattern that
+should not be modified.  DIRECTORY is used to determine the
+ignore file."
+  (setq directory (or directory default-directory))
+  (vc-call-backend (or (vc-responsible-backend directory)
+                       (error "Unknown backend"))
+                   'ignore file-or-pattern directory remove as-is))
+
+(defun vc-default-ignore (backend file-or-pattern &optional directory remove as-is)
+  ;; implements ‘vc-ignore’ generically
+  (apply #'vc-call-backend backend 'modify-ignores
+         (vc-call-backend backend 'get-ignore-file-and-pattern
+                          file-or-pattern directory as-is remove)))
+
+(defun vc-default-get-ignore-file-and-pattern (backend file-or-pattern &optional directory as-is remove)
+  "Determine ignore file and pattern for BACKEND from FILE-OR-PATTERN.
+Implements API of ‘vc-ignore’ for FILE-OR-PATTERN, DIRECTORY and AS-IS.
+REMOVE is passed through without evaluation.
+Returns (pattern ignore-file remove) suitable for calling
+‘vc-default-modify-ignores’."
+
+  (setq directory (or directory default-directory))
+  (when (not as-is)
+    (setq file-or-pattern (expand-file-name file-or-pattern directory))
+    ;; apply directory-as-file-name, otherwise, if file-or-pattern was
+    ;; a sub-repository, find-ignore-file would return the wrong
+    ;; ignore file:
+    ;; (vc-cvs-find-ignore-file "/re/po/dir/") => /re/po/dir/.cvsignore
+    ;; (vc-cvs-find-ignore-file "/re/po/dir") => /re/po/.cvsignore
+    (setq directory (file-name-directory (vc-no-final-slash file-or-pattern))))
+
+  (let* ((ignore-file (vc-call-backend backend 'find-ignore-file directory))
+         (ignore-dir (file-name-directory ignore-file))
+         is-dir ignore-param pattern)
+    (if as-is
+        (setq ignore-param vc-ignore-param-none)
+      (when (not (string= (substring file-or-pattern 0 (length ignore-dir))
+                          ignore-dir))
+        (error "Ignore spec %s is not below project root %s"
+               file-or-pattern ignore-dir))
+      ;; directory may not yet exist
+      (setq is-dir (or (file-directory-p file-or-pattern)
+                       (vc-has-final-slash file-or-pattern)))
+      (setq file-or-pattern (vc-no-final-slash
+                             (substring file-or-pattern (length ignore-dir))))
+      (setq ignore-param (vc-call-backend backend 'ignore-param ignore-file)))
+
+    (setq pattern
+          (concat
+           (plist-get ignore-param :anchor:)
+           (funcall (or (plist-get ignore-param :escape:) #'identity)
+                    file-or-pattern)
+           (or (and is-dir (plist-get ignore-param :dir-trailer:))
+               (plist-get ignore-param :trailer:))))
+    (list pattern ignore-file remove)))
+
+(defun vc-default-modify-ignores (_backend pattern ignore-file remove)
+  "Add PATTERN to IGNORE-FILE, if REMOVE is nil..
+Otherwise remove PATTERN from IGNORE-FILE."
+  (if remove
+      (vc--remove-regexp
+       (concat "^" (regexp-quote pattern) "\\(\n\\|$\\)") ignore-file)
+    (vc--add-line pattern ignore-file)))
+
+(defun vc-file-name-directory (file &optional dir dir-as-file)
+  "Get directory name for FILE.
+FILE is expanded against DIR.  If FILE is a directory and DIR-AS-FILE
+is non-nil, its parent directory is returned."
+  (and file
+       (let* ((path (expand-file-name file dir)))
+         (file-name-directory
+          (if dir-as-file
+              (vc-no-final-slash path)
+            path)))))
+
+(defun vc-file-relative-name (file &optional dir dir-is-empty)
+  "Get relative file name for FILE against DIR.
+If FILE is a directory and DIR-IS-EMPTY is non-nil, nil is returned.
+Otherwise, if FILE is a directory, the final slash is removed."
+  (and (not (and dir-is-empty (file-directory-p file)))
+       (vc-no-final-slash (file-relative-name file dir))))
+
+(defun vc-no-final-slash (s)
+  "Remove optional final slash from string S."
+  ;; based on ‘ido-no-final-slash’
+  (let ((l (vc-has-final-slash s)))
+    (if l (substring s 0 l) s)))
+
+(defun vc-has-final-slash (s)
+  ;"Return index of final slash in string S or nil."
+  (let ((l (1- (length s))))
+    (and (> l 0) (eq (aref s l) ?/) l)))

 (defun vc-default-ignore-completion-table (backend file)
   "Return the list of ignored files under BACKEND."
--
2.7.4


^ permalink raw reply related	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-09 13:57                                       ` Wolfgang Scherer
@ 2020-02-10 16:02                                         ` Eli Zaretskii
  2020-02-11  1:45                                           ` Wolfgang Scherer
  0 siblings, 1 reply; 66+ messages in thread
From: Eli Zaretskii @ 2020-02-10 16:02 UTC (permalink / raw)
  To: Wolfgang Scherer; +Cc: 37189, dgutov

> From: Wolfgang Scherer <Wolfgang.Scherer@gmx.de>
> Cc: dgutov@yandex.ru, 37189@debbugs.gnu.org
> Date: Sun, 9 Feb 2020 14:57:12 +0100
> 
> "pattern" is a generic term, which does not imply a specific syntax.
> 
> "wildcard specification" is a pattern following the rules of a glob(7)
> syntax variant.
> 
> "regexp pattern" implies one of the regular expression syntaxes
> (regex(7), Emacs, Perl, Python, ...).

Got it, thanks.

>     +-------------+-----------------+---------------+-----------------+--------------------+
>     | `file path` | glob(7)         | anchored glob | Hg `regex`      | Bzr `regex`        |
>     +=============+=================+===============+=================+====================+
>     | test[56].xx |   test\[56].xx  | /test\[56].xx | ^test\[56]\.xx$ | RE:^test\[56]\.xx$ |
>     |             |   test[[]56].xx |               |                 |                    |
>     +-------------+-----------------+---------------+-----------------+--------------------+
>     | simple.txt  |   simple.txt    | /simple.txt   | ^simple\.txt$   | RE:^simple\.txt$   |
>     |             |   simple[.]txt  |               |                 |                    |
>     +-------------+-----------------+---------------+-----------------+--------------------+

Just a note: this table might be interpreted to mean that hg and bzr
only support regexes in their ignore files, but that is of course
false: they also support glob-style widlcards.

>     The correct escaping of FILE can only be determined by the
>     backend. Therefore neither vc-dir-ignore nor lisp code calling
>     vc-ignore can escape the FILE parameter correctly without support
>     from the backend. This makes pattern input for FILE only useful
>     during interactive calls.
> 
> Even, if it was magically possible to determine the correct
> pattern in the frontend, submitting an anchored
> glob "/some-sub/file.txt" to `vc-ignore` would be interpreted as
> an absolute path.
> 
> In other words, the API specificaton
> 
>   [...] FILE is a wildcard specification, either relative to
>   DIRECTORY or absolute.
> 
> which asks for implementing the pattern use case inextricably
> mixed with the file path use case, is nonsense.
> 
> It also means, that all of the backend functions which currently
> demand a pattern are absolutely useless.

I don't think I agree.  While the direction in which you propose to
move -- which AFAIU is to offer 2 different commands, one to ignore a
file name, the other to ignore a file pattern -- is definitely
possible, I question its necessity.  In the use cases I have in mind,
the ignore file-or-pattern always comes from the user, because only
the users can decide which files they want to ignore.  And the users
always know both which backend they are working with, and whether the
file-or-pattern is a filename or a pattern.  It follows that we can
ask the users to escape and anchor the file-or-pattern argument they
type, and that is not an unreasonable expectation.  In fact, your
approach expects the same from the users, because you are asking the
users to decide which of the two commands to invoke in each case.

If there are no flaws in my way of reasoning, then I think the
resulting changes will be much less extensive, because the same
vc-ignore command can then be used for both ignoring a specific file
and for ignoring a pattern of any kind.  We just need to document that
if the argument is a pattern of any kind, the user will have to make
sure it is escaped and anchored for the backend to DTRT.

A further advantage of my proposal is that we don't need to write any
backend-specific code to escape special characters in patterns,
because we expect the users to do that.

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.

Am I missing something?





^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-10 16:02                                         ` Eli Zaretskii
@ 2020-02-11  1:45                                           ` Wolfgang Scherer
  2020-02-11 17:32                                             ` Eli Zaretskii
  0 siblings, 1 reply; 66+ messages in thread
From: Wolfgang Scherer @ 2020-02-11  1:45 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 37189, dgutov

Hi Eli,

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 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) .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.

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? (For a humorous
suggestion, see below).

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.

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. 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`.

So, if you do not plan to drop this feature from `vc-dir-mode`
altogether, then a new function must be implemented, which does
exactly what my implementation does. Therefore, it only fixes the
broken behavior in `vc-dir-mode`.

If you are just worried, that somebody will use `vc-ignore` and
expect the old broken behavior, I suggest to accept my
implementaton and add a custom variable
`vc-ignore-posixly-correct` with a default value of `t` to prefer
the same behavior as before (see below for a draft
implementation).

I don't think it is really necessary, since I removed interactive
from `vc-ignore` and `C-x v G` calls `vc-ignore-pattern` which
essentially behaves like the old `vc-ignore` minus the real bugs.
So a user would only realize the API change when executing `M-x
vc-ignore`.

Am 10.02.20 um 17:02 schrieb Eli Zaretskii:
>> From: Wolfgang Scherer <Wolfgang.Scherer@gmx.de>
>> Cc: dgutov@yandex.ru, 37189@debbugs.gnu.org
>> Date: Sun, 9 Feb 2020 14:57:12 +0100
>>
>>     +-------------+-----------------+---------------+-----------------+--------------------+
>>     | `file path` | glob(7)         | anchored glob | Hg `regex`      | Bzr `regex`        |
>>     +=============+=================+===============+=================+====================+
>>     | test[56].xx |   test\[56].xx  | /test\[56].xx | ^test\[56]\.xx$ | RE:^test\[56]\.xx$ |
>>     |             |   test[[]56].xx |               |                 |                    |
>>     +-------------+-----------------+---------------+-----------------+--------------------+
>>     | simple.txt  |   simple.txt    | /simple.txt   | ^simple\.txt$   | RE:^simple\.txt$   |
>>     |             |   simple[.]txt  |               |                 |                    |
>>     +-------------+-----------------+---------------+-----------------+--------------------+
> Just a note: this table might be interpreted to mean that hg and bzr
> only support regexes in their ignore files, but that is of course
> false: they also support glob-style widlcards.
This is not an exhaustive table of ignore specification
syntaxes. It is meant to illustrate that the case

  file path === ignore specification

is really the exception.
>>     The correct escaping of FILE can only be determined by the
>>     backend. Therefore neither vc-dir-ignore nor lisp code calling
>>     vc-ignore can escape the FILE parameter correctly without support
>>     from the backend. This makes pattern input for FILE only useful
>>     during interactive calls.
>>
>> Even, if it was magically possible to determine the correct
>> pattern in the frontend, submitting an anchored
>> glob "/some-sub/file.txt" to `vc-ignore` would be interpreted as
>> an absolute path.
>>
>> In other words, the API specificaton
>>
>>   [...] FILE is a wildcard specification, either relative to
>>   DIRECTORY or absolute.
>>
>> which asks for implementing the pattern use case inextricably
>> mixed with the file path use case, is nonsense.
>>
>> It also means, that all of the backend functions which currently
>> demand a pattern are absolutely useless.
> I don't think I agree.  While the direction in which you propose to
> move -- which AFAIU is to offer 2 different commands, one to ignore a
> file name, the other to ignore a file pattern -- is definitely
> possible,

(Sorry, I did not have time to clean the ReStructured Text source, it is rendered in HTML at http://sw-amt.ws/emacs/doc/_build/html/emacs-vc-ignore-feature.html#implementation-of-vc-ignore-parameter-extension)

The actual implementation is in a single function

:defun:`vc-ignore`. The use cases "pattern" and "file path" are
selected with the flag AS-IS.

For interactive use, there are 2 separate commands
:defun:`vc-ignore-file` and :defun:`vc-ignore-pattern`, both of which
just call :defun:`vc-ignore` with a different value for the flag
AS-IS.

This decision was made, because the prefix argument as usual choice to
specify a flag parameter interactively is already appropriated for the
flag REMOVE. Instead of an interactive y/n question for each
invocation - which is pretty annoying - 2 separate functions also
facilitate keymap definitions. It also has the advantage that only the
relevant information for each use case can be presented in the doc
string.

Granted that typing :kbd:`C-x v F` instead of :kbd:`C-x v G` or
:kbd:`F` instead of :kbd:`G` is also a decision, but it does not
necessarily have to be made each time, especially if a user only cares
about a single use case. That concludes the argument for 2 separate
interactive commands as user interface.

The implementation consists of a linear call chain with two short code
path variations. Besides backend specific implementations for some
functions, there are **never** 2 separate functions in the entire
chain.

.. uml::
   :caption: :defun:`vc-ignore` call chain

    @startuml
    participant "vc-default-\nignore" as VCDI
    participant "vc-default-\nget-ignore-file-\nand-pattern" as VCDIAP
    participant "vc-backend-\nignore-param" as VCBIP
    participant "vc-backend-\nfind-ignore-file" as VCBFIF
    participant "vc-default-\nmod-ignores" as VCDMI

    VCDI     ->  VCDIAP : (vc-call-backend FILE-OR-PATTERN\n DIRECTORY AS-IS REMOVE)
    alt AS-IS non-nil
      VCDIAP ->  VCDIAP : absolute FILE-PATH\nnormalize DIRECTORY
    end
    VCDIAP   ->  VCBFIF : (vcb DIRECTORY)
    VCDIAP   <-- VCBFIF :IGNORE-FILE
    alt AS-IS nil
      VCDIAP ->  VCDIAP : null ignore spec processing parameters
    else AS-IS non-nil
      VCDIAP ->  VCDIAP : relative FILE-PATH
      VCDIAP ->  VCBIP : (vcb IGNORE-FILE)
      VCDIAP <-- VCBIP : ignore spec processing parameters
    end
    VCDIAP   ->  VCDIAP : escape/anchor pattern
    VCDI     <-- VCDIAP : '(IGNORE-FILE PATTERN REMOVE)
    VCDI     ->  VCDMI : (vcb IGNORE-FILE PATTERN REMOVE)
    VCDI     <-- VCDMI
    @enduml

The code path variations based on the AS-IS flag are confined to
:defun:`vc-default-get-ignore-file-and-pattern`.

First, if AS-IS is nil, the FILE-OR-PATTERN argument is expanded to an
absolute FILE-PATH. DIRECTORY is set to the immediate parent directory
of :elisp:`(vc-no-final-slash FILE-PATH)`: This normalization is
necessary, because the search for an ignore file starts at DIRECTORY.

.. code-block:: elisp

   (when (not as-is)
     (setq file-or-pattern (expand-file-name file-or-pattern directory))
     (setq directory (file-name-directory (vc-no-final-slash file-or-pattern))))

Second, if AS-IS is non-nil, the parameters for escaping and anchoring
an ignore pattern are set to an identity function.

If AS-IS is nil, FILE-PATH is made relative to the path of the
directory where the pattern will be stored.  Parameters for escaping
and anchoring an ignore pattern are obtained from the VC backend.

.. code-block:: elisp

   (if as-is
       (setq ignore-param vc-ignore-param-none)
     (when (not (string= (substring file-or-pattern 0 (length ignore-dir))
                         ignore-dir))
       (error "Ignore spec %s is not below project root %s"
              file-or-pattern ignore-dir))
     (setq is-dir (or (file-directory-p file-or-pattern)
                      (vc-has-final-slash file-or-pattern)))
     (setq file-or-pattern (vc-no-final-slash
                            (substring file-or-pattern (length ignore-dir))))
     (setq ignore-param (vc-call-backend backend 'ignore-param ignore-file)))

E.g. an invocation of :elisp:`(vc-ignore "some/rel/path/" "/re/po")`
translates to:

.. code-block:: elisp

   ;; normalize FILE-PATH and DIRECTORY
   (setq file-or-pattern "/re/po/some/rel/path/" )
   (setq directory "/re/po/some/rel/" )

   ;; determine, whether file path is a directory
   (setq is-dir t)
   ;; prepare pattern as relative file path to directory of ignore file
   (setq file-or-pattern "path" )

   ;; obtain parameters for escaping and anchoring an ignore pattern from VC backend

All further processing is identical for verbatim patterns and for file paths.

If you insist on calling a code path difference of 3 lines versus 13
lines two independent functions, then technically you are correct and
we should just remove the extra 13 lines of code (and also the two
separate interactive commands) to get a **properly working**
implementation of the pattern use case for **all** VC backends, which
is still needed, because right now, the vc ignore feature is
incomplete and very buggy.
>  I question its necessity.

>   In the use cases I have in mind,
> the ignore file-or-pattern always comes from the user, because only
> the users can decide which files they want to ignore.
These are my major use cases. I do not have a real use case involving Lisp code, but it is always worth considering.
>   And the users
> always know both which backend they are working with, and whether the
> file-or-pattern is a filename or a pattern.
Yes, I do. 99.9% of the time I want to ignore one or more files in `vc-dir-mode` or `dired-mode`
>   It follows that we can
> ask the users to escape and anchor the file-or-pattern argument they
> type,

Are you aware, that the current implementation of
`vc-dir-ignore` calls `vc-ignore` with a plain absolute file
path obtained from (vc-dir-current-file)?  There is no prompting
whatsoever that would let the user escape and anchor anything.

How would you propose to handle this officially supported use
case (since 2013)?

   (defun vc-dir-ignore (&optional arg)
     "Ignore the current file."
     (interactive "P")
     (vc-ignore
      (if (called-interactively-p 'any)
      (read-string "Make the path relative to the ignore file,
    which may or may not be at the location you expect. Then guess
    the currently active pattern escape syntax, apply it to the file
    path and anchor it: " (vc-dir-current-file))
    (error "This will fail, when called from lisp code."))))

>  and that is not an unreasonable expectation.
If you analyze the above humorous prompt, you will see, that it
is entirely unreasonable, since it is no longer possible to
determine the correct ignore file for a given pattern, unless the
DIRECTORY parameter is also correct, which is always necessary for
CVS, SVN, SRC. So before calling `vc-ignore` I have to change
into the correct directory?
           
1. Locating the correct ignore file is no simple task for Git or
   Hg, which support sub-tree ignore files. The current
   implementation via

   : (expand-file-name ".hgignore" (vc-root-dir))

   is extremely simplistic and should be augmented in a separate
   effort.

2. The current syntax in an Hg ignore file may be either regex
   or glob. Since we are in a collaborative environment, after
   all, anybody may have changed the default syntax since we last
   checked.

3. Nobody knows all ignore spec syntaxes all the time (memory is
   always fading).

4. If I have to contribute to a project that uses an unfamiliar
   VC, it would be extremely reasonable, if my development
   environment (Emacs) would spare me the trouble of totally
   avoidable "ignore file" mistakes. It would be so extremely
   reasonable, that it would be actually unreasonable not to have
   it implemented.

5. I have users, which are only just learning how to program. I
   do not wish for them to delve into just another distraction
   from the task at hand. It would be very reasonable, if Emacs
   helped me keep them on point.
   ("Don't worry, Emacs will handle that for you!")
>   In fact, your
> approach expects the same from the users, because you are asking the
> users to decide which of the two commands to invoke in each case.
I handled that above. If you only ever add files from
`vc-dir-mode` or `dired-mode` you only have to make that decision
once in your lifetime.
> If there are no flaws in my way of reasoning, then I think the
> resulting changes will be much less extensive, because the same
> vc-ignore command can then be used for both ignoring a specific file
> and for ignoring a pattern of any kind.

I can give you that with my implementation as a bonus added interactive command:

   (defun vc-ignore-posixly-correct (file &optional directory remove as-is)
     (interactive
      (let ((remove current-prefix-arg) file as-is)
    (if (not (y-or-n-p "Ignore a file?"))
        (setq file (read-string "Pattern: "))
      (setq as-is (not (y-or-n-p "Escape manually?")))
      (setq file (read-file-name "File: ")))
    (list file nil remove as-is)))
     (vcignore file directory remove as-is))

>   We just need to document that
> if the argument is a pattern of any kind, the user will have to make
> sure it is escaped and anchored for the backend to DTRT.
>
> A further advantage of my proposal is that we don't need to write any
> backend-specific code to escape special characters in patterns,
> because we expect the users to do that.
Since we have to check for the active syntax anyway, it may be
better to abandon the entire ignore feature and just open the
appropriate ignore file for editing. It is a lot simpler and much
more convenient than the minibuffer for editing multiple files.
It would also help the user to determine what the correct
relative file name should be.
>
> 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". If such a command
`grep-for-file-name` were derived from `grep`, a user could
reasonably expect that the file argument is properly quoted as a
grep pattern.

The correct comparison would be with various employments of
`shell-quote-argument`, when passing a string to the shell, which
is known to be interpreted verbatim. Or as just recently reported
in bug #35492, where file arguments for Git commands must be
properly quoted to work correctly (this is not because of the
shell but because of Git!). If this quoting is omitted,
`vc-dir-mode` becomes entirely unreliable for Git. If it is done
quick and dirty instead of properly, it causes other problems.
> Am I missing something?






^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-11  1:45                                           ` Wolfgang Scherer
@ 2020-02-11 17:32                                             ` Eli Zaretskii
  2020-02-11 22:28                                               ` Wolfgang Scherer
  2020-02-12 17:23                                               ` Wolfgang Scherer
  0 siblings, 2 replies; 66+ messages in thread
From: Eli Zaretskii @ 2020-02-11 17:32 UTC (permalink / raw)
  To: Wolfgang Scherer; +Cc: 37189, dgutov

> 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.

> 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?

> 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.

> 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.

> 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.

> 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.

> 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.

> 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.

> > 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.





^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-11 17:32                                             ` Eli Zaretskii
@ 2020-02-11 22:28                                               ` Wolfgang Scherer
  2020-02-12 18:34                                                 ` Eli Zaretskii
  2020-02-12 17:23                                               ` Wolfgang Scherer
  1 sibling, 1 reply; 66+ messages in thread
From: Wolfgang Scherer @ 2020-02-11 22:28 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 37189, dgutov


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\\\)\\\)\:\|\\\) \{\} +
```






^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-11 17:32                                             ` Eli Zaretskii
  2020-02-11 22:28                                               ` Wolfgang Scherer
@ 2020-02-12 17:23                                               ` Wolfgang Scherer
  1 sibling, 0 replies; 66+ messages in thread
From: Wolfgang Scherer @ 2020-02-12 17:23 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 37189, dgutov

[-- Attachment #1: Type: text/plain, Size: 2953 bytes --]

Hi Eli,

I have refactored the API change request according to the latest
discussion. I assume, that the "file path" use case must be
implemented anyway, correct?

I have changed the code, such that the user visible changes are
absolutely minimized. Only one change remains, which never worked
before anyway.

I have also attached a complete sequence diagram, with annotations to
show, that it is not possible to determine the correct ignore file in
a subdirectory with an escaped pattern in all cases.

### API change

Function func:vc-ignore is modified to handle both the "pattern" and the
"file path" use cases.

-   The parameter FILE of *cv-ignore* is renamed to PATTERN-OR-FILE to
    clarify its dual purpose.

-   The additional optional parameter is-file is introduced to determine
    how the PATTERN-OR-FILE argument should be treated. If it is nil
    (the default) the behavior is unchanged. Otherwise, a file path is
    expected, which is transformed to fit the ignore file location and
    then escaped according to the VC pattern syntax.

-   The documenation string of *vc-ignore* is clarified to avoid
    possible misconceptions,like a file somehow being equivalent to a
    wildcard, or that glob(7) patterns are the only type of patterns for
    version control systems.

-   For the "file path" use case, users have come to expect that they
    can mark several file in *vc-dir-mode* and *dired-mode* to perform
    VC operations on that fileset. This is implemented in function
    *vc-ignore-fileset*. in *dired-mode* the user is prompted before
    making any changes (similar to a delete operation).

-   The new function *vc-ignore-pattern* is an alias for *vc-ignore* to
    allow for a clear and unambiguous documentation string that does not
    mix use cases.

-   The new function *vc-ignore-file*, when called interactively, either
    operates on the current fileset in *vc-dir-mode* and *dired-mode* or
    prompts for a file to ignore.

-   The function *vc-dir-ignore* is modified to call
    *vc-ignore-pattern* interactively. After processing, a message is
    displayed in the echo area with a hint to press "F" for ignoring files.

### Proposed keyboard shortcuts

`C-x v F` => *vc-ignore-file*\
`C-x v G` => *vc-ignore-pattern* ; functionality unchanged

in *vc-dir-mode*:

`F` => *vc-ignore-file*\
`G` => *vc-dir-ignore* ; functionality was broken

### User-visible changes

The only user-visible change in the current features is the keyboard
shortcut "G" in *vc-dir-mode*. It does no longer write an invalid
absolute file path into the ignore file, but prompts for a pattern
instead. An additional hint to press "F" for ignoring files is displayed
afterwards.

The only new feature is *vc-ignore-file* for ignoring an unescaped |file path|.

Greetings, Wolfgang



[-- Attachment #2: vc-ignore-sequence-diagram.png --]
[-- Type: image/png, Size: 94935 bytes --]

^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-11 22:28                                               ` Wolfgang Scherer
@ 2020-02-12 18:34                                                 ` Eli Zaretskii
       [not found]                                                   ` <6f3ba261-e1f9-cf19-cc22-ec8c24cf3298@gmx.de>
  0 siblings, 1 reply; 66+ messages in thread
From: Eli Zaretskii @ 2020-02-12 18:34 UTC (permalink / raw)
  To: Wolfgang Scherer; +Cc: 37189, dgutov

> Cc: dgutov@yandex.ru, 37189@debbugs.gnu.org
> From: Wolfgang Scherer <Wolfgang.Scherer@gmx.de>
> Date: Tue, 11 Feb 2020 23:28:10 +0100
> 
> 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.

What is the "file path use case"?  To what commands does it pertain?

> >> 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).

Do we support per-directory ignore files in the current codebase?  I
think we don't: I see that every backend simply looks in the root of
the repository.  If I'm right, then supporting per-directory ignore
files is an enhancement, which should then be considered separately
from fixing bugs in the current code.

> 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.

I don't think we support such nesting at this time, do we?

> > 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.

If I'm right in saying that we currently support only ignore files in
the root of the repository, then this is not an issue.  And even if it
is an issue, we already have a backend method to find the ignore file,
so we could use it (or modify it if needed).

> >> 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)

OK, so you _did_ mean a different kind of feedback.  What I meant is
the "/foo/bar/.ignore written" feedback.

> > 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's not what I see, at least not with Git as the backend.  I see
only the basename of the file being added to the ignore file.  Can you
show a use case where an absolute file name is written into the ignore
file by vc-dir-ignore?

> > 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!

True, but I don't think this detail matters for the purposes of the
analogy.

> There is no way, that the pattern argument for the *grep* command
> could be reliably parsed and quoted.

Of course it's possible.  It's just very complex and tedious, and more
importantly, requires the user to play by certain rules: e.g., the
user must agree never to quote/escape the patterns he/she types.
Instead, we give the user the freedom to decide when to quote and
when not to quote.  The same should be done with vc-ignore (but not
with vc-dir-ignore).





^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
       [not found]                                                   ` <6f3ba261-e1f9-cf19-cc22-ec8c24cf3298@gmx.de>
@ 2020-02-12 23:20                                                     ` Wolfgang Scherer
  2020-02-13  1:18                                                       ` Wolfgang Scherer
  0 siblings, 1 reply; 66+ messages in thread
From: Wolfgang Scherer @ 2020-02-12 23:20 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 37189, dgutov

Am 12.02.20 um 19:34 schrieb Eli Zaretskii:
>> From: Wolfgang Scherer <Wolfgang.Scherer@gmx.de>
>> Date: Tue, 11 Feb 2020 23:28:10 +0100
>>
>> 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.
> What is the "file path use case"?  To what commands does it pertain?

The command is vc-dir-ignore. You already responded with:

> 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.

Let me just remind you, that the key shortcut in vc-dir-mode dates back to 2013:

    vc-dir.el (Xue Fuqiao 2013-07-30  305)     (define-key map "G" 'vc-dir-ignore)

It is not new, and the intention was always to support ignoring files
from vc-dir-mode wihtout escaping before calling vc-ignore, which is
really not necessary at all.

>>>> 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).
> Do we support per-directory ignore files in the current codebase?  I
> think we don't: I see that every backend simply looks in the root of
> the repository.

That is only true for per-tree VCs, because per-directory VCs do not
necessarily have a "root":

    vc-bzr.el681:(defun vc-bzr-find-ignore-file (file)
                    (vc-bzr-root file)))
    vc-git.el974:(defun vc-git-find-ignore-file (file)
                    (vc-git-root file)))
    vc-hg.el1210:(defun vc-hg-find-ignore-file (file)
                    (vc-hg-root file)))
    vc-mtn.el105:(defun vc-mtn-find-ignore-file (file)
                    (expand-file-name ".mtnignore" (vc-mtn-root file)))

You are missing the higher priority backend hook vc-backend-ignore,
which is used by CVS and SVN. The original implementations (before my
changes) are very pristine.

The oldest stuff - the CVS code - was mainly imported from pcl-cvs
(vc-cvs-append-to-ignore), which has always supported the "file path"
use case. In the thin vc-cvs-ignore wrapper the use case is also very
much "file path", albeit incomplete. I.e., for an absolute file path,
the correct .cvsignore is found, which can very well be a sub
directory of the repository root. If the FILE argument were made
relative to the effective directory, the code would actually be
correct. See also bug #37215.

    vc-cvs.el (Glenn Morris 2013-09-11 1223) (defun vc-cvs-ignore (file &optional _directory _remove)
    vc-cvs.el (Xue Fuqiao   2013-07-30 1224)   "Ignore FILE under CVS."
    vc-cvs.el (Xue Fuqiao   2013-09-20 1225)   (vc-cvs-append-to-ignore (file-name-directory file) file))
    vc-cvs.el (Xue Fuqiao   2013-07-30 1226)
    vc-cvs.el (Xue Fuqiao   2013-09-20 1227) (defun vc-cvs-append-to-ignore (dir str &optional old-dir)
    vc-cvs.el (Xue Fuqiao   2013-07-30 1228)   "In DIR, add STR to the .cvsignore file.
    vc-cvs.el (Xue Fuqiao   2013-07-30 1229) If OLD-DIR is non-nil, then this is a directory that we don't want
    vc-cvs.el (Xue Fuqiao   2013-07-30 1230) to hear about anymore."
    vc-cvs.el (Xue Fuqiao   2013-07-30 1231)   (with-current-buffer
    vc-cvs.el (Xue Fuqiao   2013-07-30 1232)       (find-file-noselect (expand-file-name ".cvsignore" dir))
    vc-cvs.el (Xue Fuqiao   2013-07-30 1233)     (when (ignore-errors
    vc-cvs.el (Xue Fuqiao   2013-07-30 1234)        (and buffer-read-only
    vc-cvs.el (Xue Fuqiao   2013-07-30 1235)             (eq 'CVS (vc-backend buffer-file-name))
    vc-cvs.el (Xue Fuqiao   2013-07-30 1236)             (not (vc-editable-p buffer-file-name))))
    vc-cvs.el (Xue Fuqiao   2013-07-30 1237)       ;; CVSREAD=on special case
    vc-cvs.el (Xue Fuqiao   2013-07-30 1238)       (vc-checkout buffer-file-name t))
    vc-cvs.el (Xue Fuqiao   2013-07-30 1239)     (goto-char (point-max))
    vc-cvs.el (Xue Fuqiao   2013-07-30 1240)     (unless (bolp) (insert "\n"))
    vc-cvs.el (Xue Fuqiao   2013-07-30 1241)     (insert str (if old-dir "/\n" "\n"))
    vc-cvs.el (Glenn Morris 2013-09-11 1242)     ;; FIXME this is a pcvs variable.
    vc-cvs.el (Glenn Morris 2013-09-11 1243)     (if (bound-and-true-p cvs-sort-ignore-file)
    vc-cvs.el (Glenn Morris 2013-09-11 1244)         (sort-lines nil (point-min) (point-max)))
    vc-cvs.el (Xue Fuqiao   2013-07-30 1245)     (save-buffer)))

The SVN implementation is somewhat confusing, because for the "file
path" use case a relative path with possible subdirectories is
constructed, but not normalized.

    vc-svn.el (Dmitry Gutov 2014-10-03 354) (defun vc-svn-ignore (file &optional directory remove)
    vc-svn.el (Xue Fuqiao   2013-08-04 355)   "Ignore FILE under Subversion.
    vc-svn.el (Xue Fuqiao   2013-09-04 356) FILE is a file wildcard, relative to the root directory of DIRECTORY."
    vc-svn.el (Dmitry Gutov 2014-10-03 357)   (let* ((ignores (vc-svn-ignore-completion-table directory))
    vc-svn.el (Dmitry Gutov 2014-10-03 358)          (file (file-relative-name file directory))
    vc-svn.el (Dmitry Gutov 2014-10-03 359)          (ignores (if remove
    vc-svn.el (Dmitry Gutov 2014-10-03 360)                       (delete file ignores)
    vc-svn.el (Dmitry Gutov 2014-10-03 361)                     (push file ignores))))
    vc-svn.el (Dmitry Gutov 2014-10-03 362)     (vc-svn-command nil 0 nil nil "propset" "svn:ignore"
    vc-svn.el (Dmitry Gutov 2014-10-03 363)                     (mapconcat #'identity ignores "\n")
    vc-svn.el (Dmitry Gutov 2014-10-03 364)                     (expand-file-name directory))))

Since I never thought of a "pattern" use case, I submitted a fix for
SVN, which adds the basename to the correct sub directory:

    vc-svn.el (Wolf Scherer 2019-10-07 357) FILE is a wildcard specification, either relative to
    vc-svn.el (Wolf Scherer 2019-10-07 358) DIRECTORY or absolute."
    vc-svn.el (Wolf Scherer 2019-10-07 359)   (let* ((path (directory-file-name (expand-file-name file directory)))
    vc-svn.el (Wolf Scherer 2019-10-07 360)          (directory (file-name-directory path))
    vc-svn.el (Wolf Scherer 2019-10-07 361)          (file (file-name-nondirectory path))
    vc-svn.el (Wolf Scherer 2019-10-07 362)          (ignores (vc-svn-ignore-completion-table directory))

I concede, that for simple glob(7) expressions, it is pretty much
possible to get away with treating the wildcard specification as a
filename. And sure, by declaring a use case *rare* (although there are
infinitely many instances and one is enough to trigger an error) the
need for escaping literal filenames can be argued away.

However, with regular expression syntax, multiple syntax options and
sub-tree ignore files those times are long gone.

>   If I'm right, then supporting per-directory ignore
> files is an enhancement, which should then be considered separately
> from fixing bugs in the current code.

Per-directory means specifically, that the scope of the ignore
specification is the directory of the ignore file. And that has
always been supported.

The per-sub-tree ignore files are a feature of Git and Hg, where the
scope of the ignore specifications also extends into further
subdirectories. These are not supported and should definitely not be
discussed here.

>> 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.
> I don't think we support such nesting at this time, do we?

Well, we actually do. With the "file path" use case (as implemented in
vc-svn-ignore) it is perfectly possible to just press "G" on say
"sub/something" or "sub/sub2/else" and they will be ignored for SVN
correctly.

The VC backend nesting is also supported perfectly (I was
over-dramatizing for effect, sorry). The variable vc-handled-backends
determines what backends are recognized and what their priority
is. So, to solve the problem of using "pattern" mode in the above
example of nested repositories,

   (setq vc-handled-backends '(SVN CVS RCS))

is sufficient to suppress recognition of the inner VC backends. Then
you could "pattern" away for SVN to your hearts content.

>>> 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.
> If I'm right in saying that we currently support only ignore files in
> the root of the repository, then this is not an issue.

Since you are not right, that could have been the end of it ;-)

> And even if it
> is an issue, we already have a backend method to find the ignore file,
> so we could use it (or modify it if needed).

You obviously have not looked at the implementation yet. I introduce
all the missing backend methods to find ignore files (CVS, SRC, SVN
(virtual, just for directory determination)) to make the
implementation totally uniform.

If you use these methods in a front end, you obviously have to
determine the backend. And after locating the ignore file, vc-ignore
can no longer be called, because neither the backend, nor the
ignorefile can be specified. So you have to call vc-default-ignore
with the backend. You are actually programming a backend function now
and much of the frontend/backend separation goes out the window.

Then you would have to cut and paste the same code into other
functions that also need to do the same thing. Then you will realize,
that it is better to move the stuff to the backend default
implementations where it really belongs. And so you will just end up
with the optimized implementation that I am offering ;-)

>>>> 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)
> OK, so you _did_ mean a different kind of feedback.  What I meant is
> the "/foo/bar/.ignore written" feedback.

I repeated that in the vc-ignore pattern prompt, where it is much more
useful to know, which ignore file is actually used ;-)

>>> 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's not what I see, at least not with Git as the backend.  I see
> only the basename of the file being added to the ignore file.

Right, and that is incorrect for all cases, because it is

1. not anchored, i.e.it also matches files with the same name in
   subdirectories,
2. if the basename comes from a file in a subdirectory, it also
   matches files with the same name in the root directory.

These reasons also apply to Bzr,Hg, Mtn, since the same functions are
used. BTW, you are seeing these incorrect filenames so flawlessly,
because I fixed the underlying functions in #37185 extensively.

Originally, SVN at least got it right for files in the root directory,
but none of the ignore specs for files in a subdirectory ever matched
anything, which is probably better than matching the wrong thing..

> Can you
> show a use case where an absolute file name is written into the ignore
> file by vc-dir-ignore?

Yes, CVS is still broken in that manner, because the reviewer of
#37215 thinks, that FILE is always a basename only.

And there we are, *all* backend implementations fail utterly for
vc-dir-ignore. Not just *rare*, but *almost all* cases (except root
directory of SVN repo).

>>> 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!
> True, but I don't think this detail matters for the purposes of the
> analogy.
>> There is no way, that the pattern argument for the *grep* command
>> could be reliably parsed and quoted.
> Of course it's possible.

No it is not! Without quoting, you cannot decide programmatically
where the argument ends and the first filename begins, if the argument
contains spaces (or a multitude of other characters). I will not
accept "machine learning" as an answer ;-).

> It's just very complex and tedious, and more
> importantly, requires the user to play by certain rules: e.g., the
> user must agree never to quote/escape the patterns he/she types.
> Instead, we give the user the freedom to decide when to quote and
> when not to quote.  The same should be done with vc-ignore
> (but not with vc-dir-ignore).

Aaah, so! It's not a *bug*, it's a *feature*! ;-)

Anyway, that is exactly the point I am trying to make.

As it is and always will be, vc-ignore does what it does and I am not
changing it.  I am just adding another option, a freedom of choice, if
you will. Invoke vc-ignore under a different name, supply a file path
and magically, the correct ignore file is located and the proper
quoting is applied. This enables vc-dir-mode to work correctly.

And as I said before, once you think it through you will end up at the
same conclusion: It is always best to have a single call chaing
that covers all cases. If it works, it works for all of them. No need
to maintain multiple cut and paste code snippets in various frontend
functions and backends. (The confusion with the old vc-cvs-ignore and
vc-svn-ignore that you have overlooked previously shows that).

I have implemented some options for prompts from vc-ignore in
vc-dir-mode (the completion collection contains an empty line and the
unquoted file name).

    VC backend : Hg
    Working dir: /srv/install/linux/emacs/

        edited               README-emacs-vc-ignore-feature.txt

                             doc/_static/
        unregistered         doc/_static/x-vc-repair.el-002
        unregistered         doc/_static/x-vc-repair.el-003

Pressing "G" on the line "doc/_static/x-vc-repair.el-002" displays:

    Add pattern verbatim to .hgignore: ^doc/_static/x\-vc\-repair\.el\-002$

Pressing :kbd:`M-n` or :kbd:`<down>` clears the prompt:

    Add pattern verbatim to .hgignore:

Pressing :kbd:`M-n` or :kbd:`<down>` shows the unquoted file name:

    Add pattern verbatim to .hgignore: doc/_static/x-vc-repair.el-002

Invoking :kbd:`C-x v G` in subdirectory "doc/_static" of the
repository prompts with:

    Add pattern verbatim to ../../.hgignore:

Which tells you where the pattern will go when you press ENTER.

How is that for freedom of choice?






^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-12 23:20                                                     ` Wolfgang Scherer
@ 2020-02-13  1:18                                                       ` Wolfgang Scherer
  2020-02-13 15:09                                                         ` Eli Zaretskii
  2020-02-13 15:21                                                         ` Eli Zaretskii
  0 siblings, 2 replies; 66+ messages in thread
From: Wolfgang Scherer @ 2020-02-13  1:18 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 37189, dgutov

Hi Eli,

you must be using an Emacs < 27, if you get basenames in root/.gitignore, because that behavior changed with #37217:

> Only the basename of FILE is written to the ignore file. This is
> wrong for all filenames relative to project root with one ore
> more parent directories.
>
> The remove option is also implemented incorrectly.

I have totally forgotten, that I never really considered the "pattern"
use case you are so fond of, because the "file path" (vc-dir-ignore)
use case was so much more obvious to me.  Since I have fixed so much
of vc's shortcomings already, I have not really thought of the
original behavior until you asked me to show, why vc-ignore had always
been broken.

So, I just tried this in a Mercurial repository (it is fine to
use Git, as long as you can pretend that regular expressions are
valid wildcards):

1. Invoke vc-ignore `C-x v G`

2. Enter the correct regular expression for Mercurial (it is a
   wildcard for the VC according to your reasoning, is it not?):

   ^/some[/]sub

3. Check your .hgignore file and verify, that it only contains:

   ]sub

How is that behavior correct?  There should have been no change,
because the user is supposed to do everything themselves, right?

So how come, the correct documentation for vc-ignore before Emacs
27 is basically:

    (defun (vc-ignore FILE &optional DIRECTORY REMOVE)
      "...
    You will be prompted for an absolute file path, but don't think
    that it has any significance, because for Bzr, Git, Hg, Mnt:

    If you enter anything that ends with a slash, the string "./" is
    written into the nearest ignore file.

    Otherwise, everything up to and including the last slash is thrown
    away and the rest is written into the nearest ignore file.

    For CVS, whatever is entered is used as a filename to determine
    the directory of the ignore file. What ever was entered is then
    written entirely into that ignore file without modification, which
    makes no sense.

    For SVN, other strange things happen."

Applying `file-name-nondirectory` to an escaped pattern *never makes
any sense at all*, so why does it happen?

The simplest explanation is, that there is no "pattern" use case at
all. There is really only one use case, which is to ignore files by
pressing `G` in vc-dir-mode. The additional shortcut `C-x v G` for
vc-ignore is just there to make things symmetrical with other VC
commands like `i` and `C-x v i` and so forth.

That is also the reason why vc-ignore prompts for an absolute file
name. Because it is the exact same input it gets from
vc-dir-ignore. Case closed. vc-ignore does not have a pattern mode. It
was always broken entirely. Period.

So the implementation you are talking about has never existed, you are
just trying to somehow justify the broken behavior by making stuff up
and insisting that the behavior is intentional. Let me ensure you,
that it is not.

And if something is so utterly broken like the original vc ignore
feature, we are not talking about an API change but about a proper
refactoring to get it working at all. Any solution is better than no
solution.

I actually like the idea of a "pattern" input, so I am not sad about
the lost time. But I will now quit this absurd discussion of
non-existing algorithms and just keep working with my correct
implementation of the vc ignore feature.

Should you get your facts straight, we can talk further. Otherwise, I
have invested enough.time now.

Am 13.02.20 um 00:20 schrieb Wolfgang Scherer:
>>>> 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's not what I see, at least not with Git as the backend.  I see
>> only the basename of the file being added to the ignore file.
> Right, and that is incorrect for all cases, because it is
>
> 1. not anchored, i.e.it also matches files with the same name in
>    subdirectories,
> 2. if the basename comes from a file in a subdirectory, it also
>    matches files with the same name in the root directory.
>
> These reasons also apply to Bzr,Hg, Mtn, since the same functions are
> used. BTW, you are seeing these incorrect filenames so flawlessly,
> because I fixed the underlying functions in #37185 extensively.
>
> Originally, SVN at least got it right for files in the root directory,
> but none of the ignore specs for files in a subdirectory ever matched
> anything, which is probably better than matching the wrong thing..
>
>> Can you
>> show a use case where an absolute file name is written into the ignore
>> file by vc-dir-ignore?
> Yes, CVS is still broken in that manner, because the reviewer of
> #37215 thinks, that FILE is always a basename only.
>
> And there we are, *all* backend implementations fail utterly for
> vc-dir-ignore. Not just *rare*, but *almost all* cases (except root
> directory of SVN repo).
>





^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-13  1:18                                                       ` Wolfgang Scherer
@ 2020-02-13 15:09                                                         ` Eli Zaretskii
  2020-02-13 16:30                                                           ` Wolfgang Scherer
  2020-02-13 23:43                                                           ` Richard Stallman
  2020-02-13 15:21                                                         ` Eli Zaretskii
  1 sibling, 2 replies; 66+ messages in thread
From: Eli Zaretskii @ 2020-02-13 15:09 UTC (permalink / raw)
  To: Wolfgang Scherer; +Cc: 37189, dgutov

> From: Wolfgang Scherer <Wolfgang.Scherer@gmx.de>
> Cc: dgutov@yandex.ru, 37189@debbugs.gnu.org
> Date: Thu, 13 Feb 2020 02:18:00 +0100
> 
> I will now quit this absurd discussion of non-existing algorithms
> and just keep working with my correct implementation of the vc
> ignore feature.
> 
> Should you get your facts straight, we can talk further. Otherwise, I
> have invested enough.time now.

That is unfortunate, but it's your prerogative, of course.

From my POV, I can say that the amount of time I invested in this
discussion, and in studying the code you mentioned and the use cases
you described, was also unusually high.  This was exacerbated by your
frequently unkind language, like in the small excerpt above.  This
made the discussion unpleasant, to say the least.

I will remove your address from any further discussions of this issue,
per your wish.





^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-13  1:18                                                       ` Wolfgang Scherer
  2020-02-13 15:09                                                         ` Eli Zaretskii
@ 2020-02-13 15:21                                                         ` Eli Zaretskii
  2020-02-13 23:40                                                           ` Dmitry Gutov
  1 sibling, 1 reply; 66+ messages in thread
From: Eli Zaretskii @ 2020-02-13 15:21 UTC (permalink / raw)
  To: dgutov; +Cc: 37189

I'm replying to a couple of points here FTR, since the future of this
discussion (and of handling these issues) is not clear to me now.

> From: Wolfgang Scherer <Wolfgang.Scherer@gmx.de>
> Cc: dgutov@yandex.ru, 37189@debbugs.gnu.org
> Date: Thu, 13 Feb 2020 02:18:00 +0100
> 
> you must be using an Emacs < 27, if you get basenames in root/.gitignore, because that behavior changed with #37217:

No, I'm using the latest emacs-27 branch.

However, I see that "basename" was inaccurate: it is only correct when
ignoring a file in the root directory of the repository.  A more
accurate description would be "a file name relative to the
repository's root".  Which is still not an absolute file name, as was
claimed by Wolfgang.

> 1. Invoke vc-ignore `C-x v G`
> 
> 2. Enter the correct regular expression for Mercurial (it is a
>    wildcard for the VC according to your reasoning, is it not?):
> 
>    ^/some[/]sub
> 
> 3. Check your .hgignore file and verify, that it only contains:
> 
>    ]sub

I cannot reproduce this: I get the expected ^/some[/]sub there.

> Should you get your facts straight, we can talk further. Otherwise, I
> have invested enough.time now.

Dmitry, what happens from here is up to you.  You can decide to accept
Wolfgang's changes as-is, or you can decide you want to continue
discussing this with him (in which case I will stay away of the
discussions) and eventually arrive at some alternative changeset.  Or
you can make any other decision about this.

I did arrive at a few conclusions after studying the issues raised in
the discussions, so if you want, I can post those conclusions FTR, if
for nothing else.

Sorry if I caused this discussion to go awry.





^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-13 15:09                                                         ` Eli Zaretskii
@ 2020-02-13 16:30                                                           ` Wolfgang Scherer
  2020-02-13 23:43                                                           ` Richard Stallman
  1 sibling, 0 replies; 66+ messages in thread
From: Wolfgang Scherer @ 2020-02-13 16:30 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 37189, dgutov

Dear Eli,


Am 13.02.20 um 16:09 schrieb Eli Zaretskii:
>> From: Wolfgang Scherer <Wolfgang.Scherer@gmx.de>
>> Cc: dgutov@yandex.ru, 37189@debbugs.gnu.org
>> Date: Thu, 13 Feb 2020 02:18:00 +0100
>>
>> I will now quit this absurd discussion of non-existing algorithms
>> and just keep working with my correct implementation of the vc
>> ignore feature.
>>
>> Should you get your facts straight, we can talk further. Otherwise, I
>> have invested enough.time now.
> This was exacerbated by your
> frequently unkind language, like in the small excerpt above.  This
> made the discussion unpleasant, to say the least.
I am sorry, if my wording made the discussion unpleasant for you. That
was not my intent.

It was really my misconception that you were more experienced than me
regarding the code in the vc package, which made me doubt my
reasoning. And if that happens, I tend to lash out, which is a
personal flaw I sincerely apologize for.

I do have to thank you for making me produce a much higher quality of
code than I would have without the misunderstanding. So I may have not
been pleased with the experience, but I am very pleased with the
result.

Thank you.
>
> I will remove your address from any further discussions of this issue,
> per your wish.

I hope the apology still reaches you.







^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-13 15:21                                                         ` Eli Zaretskii
@ 2020-02-13 23:40                                                           ` Dmitry Gutov
  2020-02-14  9:23                                                             ` Eli Zaretskii
  0 siblings, 1 reply; 66+ messages in thread
From: Dmitry Gutov @ 2020-02-13 23:40 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 37189

Hi Eli,

On 13.02.2020 17:21, Eli Zaretskii wrote:
> Dmitry, what happens from here is up to you.  You can decide to accept
> Wolfgang's changes as-is, or you can decide you want to continue
> discussing this with him (in which case I will stay away of the
> discussions) and eventually arrive at some alternative changeset.  Or
> you can make any other decision about this.

I think the first thing I'll have to do is revert a part of an earlier 
patch to vc-default-ignore which changed its semantics a little, because 
it doesn't look like this discussion is going to culminate in a patch 
small enough for emacs-27 anyway.

Then, naturally, we'll have to look for small changes that improve the 
situation but provide as little breakage as possible. I pretty much 
agree that vc-ignore's semantics are not well thought through (or 
documented), and there is space for improvement. But the devil is in the 
details. I do have a few of my own questions/ideas, but I'll leave those 
until later.

> I did arrive at a few conclusions after studying the issues raised in
> the discussions, so if you want, I can post those conclusions FTR, if
> for nothing else.

Yes, of course, please go ahead.





^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-13 15:09                                                         ` Eli Zaretskii
  2020-02-13 16:30                                                           ` Wolfgang Scherer
@ 2020-02-13 23:43                                                           ` Richard Stallman
  2020-02-14  1:49                                                             ` Wolfgang Scherer
  1 sibling, 1 reply; 66+ messages in thread
From: Richard Stallman @ 2020-02-13 23:43 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Wolfgang.Scherer, 37189, dgutov

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

It is a shame, and a loss for the GNU Project, when people who want to
contribute have difficulty communicating.  Would the two of you like
to try to reset your relationship, and start the conversation again?

You could look at https://gnu.org/philosophy/kind-communication.html
to help get off to a smoother start this time.

-- 
Dr Richard Stallman
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)







^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-13 23:43                                                           ` Richard Stallman
@ 2020-02-14  1:49                                                             ` Wolfgang Scherer
  2020-02-16  2:29                                                               ` Richard Stallman
  0 siblings, 1 reply; 66+ messages in thread
From: Wolfgang Scherer @ 2020-02-14  1:49 UTC (permalink / raw)
  To: rms, Eli Zaretskii; +Cc: 37189, dgutov

Am 14.02.20 um 00:43 schrieb Richard Stallman:
> [[[ To any NSA and FBI agents reading my email: please consider    ]]]
> [[[ whether defending the US Constitution against all enemies,     ]]]
> [[[ foreign or domestic, requires you to follow Snowden's example. ]]]
>
> It is a shame, and a loss for the GNU Project, when people who want to
> contribute have difficulty communicating.  Would the two of you like
> to try to reset your relationship, and start the conversation again?

I did already apologize, and I meant it.
I promise to exercise restraint in the future.

As for the issue at hand, I will just go ahead and try to make a MELPA
package at last, since even if the issue were resolved in Emacs 27, it
would not help with the situation that I am stuck with Emacs 22-26 on
various hosts.

A MELPA package under the GPL also means that there is no loss to the
GNU Project, right?






^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-13 23:40                                                           ` Dmitry Gutov
@ 2020-02-14  9:23                                                             ` Eli Zaretskii
  2020-02-21  0:05                                                               ` Dmitry Gutov
  0 siblings, 1 reply; 66+ messages in thread
From: Eli Zaretskii @ 2020-02-14  9:23 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 37189

> Cc: 37189@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Fri, 14 Feb 2020 01:40:31 +0200
> 
> I think the first thing I'll have to do is revert a part of an earlier 
> patch to vc-default-ignore which changed its semantics a little, because 
> it doesn't look like this discussion is going to culminate in a patch 
> small enough for emacs-27 anyway.

Which part?  Can you show a proposed patch?

> Then, naturally, we'll have to look for small changes that improve the 
> situation but provide as little breakage as possible.

Agreed.

> > I did arrive at a few conclusions after studying the issues raised in
> > the discussions, so if you want, I can post those conclusions FTR, if
> > for nothing else.
> 
> Yes, of course, please go ahead.

OK, here goes:

1) vc-dir-ignore:

It calculates the file name for the entry into the ignore file, and
thus must escape any characters in the file name that are special in
ignore files -- which is backend-specific.

The file name should also be "anchored", at least ideally, so that no
other file is accidentally ignored.  This is also backend-specific.

VC's support for directory-specific ignore files is inconsistent: they
are supported for CVS (and actually mandatory there) and maybe SVN,
but not for Git/Bazaar/Mercurial/Monotone -- for the latter we only
support ignore files in the repository root.  This could be improved
in the future, but for now I don't see an immediate need.

The fact that vc-dir-ignore calls vc-ignore produces contradictions,
because vc-ignore is supposed to handle file-name patterns (whether
wildcards or regexps), whereas vc-dir-ignore never sends patterns, and
also because with vc-ignore the user should get to decide whether to
anchor the file name or pattern.  So I think vc-dir-ignore's
implementation should be eventually rewritten to call the backends
directly instead of relying on vc-ignore.

2) vc-ignore:

Prompts the user for the file name or pattern to add to the ignore
file, and therefore the escaping and anchoring is the user's
responsibility.  It should basically just find the ignore file and
add/remove an entry to/from there.

It should not assume the argument is a simple file name, therefore
using the likes of expand-file-name and file-name-nondirectory is not
a good idea.  For example, Git patterns that are "anchored" start with
a slash, which will cause unexpected results if used in a "file name".
As another example, backslashes used for escaping special characters
will be interpreted on Windows as directory separators, again leading
to unexpected results.  Therefore, vc-default-ignore should not use
file-name related primitives, but instead use concat and substring (if
needed).

For the same reason, using read-file-name here is too naïve, as well
as yielding an absolute file name from what the user types.

If you agree with the above, we then need to decide what, if anything,
of this should be fixed for Emacs 27.  E.g., vc-dir-ignore seems to be
OK if the file name doesn't include special characters, does it?

Thanks.





^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-14  1:49                                                             ` Wolfgang Scherer
@ 2020-02-16  2:29                                                               ` Richard Stallman
  0 siblings, 0 replies; 66+ messages in thread
From: Richard Stallman @ 2020-02-16  2:29 UTC (permalink / raw)
  To: Wolfgang Scherer; +Cc: 37189, dgutov

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > A MELPA package under the GPL also means that there is no loss to the
  > GNU Project, right?

That depends.  If its copyright is assigned to the FSF, we can put
it in GNU ELPA.  That is no loss.

However, when a package goes in MELPA because of copyright, and we
can't include it in ELPA, that is somewhat of a problem.
-- 
Dr Richard Stallman
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)







^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-14  9:23                                                             ` Eli Zaretskii
@ 2020-02-21  0:05                                                               ` Dmitry Gutov
  2020-02-21  8:10                                                                 ` Eli Zaretskii
  2020-02-21 22:22                                                                 ` Wolfgang Scherer
  0 siblings, 2 replies; 66+ messages in thread
From: Dmitry Gutov @ 2020-02-21  0:05 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 37189, Wolfgang Scherer

[-- Attachment #1: Type: text/plain, Size: 4159 bytes --]

On 14.02.2020 11:23, Eli Zaretskii wrote:
>> Cc: 37189@debbugs.gnu.org
>> From: Dmitry Gutov <dgutov@yandex.ru>
>> Date: Fri, 14 Feb 2020 01:40:31 +0200
>>
>> I think the first thing I'll have to do is revert a part of an earlier
>> patch to vc-default-ignore which changed its semantics a little, because
>> it doesn't look like this discussion is going to culminate in a patch
>> small enough for emacs-27 anyway.
> 
> Which part?  Can you show a proposed patch?

See https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37217.

The patch fixed a legitimate scenario, but made an existing one work 
worse. Trying to vc-ignore, say, '*.c' from a subdirectory in a Git 
repo, for instance, will now prepend the intermediary directories to it.

(Mentioned this before: 
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37189#41)

Now, one could argue that one use case is more important than the other 
one, and that vc-ignore has for a while been geared toward entering 
plain file names rather than glob patterns. I have little opinion on 
this subject, however, since I mostly edit ignore files by hand, and do 
so very rarely. So as far as I'm concerned, we could remove this feature 
altogether and not lose much.

But maybe it's more valuable for SVN users? Where the ignore 
configuration is more tricky and the DIRECTORY argument is actually 
important.

So the options at hand are:

- Revert almost all of the patch from bug#37217, reverting to the 
previous, admittedly broken behavior, and continue to discuss a better 
improvement for Emacs 28.
- Try to resolve the ambiguity of purpose in favor either entering 
patterns on file names only. Probably the latter, later vc-ignore to 
vc-ignore-file.
- Try to sit on both chairs... Basically, that means using the user 
input unaltered. Allowing them to enter a file name as well, but treat 
it as a pattern, without escaping or the like. This would be close to 
the original intent behind vc-ignore, AFAICT.

To do the last one, read-file-name would need to be called with the 
second argument provided, the directory against which the file path 
should be relative. For most backend, we can reuse the find-ignore-file 
backend command, but SVN (and RCS, etc) don't have it defined. Roughly 
and handwavy, we can take this case to mean "use default-directory".

As you noted, the use of read-file-name at all in vc-ignore is somewhat 
problematic, but let's see if we can keep the function sane without 
removing it first.

Attaching a patch. Eli, Wolfgang, any objections?

>> Then, naturally, we'll have to look for small changes that improve the
>> situation but provide as little breakage as possible.
> 
> Agreed.

Alas, the attached patch is probably not a "small" one. Option 1 would 
pass this criterion, though.

> 1) vc-dir-ignore:
> 
> It calculates the file name for the entry into the ignore file, and
> thus must escape any characters in the file name that are special in
> ignore files -- which is backend-specific.
> 
> The file name should also be "anchored", at least ideally, so that no
> other file is accidentally ignored.  This is also backend-specific.

I think we can add a new backend action that would escape and anchor a 
file name (and maybe turn it from an absolute into a relative one). That 
should take care of it.

> VC's support for directory-specific ignore files is inconsistent: they
> are supported for CVS (and actually mandatory there) and maybe SVN,
> but not for Git/Bazaar/Mercurial/Monotone -- for the latter we only
> support ignore files in the repository root.  This could be improved
> in the future, but for now I don't see an immediate need.

Agreed.

> For the same reason, using read-file-name here is too naïve, as well
> as yielding an absolute file name from what the user types.

For now, I kept the former but decided against the latter.

> If you agree with the above, we then need to decide what, if anything,
> of this should be fixed for Emacs 27.  E.g., vc-dir-ignore seems to be
> OK if the file name doesn't include special characters, does it?

With the attached patch, it needed a small adjustment, but with that 
should work okay-ish again.

[-- Attachment #2: vc-ignore.diff --]
[-- Type: text/x-patch, Size: 3346 bytes --]

diff --git a/lisp/vc/vc-dir.el b/lisp/vc/vc-dir.el
index 033cb27e33..e5c5e16a17 100644
--- a/lisp/vc/vc-dir.el
+++ b/lisp/vc/vc-dir.el
@@ -879,7 +879,9 @@ vc-dir-ignore
 	   (vc-ignore (vc-dir-fileinfo->name filearg))
 	   t))
        vc-ewoc)
-    (vc-ignore (vc-dir-current-file))))
+    (vc-ignore
+     (file-relative-name (vc-dir-current-file))
+     default-directory)))
 
 (defun vc-dir-current-file ()
   (let ((node (ewoc-locate vc-ewoc)))
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index ec252b74d4..72bd4d3910 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -1406,16 +1406,21 @@ vc-ignore
 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* ((backend (vc-responsible-backend default-directory))
+          (rel-dir
+           (condition-case nil
+               (file-name-directory
+                (vc-call-backend backend 'find-ignore-file
+                                 default-directory))
+             (vc-not-supported
+              default-directory)))
+          (file (read-file-name "File to ignore: ")))
+     (when (and (file-name-absolute-p file)
+                (file-in-directory-p file rel-dir))
+       (setq file (file-relative-name file rel-dir)))
+     (list file
+           rel-dir
+           current-prefix-arg)))
   (let* ((directory (or directory default-directory))
 	 (backend (or (vc-responsible-backend default-directory)
                       (error "Unknown backend"))))
@@ -1423,23 +1428,18 @@ vc-ignore
 
 (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.
+FILE is a wildcard specification relative to 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)))
-	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)))
+         (vc-call-backend backend
+                          'find-ignore-file
+                          (or directory default-directory))))
     (if remove
-	(vc--remove-regexp (concat "^" (regexp-quote pattern ) "\\(\n\\|$\\)") ignore)
-      (vc--add-line pattern ignore))))
+        (vc--remove-regexp (concat "^" (regexp-quote file) "\\(\n\\|$\\)") ignore)
+      (vc--add-line file ignore))))
 
 (defun vc-default-ignore-completion-table (backend file)
   "Return the list of ignored files under BACKEND."

^ permalink raw reply related	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-21  0:05                                                               ` Dmitry Gutov
@ 2020-02-21  8:10                                                                 ` Eli Zaretskii
  2020-02-21 22:22                                                                 ` Wolfgang Scherer
  1 sibling, 0 replies; 66+ messages in thread
From: Eli Zaretskii @ 2020-02-21  8:10 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 37189, Wolfgang.Scherer

> Cc: 37189@debbugs.gnu.org, Wolfgang Scherer <Wolfgang.Scherer@gmx.de>
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Fri, 21 Feb 2020 02:05:50 +0200
> 
> Attaching a patch. Eli, Wolfgang, any objections?

I only skimmed over it, and certainly didn't try it, but it looks OK
to me, thanks.





^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-21  0:05                                                               ` Dmitry Gutov
  2020-02-21  8:10                                                                 ` Eli Zaretskii
@ 2020-02-21 22:22                                                                 ` Wolfgang Scherer
  2020-02-22  7:44                                                                   ` Eli Zaretskii
  2020-02-22 19:30                                                                   ` Dmitry Gutov
  1 sibling, 2 replies; 66+ messages in thread
From: Wolfgang Scherer @ 2020-02-21 22:22 UTC (permalink / raw)
  To: Dmitry Gutov, Eli Zaretskii; +Cc: 37189


Am 21.02.20 um 01:05 schrieb Dmitry Gutov:
> On 14.02.2020 11:23, Eli Zaretskii wrote:
>>> Cc: 37189@debbugs.gnu.org
>>> From: Dmitry Gutov <dgutov@yandex.ru>
>>> Date: Fri, 14 Feb 2020 01:40:31 +0200
>>>
>>> I think the first thing I'll have to do is revert a part of an earlier
>>> patch to vc-default-ignore which changed its semantics a little, because
>>> it doesn't look like this discussion is going to culminate in a patch
>>> small enough for emacs-27 anyway.
>>
>> Which part?  Can you show a proposed patch?
>
> See https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37217.
>
> The patch fixed a legitimate scenario, but made an existing one work worse. Trying to vc-ignore, say, '*.c' from a subdirectory in a Git repo, for instance, will now prepend the intermediary directories to it.
>
> (Mentioned this before: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37189#41)
>
> Now, one could argue that one use case is more important than the other one, and that vc-ignore has for a while been geared toward entering plain file names rather than glob patterns. I have little opinion on this subject, however, since I mostly edit ignore files by hand, and do so very rarely. So as far as I'm concerned, we could remove this feature altogether and not lose much.
Both use cases are important for casual users of a VC. Before this
research, I did not know anything about SVN, SRC, Bzr, Mtn. Ignore file
support in 'vc' as a distilled experience cache does come in handy,
even for experienced users. Especially, if escaping/anchoring is
implemented.
>
> But maybe it's more valuable for SVN users? Where the ignore configuration is more tricky and the DIRECTORY argument is actually important.
For SVN users it is extremely useful, since there are no files that
can be edited. Commands must be issued to store ignore
patterns. However, the commands cannot add patterns to existing
patterns, so the existing patterns have to be read and a new pattern
added, before writing everything back. That is pretty much work and
support by emacs as frontend is very valued.
>
> So the options at hand are:
>
> - Revert almost all of the patch from bug#37217, reverting to the previous, admittedly broken behavior, and continue to discuss a better improvement for Emacs 28.
I think that is the best way to proceed. The correct implementation is
definitely non-trivial and should not be made in a series of adhoc
patches.
> - Try to resolve the ambiguity of purpose in favor either entering patterns on file names only. Probably the latter, later vc-ignore to vc-ignore-file.
I have never seen the pattern feature before, but since I implemented
it now, I have come to realize that it is very useful, when combined
with pre-escaped and anchored patterns from the current file in
vc-dir-mode and dired-mode. It is extremely useful for regex pattern
syntax, where patterns are a lot more ambiguous and need more
attention than patterns with glob syntax.
> - Try to sit on both chairs... Basically, that means using the user input unaltered. Allowing them to enter a file name as well, but treat it as a pattern, without escaping or the like. This would be close to the original intent behind vc-ignore, AFAICT.
Pre-escaped and anchored is very useful. But unmodified ist a good
starting point.
> To do the last one, read-file-name would need to be called with the second argument provided, the directory against which the file path should be relative. For most backend, we can reuse the find-ignore-file backend command, but SVN (and RCS, etc) don't have it defined.
RCS, SCCS do not have ignore files, so they should error out. SRC has
ignore files similar to CVS and SVN. Do not forget that CVS and SVN
have functions vc-cvs-ignore and vc-svn-ignore, which are called
*instead* of vc-default-ignore. I.e. The changes to vc-default-ignore
do not affect those VCs. For the invocation of find-ignore-file in
vc-ignore, see below.
> Roughly and handwavy, we can take this case to mean "use default-directory".
Unfortunately not. If the file or pattern to be ignored is in a
subdirectory of default-directory, the DIRECTORY argument must reflect
this for CVS, SVN, SRC.
>
> As you noted, the use of read-file-name at all in vc-ignore is somewhat problematic, but let's see if we can keep the function sane without removing it first.
I would not change it for Emacs 27.
>
> Attaching a patch. Eli, Wolfgang, any objections?

As mentioned above, for CVS, SVN, SRC default-directory is not
necessarily the correct rel-dir. This would be better:

   (let* ((backend (vc-responsible-backend default-directory))
          (file (read-file-name "File to ignore: "))
          (rel-dir
           (condition-case nil
               (file-name-directory
                (vc-call-backend backend 'find-ignore-file
                                 default-directory))
             (vc-not-supported
              (file-name-directory
               (directory-file-name (expand-file-name file)))))))
     (when (and (file-name-absolute-p file)
                (file-in-directory-p file rel-dir))
       (setq file (file-relative-name file rel-dir)))
     (list file
           rel-dir
           current-prefix-arg)))

I apologize for introducing ewoc into vc-dir-mode. Meanwhile I have
discovered vc-deduce-fileset, which does all the hard work for
preparing a fileset in vc-dir-mode, dired-mode and other
modes. vc-deduce-fileset delivers a backend and absolute file names,
not relative file names. I think introducing relative file names in
function APIs is the wrong way to go. However, as a preliminary
solution, it does suffice.
>>> Then, naturally, we'll have to look for small changes that improve the
>>> situation but provide as little breakage as possible.
>>
>> Agreed.

Definitely.







^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-21 22:22                                                                 ` Wolfgang Scherer
@ 2020-02-22  7:44                                                                   ` Eli Zaretskii
  2020-02-22 13:46                                                                     ` Wolfgang Scherer
  2020-02-22 19:30                                                                   ` Dmitry Gutov
  1 sibling, 1 reply; 66+ messages in thread
From: Eli Zaretskii @ 2020-02-22  7:44 UTC (permalink / raw)
  To: Wolfgang Scherer; +Cc: 37189, dgutov

> Cc: 37189@debbugs.gnu.org
> From: Wolfgang Scherer <Wolfgang.Scherer@gmx.de>
> Date: Fri, 21 Feb 2020 23:22:18 +0100
> 
> > Now, one could argue that one use case is more important than the other one, and that vc-ignore has for a while been geared toward entering plain file names rather than glob patterns. I have little opinion on this subject, however, since I mostly edit ignore files by hand, and do so very rarely. So as far as I'm concerned, we could remove this feature altogether and not lose much.
> 
> Both use cases are important for casual users of a VC.

I think the issue is not such general, but a more specific one: is the
use case of ignoring patterns more important than ignoring particular
files, when we are talking about usage through VC?

> SRC has ignore files similar to CVS and SVN.

That's not my reading of the SRC source, which simply does

                    if line.startswith("#") or not line.strip():
                        continue
                    elif line.startswith("!"):
                        ignorable -= set(glob.glob(line[1:].strip()))
                    else:
                        ignorable |= set(glob.glob(line.strip()))

and the Python documentation, which says:

   glob.glob(pathname, *, recursive=False)

      Return a possibly-empty list of path names that match pathname,
      which must be a string containing a path specification. pathname
      can be either absolute (like /usr/src/Python-1.5/Makefile) or
      relative (like ../../Tools/*/*.gif), and can contain shell-style
      wildcards. Broken symlinks are included in the results (as in
      the shell). Whether or not the results are sorted depends on the
      file system.

So Git-style root-directory-only .srcignore files will do for SRC.
Which doesn't surprise me at all, because SRC in general copycats
Git's behavior in many aspects.

> > Roughly and handwavy, we can take this case to mean "use default-directory".
> Unfortunately not. If the file or pattern to be ignored is in a
> subdirectory of default-directory, the DIRECTORY argument must reflect
> this for CVS, SVN, SRC.

But since CVS and SVN don't use vc-default-ignore, and SRC can do with
a single file in the root of the repository, does it really matter in
practice?





^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-22  7:44                                                                   ` Eli Zaretskii
@ 2020-02-22 13:46                                                                     ` Wolfgang Scherer
  2020-02-22 14:30                                                                       ` Eli Zaretskii
  0 siblings, 1 reply; 66+ messages in thread
From: Wolfgang Scherer @ 2020-02-22 13:46 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 37189, dgutov


Am 22.02.20 um 08:44 schrieb Eli Zaretskii:
>> Cc: 37189@debbugs.gnu.org
>> From: Wolfgang Scherer <Wolfgang.Scherer@gmx.de>
>> Date: Fri, 21 Feb 2020 23:22:18 +0100
>>
>>> Now, one could argue that one use case is more important than the other one, and that vc-ignore has for a while been geared toward entering plain file names rather than glob patterns. I have little opinion on this subject, however, since I mostly edit ignore files by hand, and do so very rarely. So as far as I'm concerned, we could remove this feature altogether and not lose much.
>> Both use cases are important for casual users of a VC.
> I think the issue is not such general, but a more specific one: is the
> use case of ignoring patterns more important than ignoring particular
> files, when we are talking about usage through VC?
How would you measure the importance?

When files are properly escaped, it is possible to emulate patterns by
ignoring all matching files separately without knowing the exact
pattern syntax. This reduces the effort to marking the appropriate files
and pressing  C-u G  in vc-dir-mode.

The other way around is a lot harder, since it fails without a working
knowledge of ignore patterns in the context of a specific VCS. As you
demonstrate below for SRC.

My personal strategy with Mercurial has been to use the ignore file
feature (in DVC) to get a properly escaped file path, then edit
 .hgignore  to modify the full path to work as a pattern. So, yes I
can do very well without the pattern use case, but not really without
the file path use case.
>> SRC has ignore files similar to CVS and SVN.
> That's not my reading of the SRC source, which simply does
>
>                     if line.startswith("#") or not line.strip():
>                         continue
>                     elif line.startswith("!"):
>                         ignorable -= set(glob.glob(line[1:].strip()))
>                     else:
>                         ignorable |= set(glob.glob(line.strip()))
>
> and the Python documentation, which says:
>
>    glob.glob(pathname, *, recursive=False)
>
>       Return a possibly-empty list of path names that match pathname,
>       which must be a string containing a path specification. pathname
>       can be either absolute (like /usr/src/Python-1.5/Makefile) or
>       relative (like ../../Tools/*/*.gif), and can contain shell-style
>       wildcards. Broken symlinks are included in the results (as in
>       the shell). Whether or not the results are sorted depends on the
>       file system.
You are reading this correctly.
> So Git-style root-directory-only .srcignore files will do for SRC.
> Which doesn't surprise me at all, because SRC in general copycats
> Git's behavior in many aspects.
However, your conclusion is unfounded. You need a root directory and
recursion for Git-style glob patterns.

SRC is RCS/SCCS revisited with a modern frontend. It is - just like
RCS and SCCS - not recursive. None of its commands work recursively.

There is also no notion of a root directory, i.e. SRC **never** checks
a parent directory for ignore patterns, which would be necessary for a
Git-style glob to work.

Here is a quote from SRC's mission statement:

http://www.catb.org/~esr/src/FAQ.html:

    You are certainly free to suggest features, but SRC is developed
    with extreme conservatism as to what features to implement or
    not. Remember, single-file, single-user, private VCS.

This means implicitely, that there is no recursion into
sub-directories by design. The  vc  package just emulates recursive
operation for RCS, SCCS, SRC. This works fine for RCS and SCCS, since
RCS can be invoked on files in a subdirectory and works properly,
e.g.:

    rcs log sub/data

However, SRC currently fails in that scenario, actually destroying the
repository identity. Which also means, that operations on
sub-directories in  vc-dir-mode  fail destructively. I have not yet
reported this issue for Emacs, but to ESR at
https://gitlab.com/esr/src/issues/14, to see, whether SRC can be
repaired.

If SRC is not repaired, it would probably be best to disable recursive
operations in  vc , since the patch to repair  vc-src.el  is a bit
ugly and the result is not even close to perfect
(https://gitlab.com/esr/src/uploads/ca532d65dbb07afcb2144afbded1b368/0001-Run-SRC-commands-separately-and-normalized-for-each-.patch).
>>> Roughly and handwavy, we can take this case to mean "use default-directory".
>> Unfortunately not. If the file or pattern to be ignored is in a
>> subdirectory of default-directory, the DIRECTORY argument must reflect
>> this for CVS, SVN, SRC.
> But since CVS and SVN don't use vc-default-ignore, and SRC can do with
> a single file in the root of the repository, does it really matter in
> practice?

IMHO, the goal should be to eliminate both vc-svn-ignore and
vc-cvs-ignore, replacing the functionality by low-level backend
functions, which is perfectly possible, iff the ignore file / ignore
directory is correctly identified: vc-cvs-find-ignore-file,
vc-svn-ignore-file (just to identify the directory), vc-svn-add-line,
vc-svn-remove-line (or a combined handler for addition and
removal). This also results in cheap ignore file support for SRC with
a single function vc-src-find-ignore-file.

It also means, that escaping/anchoring does not have to be integrated
in 3 or 4 places but only once in vc-default-ignore or vc-ignore.






^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-22 13:46                                                                     ` Wolfgang Scherer
@ 2020-02-22 14:30                                                                       ` Eli Zaretskii
  2020-02-22 19:14                                                                         ` Dmitry Gutov
  2020-02-22 23:32                                                                         ` Wolfgang Scherer
  0 siblings, 2 replies; 66+ messages in thread
From: Eli Zaretskii @ 2020-02-22 14:30 UTC (permalink / raw)
  To: Wolfgang Scherer; +Cc: 37189, dgutov

> Cc: dgutov@yandex.ru, 37189@debbugs.gnu.org
> From: Wolfgang Scherer <Wolfgang.Scherer@gmx.de>
> Date: Sat, 22 Feb 2020 14:46:16 +0100
> 
> >> Both use cases are important for casual users of a VC.
> > I think the issue is not such general, but a more specific one: is the
> > use case of ignoring patterns more important than ignoring particular
> > files, when we are talking about usage through VC?
> How would you measure the importance?

By citing experience and anecdotal evidence, I guess.  Also, by
providing arguments for relative (un)importance of each use case.

> >> SRC has ignore files similar to CVS and SVN.
> > That's not my reading of the SRC source, which simply does
> >
> >                     if line.startswith("#") or not line.strip():
> >                         continue
> >                     elif line.startswith("!"):
> >                         ignorable -= set(glob.glob(line[1:].strip()))
> >                     else:
> >                         ignorable |= set(glob.glob(line.strip()))
> >
> > and the Python documentation, which says:
> >
> >    glob.glob(pathname, *, recursive=False)
> >
> >       Return a possibly-empty list of path names that match pathname,
> >       which must be a string containing a path specification. pathname
> >       can be either absolute (like /usr/src/Python-1.5/Makefile) or
> >       relative (like ../../Tools/*/*.gif), and can contain shell-style
> >       wildcards. Broken symlinks are included in the results (as in
> >       the shell). Whether or not the results are sorted depends on the
> >       file system.
> You are reading this correctly.
> > So Git-style root-directory-only .srcignore files will do for SRC.
> > Which doesn't surprise me at all, because SRC in general copycats
> > Git's behavior in many aspects.
> However, your conclusion is unfounded. You need a root directory and
> recursion for Git-style glob patterns.
> 
> SRC is RCS/SCCS revisited with a modern frontend. It is - just like
> RCS and SCCS - not recursive. None of its commands work recursively.

When I actually try this, I see something that confirms my
understanding:

  ~$ mkdir src_vcs
  ~$ cd src_vcs
  ~/src_vcs$ mkdir .src
  ~/src_vcs$ touch file1
  ~/src_vcs$ mkdir t1
  ~/src_vcs$ touch t1/file1
  ~/src_vcs$ src status t1/file1
  ?       t1/file1
  ~/src_vcs$ cat > .srcignore
  t1/file1
  ^D
  ~/src_vcs$ src status t1/file1
  I       t1/file1
  ~/src_vcs$ src status -a
  ?       .srcignore
  ?       file1
  ?       t1

> There is also no notion of a root directory, i.e. SRC **never** checks
> a parent directory for ignore patterns, which would be necessary for a
> Git-style glob to work.

The first part is true, but if we invoke "src status" from the root
directory, the .srcignore file there will be read, and as the example
above shows, will have its effect.  Right?

> >>> Roughly and handwavy, we can take this case to mean "use default-directory".
> >> Unfortunately not. If the file or pattern to be ignored is in a
> >> subdirectory of default-directory, the DIRECTORY argument must reflect
> >> this for CVS, SVN, SRC.
> > But since CVS and SVN don't use vc-default-ignore, and SRC can do with
> > a single file in the root of the repository, does it really matter in
> > practice?
> 
> IMHO, the goal should be to eliminate both vc-svn-ignore and
> vc-cvs-ignore, replacing the functionality by low-level backend
> functions, which is perfectly possible, iff the ignore file / ignore
> directory is correctly identified: vc-cvs-find-ignore-file,
> vc-svn-ignore-file (just to identify the directory), vc-svn-add-line,
> vc-svn-remove-line (or a combined handler for addition and
> removal). This also results in cheap ignore file support for SRC with
> a single function vc-src-find-ignore-file.

I don't see how this answers my question, sorry.  Even if our
long-term goal is to remove vc-svn/cvs-ignore-file (which are already
backend functions, of course), my question is still valid and
interesting under the current situation, to which Dmitry's suggestion
above pertained, AFAIU.  So we might consider his suggestion "good
enough" under the current situation.





^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-22 14:30                                                                       ` Eli Zaretskii
@ 2020-02-22 19:14                                                                         ` Dmitry Gutov
  2020-02-22 22:04                                                                           ` Wolfgang Scherer
  2020-02-22 23:32                                                                         ` Wolfgang Scherer
  1 sibling, 1 reply; 66+ messages in thread
From: Dmitry Gutov @ 2020-02-22 19:14 UTC (permalink / raw)
  To: Eli Zaretskii, Wolfgang Scherer; +Cc: 37189

On 22.02.2020 16:30, Eli Zaretskii wrote:
>> SRC is RCS/SCCS revisited with a modern frontend. It is - just like
>> RCS and SCCS - not recursive. None of its commands work recursively.
> When I actually try this, I see something that confirms my
> understanding:

The SRC discussion is interesting, but since vc-src doesn't implement 
the 'ignore backend command, nor the 'find-ignore-file backend command, 
vc-ignore isn't going to work with it in Emacs 27.

So this VCS's particulars should have little bearing on the current 
discussion.





^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-21 22:22                                                                 ` Wolfgang Scherer
  2020-02-22  7:44                                                                   ` Eli Zaretskii
@ 2020-02-22 19:30                                                                   ` Dmitry Gutov
  2020-02-22 22:00                                                                     ` Wolfgang Scherer
  1 sibling, 1 reply; 66+ messages in thread
From: Dmitry Gutov @ 2020-02-22 19:30 UTC (permalink / raw)
  To: Wolfgang Scherer, Eli Zaretskii; +Cc: 37189

On 22.02.2020 0:22, Wolfgang Scherer wrote:
> Both use cases are important for casual users of a VC. Before this
> research, I did not know anything about SVN, SRC, Bzr, Mtn. Ignore file
> support in 'vc' as a distilled experience cache does come in handy,
> even for experienced users. Especially, if escaping/anchoring is
> implemented.

Escaping/anchoring can be added in the next step of the "series of 
ad-hoc patches".

> I think that is the best way to proceed. The correct implementation is
> definitely non-trivial and should not be made in a series of adhoc
> patches.

Let's try to discuss this one first anyway.

> RCS, SCCS do not have ignore files, so they should error out. SRC has
> ignore files similar to CVS and SVN.

They will fail anyway because they implement neither 'find-ignore-file' 
nor the 'ignore' backend actions.

> Do not forget that CVS and SVN
> have functions vc-cvs-ignore and vc-svn-ignore, which are called
> *instead*  of vc-default-ignore. I.e. The changes to vc-default-ignore
> do not affect those VCs. For the invocation of find-ignore-file in
> vc-ignore, see below.

I think the patch is compatible with those. At least with the SVN one 
(vc-cvs-ignore might need a little work).

Have you tried this patch with SVN? Is there a particular scenario where 
it fails?

>> Roughly and handwavy, we can take this case to mean "use default-directory".
> Unfortunately not. If the file or pattern to be ignored is in a
> subdirectory of default-directory, the DIRECTORY argument must reflect
> this for CVS, SVN, SRC.

vc-svn-ignore looks like it can handle either calling convention.

> I apologize for introducing ewoc into vc-dir-mode. Meanwhile I have
> discovered vc-deduce-fileset, which does all the hard work for
> preparing a fileset in vc-dir-mode, dired-mode and other
> modes. vc-deduce-fileset delivers a backend and absolute file names,
> not relative file names.

Please submit a patch fixing that whenever you have the time.

> I think introducing relative file names in
> function APIs is the wrong way to go. However, as a preliminary
> solution, it does suffice.

I think it's just fine if the function is supposed to receive an *ignore 
pattern*, and not a simple file name.





^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-22 19:30                                                                   ` Dmitry Gutov
@ 2020-02-22 22:00                                                                     ` Wolfgang Scherer
  2020-02-22 23:58                                                                       ` Dmitry Gutov
  0 siblings, 1 reply; 66+ messages in thread
From: Wolfgang Scherer @ 2020-02-22 22:00 UTC (permalink / raw)
  To: Dmitry Gutov, Eli Zaretskii; +Cc: 37189


Am 22.02.20 um 20:30 schrieb Dmitry Gutov:
> On 22.02.2020 0:22, Wolfgang Scherer wrote:
>> Both use cases are important for casual users of a VC. Before this
>> research, I did not know anything about SVN, SRC, Bzr, Mtn. Ignore file
>> support in 'vc' as a distilled experience cache does come in handy,
>> even for experienced users. Especially, if escaping/anchoring is
>> implemented.
>
> Escaping/anchoring can be added in the next step of the "series of ad-hoc patches".
I'm not saying to implement it now, I am just describing use cases, which are not limited to Emacs 27.
>> RCS, SCCS do not have ignore files, so they should error out. SRC has
>> ignore files similar to CVS and SVN.
>
> They will fail anyway because they implement neither 'find-ignore-file' nor the 'ignore' backend actions.
This is in reference to the condition-case around find-ignore-file, which is not necessary. It is OK to error out in that case.
>
>> Do not forget that CVS and SVN
>> have functions vc-cvs-ignore and vc-svn-ignore, which are called
>> *instead*  of vc-default-ignore. I.e. The changes to vc-default-ignore
>> do not affect those VCs. For the invocation of find-ignore-file in
>> vc-ignore, see below.
>
> I think the patch is compatible with those. At least with the SVN one (vc-cvs-ignore might need a little work).
>
> Have you tried this patch with SVN? Is there a particular scenario where it fails?
No, it should not. CVS is now also patched, so it will no longer fail.
>
>>> Roughly and handwavy, we can take this case to mean "use default-directory".
>> Unfortunately not. If the file or pattern to be ignored is in a
>> subdirectory of default-directory, the DIRECTORY argument must reflect
>> this for CVS, SVN, SRC.
>
> vc-svn-ignore looks like it can handle either calling convention.
As does vc-cvs-ignore. So, for right now, it is OK.
>
>> I apologize for introducing ewoc into vc-dir-mode. Meanwhile I have
>> discovered vc-deduce-fileset, which does all the hard work for
>> preparing a fileset in vc-dir-mode, dired-mode and other
>> modes. vc-deduce-fileset delivers a backend and absolute file names,
>> not relative file names.
>
> Please submit a patch fixing that whenever you have the time.
I don't think it should be done before Emacs 28.
>
>> I think introducing relative file names in
>> function APIs is the wrong way to go. However, as a preliminary
>> solution, it does suffice.
>
> I think it's just fine if the function is supposed to receive an *ignore pattern*, and not a simple file name.

Again, it is probably not the time right now to discuss these matters.

What will be the time frame for starting Emacs 28 topics?






^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-22 19:14                                                                         ` Dmitry Gutov
@ 2020-02-22 22:04                                                                           ` Wolfgang Scherer
  0 siblings, 0 replies; 66+ messages in thread
From: Wolfgang Scherer @ 2020-02-22 22:04 UTC (permalink / raw)
  To: Dmitry Gutov, Eli Zaretskii; +Cc: 37189


Am 22.02.20 um 20:14 schrieb Dmitry Gutov:
> On 22.02.2020 16:30, Eli Zaretskii wrote:
>>> SRC is RCS/SCCS revisited with a modern frontend. It is - just like
>>> RCS and SCCS - not recursive. None of its commands work recursively.
>> When I actually try this, I see something that confirms my
>> understanding:
>
> The SRC discussion is interesting, but since vc-src doesn't implement the 'ignore backend command, nor the 'find-ignore-file backend command, vc-ignore isn't going to work with it in Emacs 27.
>
> So this VCS's particulars should have little bearing on the current discussion.

They don't. As far as I am concerned, that discussion about changes for Emacs 27 is actually finished. Your proposed patch is sufficient for the status quo, CVS is patched, so there is nothing left to do.

I will just wait for the Emacs 28 discussion to begin.







^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-22 14:30                                                                       ` Eli Zaretskii
  2020-02-22 19:14                                                                         ` Dmitry Gutov
@ 2020-02-22 23:32                                                                         ` Wolfgang Scherer
  2020-02-23 15:20                                                                           ` Eli Zaretskii
  1 sibling, 1 reply; 66+ messages in thread
From: Wolfgang Scherer @ 2020-02-22 23:32 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 37189, dgutov


Am 22.02.20 um 15:30 schrieb Eli Zaretskii:
>> Cc: dgutov@yandex.ru, 37189@debbugs.gnu.org
>> From: Wolfgang Scherer <Wolfgang.Scherer@gmx.de>
>> Date: Sat, 22 Feb 2020 14:46:16 +0100
> When I actually try this, I see something that confirms my
> understanding:
>
>   ~$ mkdir src_vcs
>   ~$ cd src_vcs
>   ~/src_vcs$ mkdir .src
>   ~/src_vcs$ touch file1
>   ~/src_vcs$ mkdir t1
>   ~/src_vcs$ touch t1/file1
>   ~/src_vcs$ src status t1/file1
>   ?       t1/file1
>   ~/src_vcs$ cat > .srcignore
>   t1/file1
>   ^D
>   ~/src_vcs$ src status t1/file1
>   I       t1/file1
>   ~/src_vcs$ src status -a
>   ?       .srcignore
>   ?       file1
>   ?       t1

This is to be expected from the way glob(3) works and the way SRC is programmed.
How do you jump to the conclusion, that this particular case is an indication that SRC
works like Git, when most other evidence points to the opposite?

What about the unwillingness of src status -a to recurse into
sub-directories, how does that correspond to Git, which can
often not be stopped to recurse?

  ~/src_vcs$ src status -a t1
  ?    t1
  ~/src_vcs$ src status -a t1/
  ?    t1/
  ~/src_vcs$ src status -a t1/.
  ?    t1/.
  ~/src_vcs$ src status -a t1/*
  I    t1/file1

Also, changing into t1 does not ignore file1 any more,
which is obviously inconsistent:

  ~/src_vcs$ cd t1
  ~/src_vcs/t1$ mkdir -p .src
  ~/src_vcs/t1$ src status -a
  ?    file1

Further, if there was a similar pattern propagation as for Git,
file1 should be ignored in both directories in this case:

  ~/src_vcs$ cat >.srcignore
  file1
  ^D
  ~/src_vcs$ src status -a
  ?    .srcignore
  I    file1
  ?    t1
  ~/src_vcs$ src status t1/file1
  ?    t1/file1

And it should not matter, where in the sub-tree the status command is issued:

  ~/src_vcs$ cd t1
  /src_vcs/t1$ src status file1
  ?    file1
  ~/src_vcs/t1$ src status ../file1
  ?    ../file1

>> There is also no notion of a root directory, i.e. SRC **never** checks
>> a parent directory for ignore patterns, which would be necessary for a
>> Git-style glob to work.
> The first part is true, but if we invoke "src status" from the root
> directory, the .srcignore file there will be read, and as the example
> above shows, will have its effect.  Right?

No, if it is not consistent in all parts of a sub-tree, the
behavior is an anomaly, not an indication of intent. So actually you
have found a bug, which you should report.






^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-22 22:00                                                                     ` Wolfgang Scherer
@ 2020-02-22 23:58                                                                       ` Dmitry Gutov
  2020-02-23  0:29                                                                         ` Wolfgang Scherer
  0 siblings, 1 reply; 66+ messages in thread
From: Dmitry Gutov @ 2020-02-22 23:58 UTC (permalink / raw)
  To: Wolfgang Scherer, Eli Zaretskii; +Cc: 37189

On 23.02.2020 0:00, Wolfgang Scherer wrote:

>> Escaping/anchoring can be added in the next step of the "series of ad-hoc patches".
> I'm not saying to implement it now, I am just describing use cases, which are not limited to Emacs 27.

Sure.

>>> RCS, SCCS do not have ignore files, so they should error out. SRC has
>>> ignore files similar to CVS and SVN.
>>
>> They will fail anyway because they implement neither 'find-ignore-file' nor the 'ignore' backend actions.
> This is in reference to the condition-case around find-ignore-file, which is not necessary. It is OK to error out in that case.

Without it, SVN and CVS would fail as well (since they don't implement 
'find-ignore-file').

>> I think the patch is compatible with those. At least with the SVN one (vc-cvs-ignore might need a little work).
>>
>> Have you tried this patch with SVN? Is there a particular scenario where it fails?
> No, it should not. CVS is now also patched, so it will no longer fail.

Cool. Thank you. I have now pushed the patch to emacs-27.

Please let us know if you see any unexpected problems.

>>> I apologize for introducing ewoc into vc-dir-mode. Meanwhile I have
>>> discovered vc-deduce-fileset, which does all the hard work for
>>> preparing a fileset in vc-dir-mode, dired-mode and other
>>> modes. vc-deduce-fileset delivers a backend and absolute file names,
>>> not relative file names.
>>
>> Please submit a patch fixing that whenever you have the time.
> I don't think it should be done before Emacs 28.

It's fine to submit bug reports and patches now already. We can even 
install them on master.

>>> I think introducing relative file names in
>>> function APIs is the wrong way to go. However, as a preliminary
>>> solution, it does suffice.
>>
>> I think it's just fine if the function is supposed to receive an *ignore pattern*, and not a simple file name.
> 
> Again, it is probably not the time right now to discuss these matters.
> 
> What will be the time frame for starting Emacs 28 topics?

Since the discussion of vc-ignore in Emacs 27 seems finished, we can go 
on ahead with that right now.





^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-22 23:58                                                                       ` Dmitry Gutov
@ 2020-02-23  0:29                                                                         ` Wolfgang Scherer
  2020-02-24 23:07                                                                           ` Dmitry Gutov
  0 siblings, 1 reply; 66+ messages in thread
From: Wolfgang Scherer @ 2020-02-23  0:29 UTC (permalink / raw)
  To: Dmitry Gutov, Eli Zaretskii; +Cc: 37189


Am 23.02.20 um 00:58 schrieb Dmitry Gutov:
> On 23.02.2020 0:00, Wolfgang Scherer wrote:
>
>>>> RCS, SCCS do not have ignore files, so they should error out. SRC has
>>>> ignore files similar to CVS and SVN.
>>>
>>> They will fail anyway because they implement neither 'find-ignore-file' nor the 'ignore' backend actions.
>> This is in reference to the condition-case around find-ignore-file, which is not necessary. It is OK to error out in that case.
>
> Without it, SVN and CVS would fail as well (since they don't implement 'find-ignore-file').
Sorry, not a discussion point for Emacs 27. Let's pick it up later.
>
>>> I think the patch is compatible with those. At least with the SVN one (vc-cvs-ignore might need a little work).
>>>
>>> Have you tried this patch with SVN? Is there a particular scenario where it fails?
>> No, it should not. CVS is now also patched, so it will no longer fail.
>
> Cool. Thank you. I have now pushed the patch to emacs-27.
>
> Please let us know if you see any unexpected problems.
OK, here is one: Since vc-default-ignore no longer normalizes the
relative file names, vc-dir-ignore should do the entire processing,
just as vc-ignore in interactive mode does.

When calling vc-dir with a sub-directory of the repository root, the
relative file names against default-directory are not equivalent to
the relative file names against the root. I.e., the file names written
to the root ignore file are wrong.
>> What will be the time frame for starting Emacs 28 topics?
>
> Since the discussion of vc-ignore in Emacs 27 seems finished, we can go on ahead with that right now.
Good.





^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-22 23:32                                                                         ` Wolfgang Scherer
@ 2020-02-23 15:20                                                                           ` Eli Zaretskii
  2020-02-23 19:16                                                                             ` Wolfgang Scherer
  0 siblings, 1 reply; 66+ messages in thread
From: Eli Zaretskii @ 2020-02-23 15:20 UTC (permalink / raw)
  To: Wolfgang Scherer; +Cc: 37189, dgutov

> Cc: dgutov@yandex.ru, 37189@debbugs.gnu.org
> From: Wolfgang Scherer <Wolfgang.Scherer@gmx.de>
> Date: Sun, 23 Feb 2020 00:32:27 +0100
> 
> What about the unwillingness of src status -a to recurse into
> sub-directories, how does that correspond to Git, which can
> often not be stopped to recurse?

I was talking about this issue only from the POV of VC.  VC doesn't
have to recurse, it can always run with default-directory set to the
repository root, when SRC is the backend.





^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-23 15:20                                                                           ` Eli Zaretskii
@ 2020-02-23 19:16                                                                             ` Wolfgang Scherer
  0 siblings, 0 replies; 66+ messages in thread
From: Wolfgang Scherer @ 2020-02-23 19:16 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 37189, dgutov


Am 23.02.20 um 16:20 schrieb Eli Zaretskii:
>> Cc: dgutov@yandex.ru, 37189@debbugs.gnu.org
>> From: Wolfgang Scherer <Wolfgang.Scherer@gmx.de>
>> Date: Sun, 23 Feb 2020 00:32:27 +0100
>>
>> What about the unwillingness of src status -a to recurse into
>> sub-directories, how does that correspond to Git, which can
>> often not be stopped to recurse?
> I was talking about this issue only from the POV of VC.  VC doesn't
> have to recurse, it can always run with default-directory set to the
> repository root, when SRC is the backend.

And how is the repository root defined?

What would the function vc-src-root look like?

How would vc-dir in a sub-directory of root learn about the files ignored?
default-directory is not the root in this case.






^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-23  0:29                                                                         ` Wolfgang Scherer
@ 2020-02-24 23:07                                                                           ` Dmitry Gutov
  2020-02-25  2:22                                                                             ` Wolfgang Scherer
  0 siblings, 1 reply; 66+ messages in thread
From: Dmitry Gutov @ 2020-02-24 23:07 UTC (permalink / raw)
  To: Wolfgang Scherer, Eli Zaretskii; +Cc: 37189

On 23.02.2020 2:29, Wolfgang Scherer wrote:

>> Please let us know if you see any unexpected problems.
> OK, here is one: Since vc-default-ignore no longer normalizes the
> relative file names, vc-dir-ignore should do the entire processing,
> just as vc-ignore in interactive mode does.
> 
> When calling vc-dir with a sub-directory of the repository root, the
> relative file names against default-directory are not equivalent to
> the relative file names against the root. I.e., the file names written
> to the root ignore file are wrong.

Thanks, should be fixed now in commit 9ec6eb1065.

>>> What will be the time frame for starting Emacs 28 topics?
>>
>> Since the discussion of vc-ignore in Emacs 27 seems finished, we can go on ahead with that right now.
> Good.

Okay then. Would you like to update the patch for vc-hg-ignore that this 
bug report started from? I think it can be simplified a little, and 
otherwise it looks good to me (for Emacs 28, at least).

Or do you want to go straight for the API redesign?





^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-24 23:07                                                                           ` Dmitry Gutov
@ 2020-02-25  2:22                                                                             ` Wolfgang Scherer
  2020-03-19 23:42                                                                               ` Dmitry Gutov
  0 siblings, 1 reply; 66+ messages in thread
From: Wolfgang Scherer @ 2020-02-25  2:22 UTC (permalink / raw)
  To: Dmitry Gutov, Eli Zaretskii; +Cc: 37189


Am 25.02.20 um 00:07 schrieb Dmitry Gutov:
> On 23.02.2020 2:29, Wolfgang Scherer wrote:
>>>> What will be the time frame for starting Emacs 28 topics?
>>>
>>> Since the discussion of vc-ignore in Emacs 27 seems finished, we can go on ahead with that right now.
>> Good.
>
> Okay then. Would you like to update the patch for vc-hg-ignore that this bug report started from? I think it can be simplified a little, and otherwise it looks good to me (for Emacs 28, at least).
>
> Or do you want to go straight for the API redesign?
I would like to discuss overall strategy first.  Since I have a fully
working implementation for all supported backends at
https://github.com/wolfmanx/vc-ign that does not interfere with the
current vc commands, it would be helpful, if you could load it and
evaluate the use cases 'z i' and 'z p' in 'vc-dir-mode', 'C-x v z i'
and 'C-x v z p' in 'dired-mode' and in a file buffer, just to see if
we can agree on a direction before discussing details.

I think a good point to start is the elimination of all
backend-specific 'vc-ignore' implementations ('vc-cvs-ignore',
'vc-svn-ignore').  That is one goal which is easily achievable.  The
benefit is a uniform implementation across all backends with
'vc-default-ignore' as the central implementation of algorithms,
without duplication of code in backends.
j\x15\x04





^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-02-25  2:22                                                                             ` Wolfgang Scherer
@ 2020-03-19 23:42                                                                               ` Dmitry Gutov
  2020-07-03 20:53                                                                                 ` Wolfgang Scherer
  0 siblings, 1 reply; 66+ messages in thread
From: Dmitry Gutov @ 2020-03-19 23:42 UTC (permalink / raw)
  To: Wolfgang Scherer, Eli Zaretskii; +Cc: 37189

Hi Wolfgang,

On 25.02.2020 04:22, Wolfgang Scherer wrote:
> I would like to discuss overall strategy first.  Since I have a fully
> working implementation for all supported backends at
> https://github.com/wolfmanx/vc-ign  that does not interfere with the
> current vc commands, it would be helpful, if you could load it and
> evaluate the use cases 'z i' and 'z p' in 'vc-dir-mode', 'C-x v z i'
> and 'C-x v z p' in 'dired-mode' and in a file buffer, just to see if
> we can agree on a direction before discussing details.

I've read the patch you posted to this discussion, and there are some 
definitely good things in there, but I wonder if we can make some 
progress first without redoing everything quite as much.

The majority of backends don't support regexp ignores (AFAIK), so maybe 
it's not worth bringing that notion at the top of the API. If we can let 
individual backends handle this case in their implementations correctly, 
that would be better. Then the ignore-param-regexp and ignore-param 
generics might not be necessary. Though we could add some other(s).

> I think a good point to start is the elimination of all
> backend-specific 'vc-ignore' implementations ('vc-cvs-ignore',
> 'vc-svn-ignore').  That is one goal which is easily achievable.  The
> benefit is a uniform implementation across all backends with
> 'vc-default-ignore' as the central implementation of algorithms,
> without duplication of code in backends.

First: the name of 'vc-default-ignore' itself implies that there have to 
be backend-specific implementations. That's what the -default- in the 
name is for. Likewise for vc-default-get-ignore-file-and-pattern.

Some other thoughts:

* Why go to this much indirection with 'ignore-param' when we could have 
a backend method that would escape and anchor file name? It doesn't look 
like that option would take more code.

* Since when AS-IS is t vc-default-get-ignore-file-and-pattern is not 
doing much, maybe vc-ignore-file and vc-ignore-file shouldn't carry this 
distinction this long through the call stack.

If vc-ignore-file calls a backend method that turns the file name into a 
pattern which doesn't need any further special handling, it even could 
call vc-ignore-pattern with the resulting value.

* Shouldn't vc-ignore-fileset takes the fileset and loop through it 
calling vc-ignore-file with each value?





^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-03-19 23:42                                                                               ` Dmitry Gutov
@ 2020-07-03 20:53                                                                                 ` Wolfgang Scherer
  2020-07-03 21:49                                                                                   ` Dmitry Gutov
  0 siblings, 1 reply; 66+ messages in thread
From: Wolfgang Scherer @ 2020-07-03 20:53 UTC (permalink / raw)
  To: Dmitry Gutov, Eli Zaretskii; +Cc: 37189

Sorry Dmitry,

I did not see your mail until just yet. My window for doing other work has unfortunately closed until September/October 2020.

I am using the modified package on a daily basis and it works satisfactorily. I have not come across any errors, and I no longer have to use other methods than the vc package.

I will still try to get the MELPA package accepted -- since it covers older Emacsen -- and then come back to the topic for current Emacs development.

Cheers, Wolfgang






^ permalink raw reply	[flat|nested] 66+ messages in thread

* bug#37189: 25.4.1: vc-hg-ignore implementation is missing
  2020-07-03 20:53                                                                                 ` Wolfgang Scherer
@ 2020-07-03 21:49                                                                                   ` Dmitry Gutov
  0 siblings, 0 replies; 66+ messages in thread
From: Dmitry Gutov @ 2020-07-03 21:49 UTC (permalink / raw)
  To: Wolfgang Scherer, Eli Zaretskii; +Cc: 37189

Hi Wolfgang,

On 03.07.2020 23:53, Wolfgang Scherer wrote:
> I did not see your mail until just yet. My window for doing other work has unfortunately closed until September/October 2020.

Let's pick it up then, there's no big hurry.

Thanks,
Dmitry.





^ permalink raw reply	[flat|nested] 66+ messages in thread

end of thread, other threads:[~2020-07-03 21:49 UTC | newest]

Thread overview: 66+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-26  0:21 bug#37189: 25.4.1: vc-hg-ignore implementation is missing Wolfgang Scherer
     [not found] ` <handler.37189.B.15667808855126.ack@debbugs.gnu.org>
2019-08-26 23:25   ` bug#37189: Acknowledgement (25.4.1: vc-hg-ignore implementation is missing) Wolfgang Scherer
2019-08-27  7:45     ` Eli Zaretskii
2019-08-28  1:46       ` bug#37189: *** GMX Spamverdacht *** " Wolfgang Scherer
2019-08-28  6:16         ` Eli Zaretskii
2019-08-29  1:23           ` bug#37189: 25.4.1: vc-hg-ignore implementation is missing Wolfgang Scherer
2019-08-29  0:38         ` Wolfgang Scherer
2019-08-29 15:52           ` Wolfgang Scherer
2019-12-25  0:16             ` Dmitry Gutov
2020-01-05  3:46               ` Wolfgang Scherer
2020-01-05  8:58                 ` Andreas Schwab
2020-01-05 17:25                   ` Wolfgang Scherer
2020-01-14  1:14                 ` Dmitry Gutov
2020-02-01  1:20                   ` Wolfgang Scherer
2020-02-01  8:27                     ` Eli Zaretskii
2020-02-03  1:16                       ` Wolfgang Scherer
2020-02-04 18:55                         ` Eli Zaretskii
2020-02-05  5:18                           ` Wolfgang Scherer
2020-02-05 19:06                           ` Wolfgang Scherer
2020-02-07  9:57                             ` Eli Zaretskii
2020-02-08  9:57                               ` Dmitry Gutov
2020-02-08 19:45                                 ` Wolfgang Scherer
2020-02-08 20:05                                   ` Eli Zaretskii
2020-02-08 23:12                                     ` Wolfgang Scherer
2020-02-09 13:57                                       ` Wolfgang Scherer
2020-02-09 13:57                                       ` Wolfgang Scherer
2020-02-10 16:02                                         ` Eli Zaretskii
2020-02-11  1:45                                           ` Wolfgang Scherer
2020-02-11 17:32                                             ` Eli Zaretskii
2020-02-11 22:28                                               ` Wolfgang Scherer
2020-02-12 18:34                                                 ` Eli Zaretskii
     [not found]                                                   ` <6f3ba261-e1f9-cf19-cc22-ec8c24cf3298@gmx.de>
2020-02-12 23:20                                                     ` Wolfgang Scherer
2020-02-13  1:18                                                       ` Wolfgang Scherer
2020-02-13 15:09                                                         ` Eli Zaretskii
2020-02-13 16:30                                                           ` Wolfgang Scherer
2020-02-13 23:43                                                           ` Richard Stallman
2020-02-14  1:49                                                             ` Wolfgang Scherer
2020-02-16  2:29                                                               ` Richard Stallman
2020-02-13 15:21                                                         ` Eli Zaretskii
2020-02-13 23:40                                                           ` Dmitry Gutov
2020-02-14  9:23                                                             ` Eli Zaretskii
2020-02-21  0:05                                                               ` Dmitry Gutov
2020-02-21  8:10                                                                 ` Eli Zaretskii
2020-02-21 22:22                                                                 ` Wolfgang Scherer
2020-02-22  7:44                                                                   ` Eli Zaretskii
2020-02-22 13:46                                                                     ` Wolfgang Scherer
2020-02-22 14:30                                                                       ` Eli Zaretskii
2020-02-22 19:14                                                                         ` Dmitry Gutov
2020-02-22 22:04                                                                           ` Wolfgang Scherer
2020-02-22 23:32                                                                         ` Wolfgang Scherer
2020-02-23 15:20                                                                           ` Eli Zaretskii
2020-02-23 19:16                                                                             ` Wolfgang Scherer
2020-02-22 19:30                                                                   ` Dmitry Gutov
2020-02-22 22:00                                                                     ` Wolfgang Scherer
2020-02-22 23:58                                                                       ` Dmitry Gutov
2020-02-23  0:29                                                                         ` Wolfgang Scherer
2020-02-24 23:07                                                                           ` Dmitry Gutov
2020-02-25  2:22                                                                             ` Wolfgang Scherer
2020-03-19 23:42                                                                               ` Dmitry Gutov
2020-07-03 20:53                                                                                 ` Wolfgang Scherer
2020-07-03 21:49                                                                                   ` Dmitry Gutov
2020-02-12 17:23                                               ` Wolfgang Scherer
2020-02-09 13:57                                       ` Wolfgang Scherer
2020-02-09 14:07                                         ` Wolfgang Scherer
2020-02-08 23:59                                     ` Wolfgang Scherer
2020-02-09 21:06                               ` Wolfgang Scherer

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).