* bug#37215: [PATCH] vc-cvs-ignore writes absolute filenames and duplicate strings
@ 2019-08-28 22:32 Wolfgang Scherer
2019-09-15 13:12 ` Lars Ingebrigtsen
2020-02-28 16:07 ` Mattias Engdegård
0 siblings, 2 replies; 26+ messages in thread
From: Wolfgang Scherer @ 2019-08-28 22:32 UTC (permalink / raw)
To: 37215
[-- Attachment #1: Type: text/plain, Size: 237 bytes --]
When called with an absolute filename (which is the default
case), `vc-cvs-ignore' writes the entire path into the .cvsignore
file.
`vc-cvs-ignore' also writes duplicate strings into .cvsignore.
The attached patch fixes these errors.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Do-not-write-absolute-filenames-and-duplicate-string.patch --]
[-- Type: text/x-patch; name="0001-Do-not-write-absolute-filenames-and-duplicate-string.patch", Size: 2257 bytes --]
From 2b7b90b94a426754a99c965bf708bf5854008b76 Mon Sep 17 00:00:00 2001
From: Wolfgang Scherer <wolfgang.scherer@gmx.de>
Date: Thu, 29 Aug 2019 00:29:31 +0200
Subject: [PATCH] Do not write absolute filenames and duplicate strings into
.cvsignore
* lisp/vc/vc-cvs.el: (vc-cvs-ignore) Expand filename correctly
and pass on basename only.
(vc-cvs-append-to-ignore) Do not write duplicate strings to
.cvsignore.
---
lisp/vc/vc-cvs.el | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/lisp/vc/vc-cvs.el b/lisp/vc/vc-cvs.el
index d84700f..1d0387b 100644
--- a/lisp/vc/vc-cvs.el
+++ b/lisp/vc/vc-cvs.el
@@ -1220,9 +1220,11 @@ 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-ignore (file &optional directory _remove)
+ "Ignore FILE under CVS.
+FILE is either absolute or relative to DIRECTORY."
+ (setq file (directory-file-name (expand-file-name file directory)))
+ (vc-cvs-append-to-ignore (file-name-directory file) (file-name-nondirectory file)))
(defun vc-cvs-append-to-ignore (dir str &optional old-dir)
"In DIR, add STR to the .cvsignore file.
@@ -1236,13 +1238,16 @@ to hear about anymore."
(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)))
+ (goto-char (point-min))
+ (save-match-data
+ (unless (re-search-forward (concat "^" (regexp-quote str) "$") nil 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)))))
(provide 'vc-cvs)
--
2.7.4
^ permalink raw reply related [flat|nested] 26+ messages in thread
* bug#37215: [PATCH] vc-cvs-ignore writes absolute filenames and duplicate strings
2019-08-28 22:32 bug#37215: [PATCH] vc-cvs-ignore writes absolute filenames and duplicate strings Wolfgang Scherer
@ 2019-09-15 13:12 ` Lars Ingebrigtsen
2020-01-05 3:59 ` Wolfgang Scherer
2020-02-28 16:07 ` Mattias Engdegård
1 sibling, 1 reply; 26+ messages in thread
From: Lars Ingebrigtsen @ 2019-09-15 13:12 UTC (permalink / raw)
To: Wolfgang Scherer; +Cc: 37215
Wolfgang Scherer <Wolfgang.Scherer@gmx.de> writes:
> * lisp/vc/vc-cvs.el: (vc-cvs-ignore) Expand filename correctly
> and pass on basename only.
> (vc-cvs-append-to-ignore) Do not write duplicate strings to
> .cvsignore.
This looks correct to me, but:
[...]
> + ;; FIXME this is a pcvs variable.
> + (if (bound-and-true-p cvs-sort-ignore-file)
> + (sort-lines nil (point-min) (point-max)))
Does it make sense to heed a pcvs variable here? I think it would be
surprising that vc-cvs behaves differently depending on whether you've
loaded pcvs or not.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply [flat|nested] 26+ messages in thread
* bug#37215: [PATCH] vc-cvs-ignore writes absolute filenames and duplicate strings
2019-09-15 13:12 ` Lars Ingebrigtsen
@ 2020-01-05 3:59 ` Wolfgang Scherer
2020-01-22 12:43 ` Lars Ingebrigtsen
0 siblings, 1 reply; 26+ messages in thread
From: Wolfgang Scherer @ 2020-01-05 3:59 UTC (permalink / raw)
To: Lars Ingebrigtsen; +Cc: 37215
Am 15.09.19 um 15:12 schrieb Lars Ingebrigtsen:
> Wolfgang Scherer <Wolfgang.Scherer@gmx.de> writes:
>
>> + ;; FIXME this is a pcvs variable.
>> + (if (bound-and-true-p cvs-sort-ignore-file)
>> + (sort-lines nil (point-min) (point-max)))
> Does it make sense to heed a pcvs variable here? I think it would be
> surprising that vc-cvs behaves differently depending on whether you've
> loaded pcvs or not.
Just to clarifiy: my patch does not introduce this FIXME. Only the indentation is changed. This question should be handled in a separate bug report.
^ permalink raw reply [flat|nested] 26+ messages in thread
* bug#37215: [PATCH] vc-cvs-ignore writes absolute filenames and duplicate strings
2020-01-05 3:59 ` Wolfgang Scherer
@ 2020-01-22 12:43 ` Lars Ingebrigtsen
2020-01-30 19:44 ` Wolfgang Scherer
0 siblings, 1 reply; 26+ messages in thread
From: Lars Ingebrigtsen @ 2020-01-22 12:43 UTC (permalink / raw)
To: Wolfgang Scherer; +Cc: 37215
Wolfgang Scherer <Wolfgang.Scherer@gmx.de> writes:
> Am 15.09.19 um 15:12 schrieb Lars Ingebrigtsen:
>> Wolfgang Scherer <Wolfgang.Scherer@gmx.de> writes:
>>
>>> + ;; FIXME this is a pcvs variable.
>>> + (if (bound-and-true-p cvs-sort-ignore-file)
>>> + (sort-lines nil (point-min) (point-max)))
>> Does it make sense to heed a pcvs variable here? I think it would be
>> surprising that vc-cvs behaves differently depending on whether you've
>> loaded pcvs or not.
>
> Just to clarifiy: my patch does not introduce this FIXME. Only the
> indentation is changed. This question should be handled in a separate
> bug report.
Right; sorry. Looking at the patch again, I don't quite get the logic here:
> -(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-ignore (file &optional directory _remove)
> + "Ignore FILE under CVS.
> +FILE is either absolute or relative to DIRECTORY."
> + (setq file (directory-file-name (expand-file-name file directory)))
> + (vc-cvs-append-to-ignore (file-name-directory file) (file-name-nondirectory file)))
This is basically
(file-name-nondirectory (directory-file-name (expand-file-name "foo" directory)))
isn't it? In what circumstances does that evaluate to something other
than "foo"? That is, what DIRECTORY is doesn't seem to matter, if I'm
reading this right?
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply [flat|nested] 26+ messages in thread
* bug#37215: [PATCH] vc-cvs-ignore writes absolute filenames and duplicate strings
2020-01-22 12:43 ` Lars Ingebrigtsen
@ 2020-01-30 19:44 ` Wolfgang Scherer
2020-02-13 19:36 ` Eli Zaretskii
0 siblings, 1 reply; 26+ messages in thread
From: Wolfgang Scherer @ 2020-01-30 19:44 UTC (permalink / raw)
To: Lars Ingebrigtsen; +Cc: 37215
Am 22.01.20 um 13:43 schrieb Lars Ingebrigtsen:
> Wolfgang Scherer <Wolfgang.Scherer@gmx.de> writes:
>
>> Just to clarifiy: my patch does not introduce this FIXME. Only the
>> indentation is changed. This question should be handled in a separate
>> bug report.
> Right; sorry. Looking at the patch again, I don't quite get the logic here:
>
>> -(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-ignore (file &optional directory _remove)
>> + "Ignore FILE under CVS.
>> +FILE is either absolute or relative to DIRECTORY."
>> + (setq file (directory-file-name (expand-file-name file directory)))
>> + (vc-cvs-append-to-ignore (file-name-directory file) (file-name-nondirectory file)))
> This is basically
>
> (file-name-nondirectory (directory-file-name (expand-file-name "foo" directory)))
>
> isn't it?
It is for `file` equal to "foo" (a simple basename).
> In what circumstances does that evaluate to something other
> than "foo"?
If "foo" is something other than a simple basename (see below).
> That is, what DIRECTORY is doesn't seem to matter, if I'm
> reading this right?
Your assumption, that `file` is always a simple basename is wrong.
(defun xx-vc-cvs-ignore (file &optional directory _remove)
"Ignore FILE under CVS.
FILE is either absolute or relative to DIRECTORY."
(let* ((norm-path (directory-file-name (expand-file-name file directory)))
(norm-dir (file-name-directory norm-path))
(norm-file (file-name-nondirectory norm-path)))
(format "'%S\n'%S"
(list :equal: (if (equal norm-file file) "yes" "no ")
:file: norm-file :orig: file)
(list :equal: (if (equal norm-dir directory) "yes" "no ")
:dir_: norm-dir :orig: directory))))
Default directory is "/home/ws/tmp/", home directory is also "/home/ws/".
;; (insert (format "\n;; 1)\n%s" (xx-vc-cvs-ignore "x-vc-repair.el" nil)))
;; 1)
'(:equal: "yes" :file: "x-vc-repair.el" :orig: "x-vc-repair.el")
'(:equal: "no " :dir_: "/home/ws/tmp/" :orig: nil)
;; (insert (format "\n;; 2)\n%s" (xx-vc-cvs-ignore "x-vc-repair.el" "/root/emacs-init/.emacs.def/")))
;; 2)
'(:equal: "yes" :file: "x-vc-repair.el" :orig: "x-vc-repair.el")
'(:equal: "yes" :dir_: "/root/emacs-init/.emacs.def/" :orig: "/root/emacs-init/.emacs.def/")
;; (insert (format "\n;; 3)\n%s" (xx-vc-cvs-ignore "~/x-vc-repair.el" "/root/other-dir/")))
;; 3)
'(:equal: "no " :file: "x-vc-repair.el" :orig: "~/x-vc-repair.el")
'(:equal: "no " :dir_: "/home/ws/" :orig: "/root/other-dir/")
;; (insert (format "\n;; 4)\n%s" (xx-vc-cvs-ignore "/root/emacs-init/.emacs.def/x-vc-repair.el" "/root/other-dir/")))
;; 4)
'(:equal: "no " :file: "x-vc-repair.el" :orig: "/root/emacs-init/.emacs.def/x-vc-repair.el")
'(:equal: "no " :dir_: "/root/emacs-init/.emacs.def/" :orig: "/root/other-dir/")
;; (insert (format "\n;; 5)\n%s" (xx-vc-cvs-ignore ".emacs.def/x-vc-repair.el" "/root/emacs-init/")))
;; 5)
'(:equal: "no " :file: "x-vc-repair.el" :orig: ".emacs.def/x-vc-repair.el")
'(:equal: "no " :dir_: "/root/emacs-init/.emacs.def/" :orig: "/root/emacs-init/")
^ permalink raw reply [flat|nested] 26+ messages in thread
* bug#37215: [PATCH] vc-cvs-ignore writes absolute filenames and duplicate strings
2020-01-30 19:44 ` Wolfgang Scherer
@ 2020-02-13 19:36 ` Eli Zaretskii
2020-02-14 1:24 ` Wolfgang Scherer
2020-02-14 2:50 ` Wolfgang Scherer
0 siblings, 2 replies; 26+ messages in thread
From: Eli Zaretskii @ 2020-02-13 19:36 UTC (permalink / raw)
To: Wolfgang Scherer; +Cc: 37215, larsi
> From: Wolfgang Scherer <Wolfgang.Scherer@gmx.de>
> Date: Thu, 30 Jan 2020 20:44:00 +0100
> Cc: 37215@debbugs.gnu.org
>
> >> -(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-ignore (file &optional directory _remove)
> >> + "Ignore FILE under CVS.
> >> +FILE is either absolute or relative to DIRECTORY."
> >> + (setq file (directory-file-name (expand-file-name file directory)))
> >> + (vc-cvs-append-to-ignore (file-name-directory file) (file-name-nondirectory file)))
> > This is basically
> >
> > (file-name-nondirectory (directory-file-name (expand-file-name "foo" directory)))
> >
> > isn't it?
> It is for `file` equal to "foo" (a simple basename).
> > In what circumstances does that evaluate to something other
> > than "foo"?
> If "foo" is something other than a simple basename (see below).
> > That is, what DIRECTORY is doesn't seem to matter, if I'm
> > reading this right?
>
> Your assumption, that `file` is always a simple basename is wrong.
Yes, but when does it make sense to have FILE not absolute and not
just a basename (i.e. with leading directories)? Do we have such use
cases? Because if that happens, the file's name will be added to
.cvsignore not in DIRECTORY but in one of its subdirectories. Would
that be surprising? And if so, perhaps we should warn about that or
even error out?
^ permalink raw reply [flat|nested] 26+ messages in thread
* bug#37215: [PATCH] vc-cvs-ignore writes absolute filenames and duplicate strings
2020-02-13 19:36 ` Eli Zaretskii
@ 2020-02-14 1:24 ` Wolfgang Scherer
2020-02-14 8:33 ` Eli Zaretskii
2020-02-14 2:50 ` Wolfgang Scherer
1 sibling, 1 reply; 26+ messages in thread
From: Wolfgang Scherer @ 2020-02-14 1:24 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 37215, larsi
Am 13.02.20 um 20:36 schrieb Eli Zaretskii:
>> From: Wolfgang Scherer <Wolfgang.Scherer@gmx.de>
>> Date: Thu, 30 Jan 2020 20:44:00 +0100
>> Cc: 37215@debbugs.gnu.org
>>
>>>> -(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-ignore (file &optional directory _remove)
>>>> + "Ignore FILE under CVS.
>>>> +FILE is either absolute or relative to DIRECTORY."
>>>> + (setq file (directory-file-name (expand-file-name file directory)))
>>>> + (vc-cvs-append-to-ignore (file-name-directory file) (file-name-nondirectory file)))
>>> This is basically
>>>
>>> (file-name-nondirectory (directory-file-name (expand-file-name "foo" directory)))
>>>
>>> isn't it?
>> It is for `file` equal to "foo" (a simple basename).
>>> In what circumstances does that evaluate to something other
>>> than "foo"?
>> If "foo" is something other than a simple basename (see below).
>>> That is, what DIRECTORY is doesn't seem to matter, if I'm
>>> reading this right?
>> Your assumption, that `file` is always a simple basename is wrong.
> Yes, but when does it make sense to have FILE not absolute and not
> just a basename (i.e. with leading directories)? Do we have such use
> cases?
vc-dir-ignore with patch from #37240
> Because if that happens, the file's name will be added to
> .cvsignore not in DIRECTORY but in one of its subdirectories. Would
> that be surprising?
Not for anybody familiar with CVS. (Any other old-timers that can chime in here?)
> And if so, perhaps we should warn about that or
> even error out?
Certainly not.
Here is a long explanation of what is going on:
The latest patch to vc-dir-ignore (#37240) uses ewoc to get a list
of marked files. The files in this list are relative file pathes and
can also be in subdirectories. e.g.:
VC backend : CVS
Working dir: /re/po
./
unregistered .cvsignore
edited data
sub/
unregistered sub/.cvsignore
* unregistered sub/sub-file
sub/sub/
unregistered sub/sub/.cvsignore
edited sub/sub/data
Pressing G while "sub/sub-file" is marked, initiates the call chain
vc-dir-ignore => vc-ignore => vc-cvs-ignore with "sub/sub-file"
as FILE argument.
CVS has per-directory ignore files, which can only handle simple
filenames without directory parts. I.e,, it is incorrect to write
"sub/sub-file" into "/re/po/.cvsignore". Instead "sub-file" must be
written into "/re/po/sub/.cvsignore" to have the desired effect.
Therefore, it is necessary to expand the FILE parameter to
(directory-file-name (expand-file-name "sub/sub-file" "/re/po"))
=> "/re/po/sub/sub-file"
And the resulting absolute file path can be used to determine the
ignore file, which is located in the directory:
(expand-file-name ".cvsignore" (file-name-directory "/re/po/sub/sub-file"))
=> "/re/po/sub/.cvsignore"
and the string that must be written to it is the basename:
(file-name-nondirectory "/re/po/sub/sub-file") => "sub-file"
So, the algorithm is correct and I assume that anybody familiar with
CVS would expect just that behavior.
Citing from the CVS texinfo manual:
CVS has a list of files (or sh(1) file name patterns) that it should
ignore while running 'update', 'import' and 'release'. This list is
constructed in the following way.
* The list is initialized to include certain file name patterns:
[...]
* As CVS traverses through your directories, the contents of any
'.cvsignore' will be appended to the list. The patterns found in
'.cvsignore' are only valid for the directory that contains them,
not for any sub-directories.
^ permalink raw reply [flat|nested] 26+ messages in thread
* bug#37215: [PATCH] vc-cvs-ignore writes absolute filenames and duplicate strings
2020-02-13 19:36 ` Eli Zaretskii
2020-02-14 1:24 ` Wolfgang Scherer
@ 2020-02-14 2:50 ` Wolfgang Scherer
2020-02-14 8:39 ` Eli Zaretskii
1 sibling, 1 reply; 26+ messages in thread
From: Wolfgang Scherer @ 2020-02-14 2:50 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 37215, larsi
Just FYI, this is the function that writes absolute path names
into the .cvsignore file when called with an absolute path name,
which happens if there are no marked files in a
vc-dir-mode buffer and `G` is pressed.
^ permalink raw reply [flat|nested] 26+ messages in thread
* bug#37215: [PATCH] vc-cvs-ignore writes absolute filenames and duplicate strings
2020-02-14 1:24 ` Wolfgang Scherer
@ 2020-02-14 8:33 ` Eli Zaretskii
2020-02-15 1:42 ` Wolfgang Scherer
0 siblings, 1 reply; 26+ messages in thread
From: Eli Zaretskii @ 2020-02-14 8:33 UTC (permalink / raw)
To: Wolfgang Scherer; +Cc: 37215, larsi
> Cc: larsi@gnus.org, 37215@debbugs.gnu.org
> From: Wolfgang Scherer <Wolfgang.Scherer@gmx.de>
> Date: Fri, 14 Feb 2020 02:24:12 +0100
>
> >> Your assumption, that `file` is always a simple basename is wrong.
> > Yes, but when does it make sense to have FILE not absolute and not
> > just a basename (i.e. with leading directories)? Do we have such use
> > cases?
> vc-dir-ignore with patch from #37240
OK, but then please document this use case and how DIRECTORY is used
in this function. The various -ignore functions in vc.el and in
backends assign different semantics to their DIRECTORY argument, and I
think these (largely undocumented) differences are a source of some
confusion and bugs in this area.
> > Because if that happens, the file's name will be added to
> > .cvsignore not in DIRECTORY but in one of its subdirectories. Would
> > that be surprising?
> Not for anybody familiar with CVS.
This should be documented, IMO. The existing documentation of
.cvsignore is minimal, and doesn't mention several important aspects.
For example, the fact that only basenames are allowed is only hinted
upon, and there's no information whatsoever AFAICT whether characters
special to wildcards can be escaped. So I think we should provide
this minimal information either in doc strings or at least in comments
in the code.
Thanks.
^ permalink raw reply [flat|nested] 26+ messages in thread
* bug#37215: [PATCH] vc-cvs-ignore writes absolute filenames and duplicate strings
2020-02-14 2:50 ` Wolfgang Scherer
@ 2020-02-14 8:39 ` Eli Zaretskii
2020-02-14 9:24 ` Eli Zaretskii
0 siblings, 1 reply; 26+ messages in thread
From: Eli Zaretskii @ 2020-02-14 8:39 UTC (permalink / raw)
To: Wolfgang Scherer; +Cc: 37215, larsi
> Cc: larsi@gnus.org, 37215@debbugs.gnu.org
> From: Wolfgang Scherer <Wolfgang.Scherer@gmx.de>
> Date: Fri, 14 Feb 2020 03:50:32 +0100
>
> Just FYI, this is the function that writes absolute path names
> into the .cvsignore file when called with an absolute path name,
> which happens if there are no marked files in a
> vc-dir-mode buffer and `G` is pressed.
That was the reason for the bug report, and the purpose of the changes
that were installed, right? So it's quite obvious.
^ permalink raw reply [flat|nested] 26+ messages in thread
* bug#37215: [PATCH] vc-cvs-ignore writes absolute filenames and duplicate strings
2020-02-14 8:39 ` Eli Zaretskii
@ 2020-02-14 9:24 ` Eli Zaretskii
0 siblings, 0 replies; 26+ messages in thread
From: Eli Zaretskii @ 2020-02-14 9:24 UTC (permalink / raw)
To: Wolfgang.Scherer; +Cc: 37215, larsi
> Date: Fri, 14 Feb 2020 10:39:22 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 37215@debbugs.gnu.org, larsi@gnus.org
>
> That was the reason for the bug report, and the purpose of the changes
> that were installed, right? So it's quite obvious.
Sorry, I was wrong about it being installed, it seems. But
everything else is correct.
^ permalink raw reply [flat|nested] 26+ messages in thread
* bug#37215: [PATCH] vc-cvs-ignore writes absolute filenames and duplicate strings
2020-02-14 8:33 ` Eli Zaretskii
@ 2020-02-15 1:42 ` Wolfgang Scherer
2020-02-15 7:44 ` Eli Zaretskii
0 siblings, 1 reply; 26+ messages in thread
From: Wolfgang Scherer @ 2020-02-15 1:42 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 37215, larsi, Dmitry Gutov
Am 14.02.20 um 09:33 schrieb Eli Zaretskii:
>> Cc: larsi@gnus.org, 37215@debbugs.gnu.org
>> From: Wolfgang Scherer <Wolfgang.Scherer@gmx.de>
>> Date: Fri, 14 Feb 2020 02:24:12 +0100
>>
>>>> Your assumption, that `file` is always a simple basename is wrong.
>>> Yes, but when does it make sense to have FILE not absolute and not
>>> just a basename (i.e. with leading directories)? Do we have such use
>>> cases?
>> vc-dir-ignore with patch from #37240
> OK, but then please document this use case and how DIRECTORY is used
> in this function. The various -ignore functions in vc.el and in
> backends assign different semantics to their DIRECTORY argument, and I
> think these (largely undocumented) differences are a source of some
> confusion and bugs in this area.
This is one of the first errors I ran into and I know a lot more now,
after doing the research.
There is a problem with describing the use case, because
*vc-cvs-append-to-ignore* has several of them. So I think it is best,
that Dmitry comes up with an overall solution for *vc-ignore* - as he
has planned - which would naturally solve this problem, too. Since CVS
is a special case, because another package, PCL-CVS, is also involved,
therefore increasing the danger that something breaks, I have put my
findings at the end for your convenience.
>
>>> Because if that happens, the file's name will be added to
>>> .cvsignore not in DIRECTORY but in one of its subdirectories. Would
>>> that be surprising?
>> Not for anybody familiar with CVS.
> This should be documented, IMO. The existing documentation of
> .cvsignore is minimal, and doesn't mention several important aspects.
> For example, the fact that only basenames are allowed is only hinted
> upon, and there's no information whatsoever AFAICT whether characters
> special to wildcards can be escaped. So I think we should provide
> this minimal information either in doc strings or at least in comments
> in the code.
As for the pattern syntax for CVS, it is following POSIX and is fully
described in the man page for glob(7) including the escape mechanism
with backslash. There is one quirk in the .cvsignore file parser, which
breaks patterns not only at lines but also at other whitespace on the
same line. It is therefore better to match a filename containing spaces
with a `?` for each space character.
As for the use cases:
Besides **vc** there is also PCL-CVS (**pcvs**), which is also part of
GNU Emacs. PCL-CVS handles strictly CVS, nothing else.
When the *vc-ignore* feature was introduced, the function
*cvs-append-to-ignore* was moved to `vc-cvs.el` and renamed to
*vc-cvs-append-to-ignore*:
```elisp
(define-obsolete-function-alias 'cvs-append-to-ignore 'vc-cvs-append-to-ignore
"24.4")
```
1. The sole PCL-CVS use case can be found in *cvs-mode-ignore*:
```elisp
(defun-cvs-mode cvs-mode-ignore ()
"Arrange so that CVS ignores the selected files.
This command ignores files that are not flagged as `Unknown'."
(interactive)
(dolist (fi (cvs-mode-marked 'ignore))
(vc-cvs-append-to-ignore (cvs-fileinfo->dir fi) (cvs-fileinfo->file fi)
(eq (cvs-fileinfo->subtype fi) 'NEW-DIR))
(setf (cvs-fileinfo->type fi) 'DEAD))
(cvs-cleanup-collection cvs-cookies nil nil nil))
```
When *cvs-examine* is called in a CVS repository, PCL-CVS creates a
buffer `*cvs*`, which looks simiilar to *vc-dir-mode*, e.g.:
```text
Repository : /re/po/root-cvs
Module : check-cvs
Working dir: /re/po/check-cvs/
In directory .:
Unknown .cvsignore
Modified data
* Unknown test2.xx
In directory sub:
Unknown sub/.cvsignore
Modified sub/data
* Unknown sub/test2.xx
In directory sub/sub:
Unknown sub/sub/.cvsignore
Modified sub/sub/data
Unknown sub/sub/test2.xx
```
With `test2.xx` and `sub/test2.xx` marked, invoking
*cvs-mode-ignore* by pressing `i` to ignore the marked files
- writes "test2.xx" into `/re/po/check-cvs/.cvsignore`
- and "test2.xx" into `/re/po/check-cvs/sub/.cvsignore`.
As far as I am concerned, this use case is sufficiently documented
both in *cvs-mode-ignore* and *vc-cvs-append-to-ignore*, which were
written to fit together.
2. When *vc-cvs-append-to-ignore* was imported from **pcvs**, the
function *vc-dir-ignore* - when pressing `G` in *vc-dir-mode* -
```elisp
(defun vc-dir-ignore ()
"Ignore the current file."
(interactive)
(vc-ignore (vc-dir-current-file)))
```
only used the current file - not all marked files - and sent the
absolute pathname effectively to *vc-cvs-ignore*
```elisp
(defun vc-cvs-ignore (file &optional _directory _remove)
"Ignore FILE under CVS."
(vc-cvs-append-to-ignore (file-name-directory file) file))
```
As can be seen from *cvs-mode-ignore*, the invocation of
*vc-cvs-append-to-ignore* for the case of an absolute pathname
should have been:
```elisp
(vc-cvs-append-to-ignore (file-name-directory file)
(file-name-nondirectory file)))
```
3. Pressing `C-x v G` to invoke *vc-ignore* interactively, prompts for
an absolute pathname FILE, which is described as:
Normally, FILE is a wildcard specification that matches the files
to be ignored..
This actually works with the current implementation, if FILE does
not contain any directory components. It does not work, if the
default absolute pathname is sent as is (same use case as with
*vc-dir-ignore*).
So it seems the correct solution for all three use cases requires that
*vc-dir-cvsignore* call *vc-ignore* in the same manner as
*cvs-mode-ignore* calls *vc-cvs-append-to-ignore*:
```elisp
(defun vc-dir-ignore ()
"Ignore the current file."
(interactive)
(let ((fi (vc-dir-current-file)))
(vc-ignore (file-name-nondirectory fi) (file-name-directory fi))))
```
and *vc-cvs-ignore* just passes on both DIRECTORY and FILE:
```elisp
(defun vc-cvs-ignore (file &optional directory _remove)
"Ignore FILE under CVS."
(vc-cvs-append-to-ignore directory file))
```
However, this would mean that all modifications so far (except maybe for
SVN) have to be mostly rolled back, which will lead to other problems.
For my own needs, which is to cover the range of Emacsen from 22 to 26
and beyond, I have prepared a standalone extension package which is
mostly independent from the:current defun:vc-ignore subsystem, It shows,
that the problem can be solved and you are welcome to refer to the
escape and anchoring mechanisms there [wolfmanx/vc-ign: Emacs VC ignore
feature](https://github.com/wolfmanx/vc-ign).
^ permalink raw reply [flat|nested] 26+ messages in thread
* bug#37215: [PATCH] vc-cvs-ignore writes absolute filenames and duplicate strings
2020-02-15 1:42 ` Wolfgang Scherer
@ 2020-02-15 7:44 ` Eli Zaretskii
2020-02-15 12:55 ` Dmitry Gutov
` (2 more replies)
0 siblings, 3 replies; 26+ messages in thread
From: Eli Zaretskii @ 2020-02-15 7:44 UTC (permalink / raw)
To: Wolfgang Scherer; +Cc: 37215, larsi, dgutov
> Cc: larsi@gnus.org, 37215@debbugs.gnu.org, Dmitry Gutov <dgutov@yandex.ru>
> From: Wolfgang Scherer <Wolfgang.Scherer@gmx.de>
> Date: Sat, 15 Feb 2020 02:42:28 +0100
>
> >> vc-dir-ignore with patch from #37240
> > OK, but then please document this use case and how DIRECTORY is used
> > in this function. The various -ignore functions in vc.el and in
> > backends assign different semantics to their DIRECTORY argument, and I
> > think these (largely undocumented) differences are a source of some
> > confusion and bugs in this area.
> This is one of the first errors I ran into and I know a lot more now,
> after doing the research.
>
> There is a problem with describing the use case, because
> *vc-cvs-append-to-ignore* has several of them. So I think it is best,
> that Dmitry comes up with an overall solution for *vc-ignore* - as he
> has planned - which would naturally solve this problem, too. Since CVS
> is a special case, because another package, PCL-CVS, is also involved,
> therefore increasing the danger that something breaks, I have put my
> findings at the end for your convenience.
> >
> >>> Because if that happens, the file's name will be added to
> >>> .cvsignore not in DIRECTORY but in one of its subdirectories. Would
> >>> that be surprising?
> >> Not for anybody familiar with CVS.
> > This should be documented, IMO. The existing documentation of
> > .cvsignore is minimal, and doesn't mention several important aspects.
> > For example, the fact that only basenames are allowed is only hinted
> > upon, and there's no information whatsoever AFAICT whether characters
> > special to wildcards can be escaped. So I think we should provide
> > this minimal information either in doc strings or at least in comments
> > in the code.
>
> As for the pattern syntax for CVS, it is following POSIX and is fully
> described in the man page for glob(7) including the escape mechanism
> with backslash. There is one quirk in the .cvsignore file parser, which
> breaks patterns not only at lines but also at other whitespace on the
> same line. It is therefore better to match a filename containing spaces
> with a `?` for each space character.
I'd like to install your patch, and I'd like to do it soon, so it
makes it into Emacs 27. Can you please propose a modified patch which
adds some minimal information about what's going on to the doc string
and/or the comments around the code?
IOW, I don't think we need any further discussions of this issue, we
need a somewhat improved patch that explains more why its code is
correct. Do you think you could provide such an improved patch? I
don't think it's wise to make this patch wait for untangling the more
general vc-ignore issues.
Thanks.
^ permalink raw reply [flat|nested] 26+ messages in thread
* bug#37215: [PATCH] vc-cvs-ignore writes absolute filenames and duplicate strings
2020-02-15 7:44 ` Eli Zaretskii
@ 2020-02-15 12:55 ` Dmitry Gutov
2020-02-19 23:02 ` Wolfgang Scherer
2020-02-16 0:20 ` Wolfgang Scherer
2020-02-19 23:06 ` Wolfgang Scherer
2 siblings, 1 reply; 26+ messages in thread
From: Dmitry Gutov @ 2020-02-15 12:55 UTC (permalink / raw)
To: Eli Zaretskii, Wolfgang Scherer; +Cc: 37215, larsi
On 15.02.2020 9:44, Eli Zaretskii wrote:
> I
> don't think it's wise to make this patch wait for untangling the more
> general vc-ignore issues.
And I'm hoping this patch will fit the current contract of vc-ignore
(however imprecise it is), rather than the redesigned one.
^ permalink raw reply [flat|nested] 26+ messages in thread
* bug#37215: [PATCH] vc-cvs-ignore writes absolute filenames and duplicate strings
2020-02-15 7:44 ` Eli Zaretskii
2020-02-15 12:55 ` Dmitry Gutov
@ 2020-02-16 0:20 ` Wolfgang Scherer
2020-02-19 23:06 ` Wolfgang Scherer
2 siblings, 0 replies; 26+ messages in thread
From: Wolfgang Scherer @ 2020-02-16 0:20 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 37215, larsi, dgutov
Am 15.02.20 um 08:44 schrieb Eli Zaretskii:
> I'd like to install your patch, and I'd like to do it soon, so it
> makes it into Emacs 27. Can you please propose a modified patch which
> adds some minimal information about what's going on to the doc string
> and/or the comments around the code?
Will do, and I will keep it short.
>
> IOW, I don't think we need any further discussions of this issue,
Definitely not.
> we
> need a somewhat improved patch that explains more why its code is
> correct.
Although it can't be 100% correct, I will try.
> Do you think you could provide such an improved patch?
Yes, some time on Sunday.
> I
> don't think it's wise to make this patch wait for untangling the more
> general vc-ignore issues.
OK.
^ permalink raw reply [flat|nested] 26+ messages in thread
* bug#37215: [PATCH] vc-cvs-ignore writes absolute filenames and duplicate strings
2020-02-15 12:55 ` Dmitry Gutov
@ 2020-02-19 23:02 ` Wolfgang Scherer
0 siblings, 0 replies; 26+ messages in thread
From: Wolfgang Scherer @ 2020-02-19 23:02 UTC (permalink / raw)
To: Dmitry Gutov, Eli Zaretskii; +Cc: 37215, larsi
Am 15.02.20 um 13:55 schrieb Dmitry Gutov:
> On 15.02.2020 9:44, Eli Zaretskii wrote:
>> I
>> don't think it's wise to make this patch wait for untangling the more
>> general vc-ignore issues.
>
> And I'm hoping this patch will fit the current contract of vc-ignore (however imprecise it is), rather than the redesigned one.
Unfortunately, I have no idea, what that contract is. Could you state it for me?
The attached patch fits the use case, where pressing "G" in vc-dir-mode sends a relative or absolute file path to vc-ignore, which passes it along to vc-cvs-ignore. The patch correctly identifies the ignore file and writes the correct filename part (basename).
^ permalink raw reply [flat|nested] 26+ messages in thread
* bug#37215: [PATCH] vc-cvs-ignore writes absolute filenames and duplicate strings
2020-02-15 7:44 ` Eli Zaretskii
2020-02-15 12:55 ` Dmitry Gutov
2020-02-16 0:20 ` Wolfgang Scherer
@ 2020-02-19 23:06 ` Wolfgang Scherer
2020-02-21 9:31 ` Eli Zaretskii
2 siblings, 1 reply; 26+ messages in thread
From: Wolfgang Scherer @ 2020-02-19 23:06 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 37215, larsi, dgutov
[-- Attachment #1: Type: text/plain, Size: 420 bytes --]
Am 15.02.20 um 08:44 schrieb Eli Zaretskii:
>
> I'd like to install your patch, and I'd like to do it soon, so it
> makes it into Emacs 27. Can you please propose a modified patch which
> adds some minimal information about what's going on to the doc string
> and/or the comments around the code?
Here is the revised patch. I have cleared the FIXME issue mentioned by Lars and added commentary to what is going on.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Do-not-write-absolute-filenames-and-duplicate-string.patch --]
[-- Type: text/x-patch; name="0001-Do-not-write-absolute-filenames-and-duplicate-string.patch", Size: 4224 bytes --]
From 375bdad57a2b56c63722e1d93c1e1de13566e8a2 Mon Sep 17 00:00:00 2001
From: Wolfgang Scherer <wolfgang.scherer@gmx.de>
Date: Wed, 19 Feb 2020 23:53:10 +0100
Subject: [PATCH] Do not write absolute filenames and duplicate strings into
.cvsignore
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* lisp/vc/vc-cvs.el: (vc-cvs-ignore) Expand filename correctly
and pass on basename only.
(vc-cvs-append-to-ignore) Do not write duplicate strings to
.cvsignore. Addtional optional parameter SORT.
(lisp/vc/pcvs.el) Call ‘vc-cvs-append-to-ignore’ with SORT argument.
---
lisp/vc/pcvs.el | 4 ++--
lisp/vc/vc-cvs.el | 40 +++++++++++++++++++++++++++-------------
2 files changed, 29 insertions(+), 15 deletions(-)
diff --git a/lisp/vc/pcvs.el b/lisp/vc/pcvs.el
index dcba504..cb0494e 100644
--- a/lisp/vc/pcvs.el
+++ b/lisp/vc/pcvs.el
@@ -106,7 +106,6 @@
;; right now, it's killed without further ado.
;; - make `cvs-mode-ignore' allow manually entering a pattern.
;; to which dir should it apply ?
-;; - cvs-mode-ignore should try to remove duplicate entries.
;; - maybe poll/check CVS/Entries files to react to external `cvs' commands ?
;; - some kind of `cvs annotate' support ?
;; but vc-annotate can be used instead.
@@ -1972,7 +1971,8 @@ This command ignores files that are not flagged as `Unknown'."
(interactive)
(dolist (fi (cvs-mode-marked 'ignore))
(vc-cvs-append-to-ignore (cvs-fileinfo->dir fi) (cvs-fileinfo->file fi)
- (eq (cvs-fileinfo->subtype fi) 'NEW-DIR))
+ (eq (cvs-fileinfo->subtype fi) 'NEW-DIR)
+ cvs-sort-ignore-file)
(setf (cvs-fileinfo->type fi) 'DEAD))
(cvs-cleanup-collection cvs-cookies nil nil nil))
diff --git a/lisp/vc/vc-cvs.el b/lisp/vc/vc-cvs.el
index 16566a8..00459e8 100644
--- a/lisp/vc/vc-cvs.el
+++ b/lisp/vc/vc-cvs.el
@@ -1220,14 +1220,27 @@ 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)
+(defun vc-cvs-ignore (file &optional directory _remove)
+ "Ignore FILE under CVS.
+FILE is either absolute or relative to DIRECTORY.
+
+There is a CVS ignore file in each subdirectory. Patterns only
+match files in the same directory. Since FILE can be a relative
+filename with leading diretories, FILE is expanded against
+DIRECTORY to determine the correct absolute filename. This path
+is then used to determine the directory and the pattern for the
+ignore file.
+
+Patterns follow glob(7) syntax. Special characters \"?*[\\\" are
+escaped with a backslash."
+ (setq file (directory-file-name (expand-file-name file directory)))
+ (vc-cvs-append-to-ignore (file-name-directory file) (file-name-nondirectory file)))
+
+(defun vc-cvs-append-to-ignore (dir str &optional old-dir sort)
"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."
+to hear about anymore. If SORT is non-nil, sort the ines of the
+ignore file."
(with-current-buffer
(find-file-noselect (expand-file-name ".cvsignore" dir))
(when (ignore-errors
@@ -1236,13 +1249,14 @@ to hear about anymore."
(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)))
+ (goto-char (point-min))
+ (save-match-data
+ (unless (re-search-forward (concat "^" (regexp-quote str) "$") nil t)
+ (goto-char (point-max))
+ (unless (bolp) (insert "\n"))
+ (insert str (if old-dir "/\n" "\n"))
+ (if sort (sort-lines nil (point-min) (point-max)))
+ (save-buffer)))))
(provide 'vc-cvs)
--
2.7.4
^ permalink raw reply related [flat|nested] 26+ messages in thread
* bug#37215: [PATCH] vc-cvs-ignore writes absolute filenames and duplicate strings
2020-02-19 23:06 ` Wolfgang Scherer
@ 2020-02-21 9:31 ` Eli Zaretskii
2020-02-21 10:16 ` Dmitry Gutov
2020-02-21 20:32 ` Wolfgang Scherer
0 siblings, 2 replies; 26+ messages in thread
From: Eli Zaretskii @ 2020-02-21 9:31 UTC (permalink / raw)
To: Wolfgang Scherer; +Cc: 37215, larsi, dgutov
> Cc: larsi@gnus.org, 37215@debbugs.gnu.org, dgutov@yandex.ru
> From: Wolfgang Scherer <Wolfgang.Scherer@gmx.de>
> Date: Thu, 20 Feb 2020 00:06:11 +0100
>
> > I'd like to install your patch, and I'd like to do it soon, so it
> > makes it into Emacs 27. Can you please propose a modified patch which
> > adds some minimal information about what's going on to the doc string
> > and/or the comments around the code?
>
> Here is the revised patch. I have cleared the FIXME issue mentioned by Lars and added commentary to what is going on.
Thanks, this LGTM. Just a couple of minor nits, and we can install
this.
> (lisp/vc/pcvs.el) Call ‘vc-cvs-append-to-ignore’ with SORT argument.
Please quote 'like this' in log messages, and try to avoid non-ASCII
characters there (they are generally only necessary in people's
names).
> +Patterns follow glob(7) syntax. Special characters \"?*[\\\" are
> +escaped with a backslash."
I'd say "should be escaped" here, since this is a requirement for the
argument passed to this function.
Also, I'd mention that FILE can be a pattern, otherwise the reference
to patterns might come as a surprise to the reader.
> +to hear about anymore. If SORT is non-nil, sort the ines of the
> +ignore file." ^^^^
Typo: should be "lines".
> + (goto-char (point-min))
> + (save-match-data
> + (unless (re-search-forward (concat "^" (regexp-quote str) "$") nil t)
> + (goto-char (point-max))
You could use non-nil, non-t 3rd argument of re-search-forward, in
which case the following goto-char would be redundant, right?
^ permalink raw reply [flat|nested] 26+ messages in thread
* bug#37215: [PATCH] vc-cvs-ignore writes absolute filenames and duplicate strings
2020-02-21 9:31 ` Eli Zaretskii
@ 2020-02-21 10:16 ` Dmitry Gutov
2020-02-21 20:44 ` Wolfgang Scherer
2020-02-21 20:32 ` Wolfgang Scherer
1 sibling, 1 reply; 26+ messages in thread
From: Dmitry Gutov @ 2020-02-21 10:16 UTC (permalink / raw)
To: Eli Zaretskii, Wolfgang Scherer; +Cc: 37215, larsi
On 21.02.2020 11:31, Eli Zaretskii wrote:
> Also, I'd mention that FILE can be a pattern, otherwise the reference
> to patterns might come as a surprise to the reader.
And, if we accept the patch from a neighboring bug report, it will never
be an absolute file name.
^ permalink raw reply [flat|nested] 26+ messages in thread
* bug#37215: [PATCH] vc-cvs-ignore writes absolute filenames and duplicate strings
2020-02-21 9:31 ` Eli Zaretskii
2020-02-21 10:16 ` Dmitry Gutov
@ 2020-02-21 20:32 ` Wolfgang Scherer
2020-02-22 8:59 ` Eli Zaretskii
1 sibling, 1 reply; 26+ messages in thread
From: Wolfgang Scherer @ 2020-02-21 20:32 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 37215, larsi, dgutov
[-- Attachment #1: Type: text/plain, Size: 1716 bytes --]
Am 21.02.20 um 10:31 schrieb Eli Zaretskii:
>> Cc: larsi@gnus.org, 37215@debbugs.gnu.org, dgutov@yandex.ru
>> From: Wolfgang Scherer <Wolfgang.Scherer@gmx.de>
>> Date: Thu, 20 Feb 2020 00:06:11 +0100
>> (lisp/vc/pcvs.el) Call ‘vc-cvs-append-to-ignore’ with SORT argument.
> Please quote 'like this' in log messages, and try to avoid non-ASCII
> characters there (they are generally only necessary in people's
> names).
OK
>> +Patterns follow glob(7) syntax. Special characters \"?*[\\\" are
>> +escaped with a backslash."
> I'd say "should be escaped" here, since this is a requirement for the
> argument passed to this function.
Here is that amgbuity again ;-). It is only required, if the user
wants a special character to match literally. It's perfectly fine to
specify *.pyc as a pattern. I have phrased it like that.
> Also, I'd mention that FILE can be a pattern, otherwise the reference
> to patterns might come as a surprise to the reader.
I emphasized more, that the basename of the FILE argument (not the
entire FILE) is in fact a CVS ignore pattern.\x15\x04
>> +to hear about anymore. If SORT is non-nil, sort the ines of the
>> +ignore file." ^^^^
> Typo: should be "lines".
Right.
>> + (goto-char (point-min))
>> + (save-match-data
>> + (unless (re-search-forward (concat "^" (regexp-quote str) "$") nil t)
>> + (goto-char (point-max))
> You could use non-nil, non-t 3rd argument of re-search-forward, in
> which case the following goto-char would be redundant, right?
Right, I just left the goto-char in there, because it makes it
obvious what is going on. Switching to the side-effect optimization ...
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Do-not-write-absolute-filenames-and-duplicate-string.patch --]
[-- Type: text/x-patch; name="0001-Do-not-write-absolute-filenames-and-duplicate-string.patch", Size: 4541 bytes --]
From edea70777b81f745b86f812c7ca2e0021a9ecb83 Mon Sep 17 00:00:00 2001
From: Wolfgang Scherer <wolfgang.scherer@gmx.de>
Date: Fri, 21 Feb 2020 21:28:11 +0100
Subject: [PATCH] Do not write absolute filenames and duplicate strings into
CVS ignore files
* lisp/vc/vc-cvs.el: (vc-cvs-ignore) Expand filename correctly
and pass on basename as pattern only.
(vc-cvs-append-to-ignore) Do not write duplicate strings to
.cvsignore. Addtional optional parameter SORT.
(lisp/vc/pcvs.el) Call 'vc-cvs-append-to-ignore' with SORT argument.
---
lisp/vc/pcvs.el | 4 ++--
lisp/vc/vc-cvs.el | 45 ++++++++++++++++++++++++++++++++-------------
2 files changed, 34 insertions(+), 15 deletions(-)
diff --git a/lisp/vc/pcvs.el b/lisp/vc/pcvs.el
index dcba504..cb0494e 100644
--- a/lisp/vc/pcvs.el
+++ b/lisp/vc/pcvs.el
@@ -106,7 +106,6 @@
;; right now, it's killed without further ado.
;; - make `cvs-mode-ignore' allow manually entering a pattern.
;; to which dir should it apply ?
-;; - cvs-mode-ignore should try to remove duplicate entries.
;; - maybe poll/check CVS/Entries files to react to external `cvs' commands ?
;; - some kind of `cvs annotate' support ?
;; but vc-annotate can be used instead.
@@ -1972,7 +1971,8 @@ This command ignores files that are not flagged as `Unknown'."
(interactive)
(dolist (fi (cvs-mode-marked 'ignore))
(vc-cvs-append-to-ignore (cvs-fileinfo->dir fi) (cvs-fileinfo->file fi)
- (eq (cvs-fileinfo->subtype fi) 'NEW-DIR))
+ (eq (cvs-fileinfo->subtype fi) 'NEW-DIR)
+ cvs-sort-ignore-file)
(setf (cvs-fileinfo->type fi) 'DEAD))
(cvs-cleanup-collection cvs-cookies nil nil nil))
diff --git a/lisp/vc/vc-cvs.el b/lisp/vc/vc-cvs.el
index 16566a8..b6afda6 100644
--- a/lisp/vc/vc-cvs.el
+++ b/lisp/vc/vc-cvs.el
@@ -1220,14 +1220,33 @@ 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)
+(defun vc-cvs-ignore (file &optional directory _remove)
+ "Ignore FILE under CVS.
+FILE is either absolute or relative to DIRECTORY. The basename
+of FILE is written unmodified into the ignore file and is
+therefore evaluated by CVS as an ignore pattern which follows
+glob(7) syntax. If the pattern should match any of the special
+characters ‘?*[\\\’ literally, they must be escaped with a
+backslash.
+
+CVS processes one ignore file for each subdirectory. Patterns
+are separated by whitespace and only match files in the same
+directory. Since FILE can be a relative filename with leading
+diretories, FILE is expanded against DIRECTORY to determine the
+correct absolute filename. The directory name of this path is
+then used to determine the location of the ignore file. The base
+name of this path is used as pattern for the ignore file.
+
+Since patterns are whitespace sparated, it is usually better to
+replace spaces in filenames with question marks ‘?’."
+ (setq file (directory-file-name (expand-file-name file directory)))
+ (vc-cvs-append-to-ignore (file-name-directory file) (file-name-nondirectory file)))
+
+(defun vc-cvs-append-to-ignore (dir str &optional old-dir sort)
"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."
+to hear about anymore. If SORT is non-nil, sort the lines of the
+ignore file."
(with-current-buffer
(find-file-noselect (expand-file-name ".cvsignore" dir))
(when (ignore-errors
@@ -1236,13 +1255,13 @@ to hear about anymore."
(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)))
+ (goto-char (point-min))
+ (save-match-data
+ (unless (re-search-forward (concat "^" (regexp-quote str) "$") nil 'move)
+ (unless (bolp) (insert "\n"))
+ (insert str (if old-dir "/\n" "\n"))
+ (if sort (sort-lines nil (point-min) (point-max)))
+ (save-buffer)))))
(provide 'vc-cvs)
--
2.7.4
^ permalink raw reply related [flat|nested] 26+ messages in thread
* bug#37215: [PATCH] vc-cvs-ignore writes absolute filenames and duplicate strings
2020-02-21 10:16 ` Dmitry Gutov
@ 2020-02-21 20:44 ` Wolfgang Scherer
2020-02-21 21:30 ` Dmitry Gutov
0 siblings, 1 reply; 26+ messages in thread
From: Wolfgang Scherer @ 2020-02-21 20:44 UTC (permalink / raw)
To: Dmitry Gutov, Eli Zaretskii; +Cc: 37215, larsi
Am 21.02.20 um 11:16 schrieb Dmitry Gutov:
> On 21.02.2020 11:31, Eli Zaretskii wrote:
>> Also, I'd mention that FILE can be a pattern, otherwise the reference
>> to patterns might come as a surprise to the reader.
>
> And, if we accept the patch from a neighboring bug report, it will never be an absolute file name.
I am not aware of that bug report.
Currently absolute file names still come in via vc-dir-ignore . I'd
rather not document a possible future, since it seems hard enough to
document the current state ;-)
\x7f
^ permalink raw reply [flat|nested] 26+ messages in thread
* bug#37215: [PATCH] vc-cvs-ignore writes absolute filenames and duplicate strings
2020-02-21 20:44 ` Wolfgang Scherer
@ 2020-02-21 21:30 ` Dmitry Gutov
2020-02-21 22:23 ` Wolfgang Scherer
0 siblings, 1 reply; 26+ messages in thread
From: Dmitry Gutov @ 2020-02-21 21:30 UTC (permalink / raw)
To: Wolfgang Scherer; +Cc: 37215
On 21.02.2020 22:44, Wolfgang Scherer wrote:
> I am not aware of that bug report.
Here's a recent message with a patch waiting for feedback from you as well:
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37189#143
^ permalink raw reply [flat|nested] 26+ messages in thread
* bug#37215: [PATCH] vc-cvs-ignore writes absolute filenames and duplicate strings
2020-02-21 21:30 ` Dmitry Gutov
@ 2020-02-21 22:23 ` Wolfgang Scherer
0 siblings, 0 replies; 26+ messages in thread
From: Wolfgang Scherer @ 2020-02-21 22:23 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 37215
Am 21.02.20 um 22:30 schrieb Dmitry Gutov:
> On 21.02.2020 22:44, Wolfgang Scherer wrote:
>> I am not aware of that bug report.
>
> Here's a recent message with a patch waiting for feedback from you as well:
>
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37189#143
I see. I just answered.
^ permalink raw reply [flat|nested] 26+ messages in thread
* bug#37215: [PATCH] vc-cvs-ignore writes absolute filenames and duplicate strings
2020-02-21 20:32 ` Wolfgang Scherer
@ 2020-02-22 8:59 ` Eli Zaretskii
0 siblings, 0 replies; 26+ messages in thread
From: Eli Zaretskii @ 2020-02-22 8:59 UTC (permalink / raw)
To: Wolfgang Scherer; +Cc: larsi, 37215-done, dgutov
> Cc: larsi@gnus.org, 37215@debbugs.gnu.org, dgutov@yandex.ru
> From: Wolfgang Scherer <Wolfgang.Scherer@gmx.de>
> Date: Fri, 21 Feb 2020 21:32:03 +0100
>
> > Please quote 'like this' in log messages, and try to avoid non-ASCII
> > characters there (they are generally only necessary in people's
> > names).
> OK
> >> +Patterns follow glob(7) syntax. Special characters \"?*[\\\" are
> >> +escaped with a backslash."
> > I'd say "should be escaped" here, since this is a requirement for the
> > argument passed to this function.
> Here is that amgbuity again ;-). It is only required, if the user
> wants a special character to match literally. It's perfectly fine to
> specify *.pyc as a pattern. I have phrased it like that.
> > Also, I'd mention that FILE can be a pattern, otherwise the reference
> > to patterns might come as a surprise to the reader.
> I emphasized more, that the basename of the FILE argument (not the
> entire FILE) is in fact a CVS ignore pattern.\x15\x04
> >> +to hear about anymore. If SORT is non-nil, sort the ines of the
> >> +ignore file." ^^^^
> > Typo: should be "lines".
> Right.
> >> + (goto-char (point-min))
> >> + (save-match-data
> >> + (unless (re-search-forward (concat "^" (regexp-quote str) "$") nil t)
> >> + (goto-char (point-max))
> > You could use non-nil, non-t 3rd argument of re-search-forward, in
> > which case the following goto-char would be redundant, right?
> Right, I just left the goto-char in there, because it makes it
> obvious what is going on. Switching to the side-effect optimization ...
Thanks, pushed to the release branch.
Please note that I need to make minor copyedits in the log message;
please try following this style in the future.
With this, I'm closing this bug report.
^ permalink raw reply [flat|nested] 26+ messages in thread
* bug#37215: [PATCH] vc-cvs-ignore writes absolute filenames and duplicate strings
2019-08-28 22:32 bug#37215: [PATCH] vc-cvs-ignore writes absolute filenames and duplicate strings Wolfgang Scherer
2019-09-15 13:12 ` Lars Ingebrigtsen
@ 2020-02-28 16:07 ` Mattias Engdegård
2020-02-28 16:24 ` Eli Zaretskii
1 sibling, 1 reply; 26+ messages in thread
From: Mattias Engdegård @ 2020-02-28 16:07 UTC (permalink / raw)
To: Eli Zaretskii, Wolfgang Scherer, Lars Ingebrigtsen, Dmitry Gutov; +Cc: 37215
Some doc string tweaks committed to emacs-27 (4dec693f70); hope that's all right.
^ permalink raw reply [flat|nested] 26+ messages in thread
* bug#37215: [PATCH] vc-cvs-ignore writes absolute filenames and duplicate strings
2020-02-28 16:07 ` Mattias Engdegård
@ 2020-02-28 16:24 ` Eli Zaretskii
0 siblings, 0 replies; 26+ messages in thread
From: Eli Zaretskii @ 2020-02-28 16:24 UTC (permalink / raw)
To: Mattias Engdegård; +Cc: 37215, larsi, Wolfgang.Scherer, dgutov
> From: Mattias Engdegård <mattiase@acm.org>
> Date: Fri, 28 Feb 2020 17:07:14 +0100
> Cc: 37215@debbugs.gnu.org
>
> Some doc string tweaks committed to emacs-27 (4dec693f70); hope that's all right.
Documentation fixes are always OK on the release branch.
Thanks.
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2020-02-28 16:24 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-28 22:32 bug#37215: [PATCH] vc-cvs-ignore writes absolute filenames and duplicate strings Wolfgang Scherer
2019-09-15 13:12 ` Lars Ingebrigtsen
2020-01-05 3:59 ` Wolfgang Scherer
2020-01-22 12:43 ` Lars Ingebrigtsen
2020-01-30 19:44 ` Wolfgang Scherer
2020-02-13 19:36 ` Eli Zaretskii
2020-02-14 1:24 ` Wolfgang Scherer
2020-02-14 8:33 ` Eli Zaretskii
2020-02-15 1:42 ` Wolfgang Scherer
2020-02-15 7:44 ` Eli Zaretskii
2020-02-15 12:55 ` Dmitry Gutov
2020-02-19 23:02 ` Wolfgang Scherer
2020-02-16 0:20 ` Wolfgang Scherer
2020-02-19 23:06 ` Wolfgang Scherer
2020-02-21 9:31 ` Eli Zaretskii
2020-02-21 10:16 ` Dmitry Gutov
2020-02-21 20:44 ` Wolfgang Scherer
2020-02-21 21:30 ` Dmitry Gutov
2020-02-21 22:23 ` Wolfgang Scherer
2020-02-21 20:32 ` Wolfgang Scherer
2020-02-22 8:59 ` Eli Zaretskii
2020-02-14 2:50 ` Wolfgang Scherer
2020-02-14 8:39 ` Eli Zaretskii
2020-02-14 9:24 ` Eli Zaretskii
2020-02-28 16:07 ` Mattias Engdegård
2020-02-28 16:24 ` Eli Zaretskii
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).