* 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[parent not found: <handler.37189.B.15667808855126.ack@debbugs.gnu.org>]
* 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 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-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-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 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-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-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 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
[parent not found: <6f3ba261-e1f9-cf19-cc22-ec8c24cf3298@gmx.de>]
* 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 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: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-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-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: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 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 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-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 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-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 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-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
* 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-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-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-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-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
![[-- Attachment #2: vc-ignore-sequence-diagram.png --] [-- Type: image/png, Size: 94935 bytes --]](../../b0e313e3-8fee-b0cb-51fb-a20af44acf4a@gmx.de/2-vc-ignore-sequence-diagram.png){kind=link}
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).