GNU bug report logs -
#37215
[PATCH] vc-cvs-ignore writes absolute filenames and duplicate strings
Previous Next
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 37215 in the body.
You can then email your comments to 37215 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#37215; Package
emacs.
(Wed, 28 Aug 2019 22:33:02 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Wolfgang Scherer <Wolfgang.Scherer <at> gmx.de>:
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org.
(Wed, 28 Aug 2019 22:33:02 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
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.
[0001-Do-not-write-absolute-filenames-and-duplicate-string.patch (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#37215; Package
emacs.
(Sun, 15 Sep 2019 13:13:02 GMT)
Full text and
rfc822 format available.
Message #8 received at 37215 <at> debbugs.gnu.org (full text, mbox):
Wolfgang Scherer <Wolfgang.Scherer <at> 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
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#37215; Package
emacs.
(Sun, 05 Jan 2020 04:01:02 GMT)
Full text and
rfc822 format available.
Message #11 received at 37215 <at> debbugs.gnu.org (full text, mbox):
Am 15.09.19 um 15:12 schrieb Lars Ingebrigtsen:
> Wolfgang Scherer <Wolfgang.Scherer <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#37215; Package
emacs.
(Wed, 22 Jan 2020 12:44:01 GMT)
Full text and
rfc822 format available.
Message #14 received at 37215 <at> debbugs.gnu.org (full text, mbox):
Wolfgang Scherer <Wolfgang.Scherer <at> gmx.de> writes:
> Am 15.09.19 um 15:12 schrieb Lars Ingebrigtsen:
>> Wolfgang Scherer <Wolfgang.Scherer <at> 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
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#37215; Package
emacs.
(Thu, 30 Jan 2020 19:45:02 GMT)
Full text and
rfc822 format available.
Message #17 received at 37215 <at> debbugs.gnu.org (full text, mbox):
Am 22.01.20 um 13:43 schrieb Lars Ingebrigtsen:
> Wolfgang Scherer <Wolfgang.Scherer <at> 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/")
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#37215; Package
emacs.
(Thu, 13 Feb 2020 19:37:02 GMT)
Full text and
rfc822 format available.
Message #20 received at 37215 <at> debbugs.gnu.org (full text, mbox):
> From: Wolfgang Scherer <Wolfgang.Scherer <at> gmx.de>
> Date: Thu, 30 Jan 2020 20:44:00 +0100
> Cc: 37215 <at> 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?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#37215; Package
emacs.
(Fri, 14 Feb 2020 01:25:02 GMT)
Full text and
rfc822 format available.
Message #23 received at 37215 <at> debbugs.gnu.org (full text, mbox):
Am 13.02.20 um 20:36 schrieb Eli Zaretskii:
>> From: Wolfgang Scherer <Wolfgang.Scherer <at> gmx.de>
>> Date: Thu, 30 Jan 2020 20:44:00 +0100
>> Cc: 37215 <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#37215; Package
emacs.
(Fri, 14 Feb 2020 02:51:02 GMT)
Full text and
rfc822 format available.
Message #26 received at 37215 <at> debbugs.gnu.org (full text, mbox):
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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#37215; Package
emacs.
(Fri, 14 Feb 2020 08:34:02 GMT)
Full text and
rfc822 format available.
Message #29 received at 37215 <at> debbugs.gnu.org (full text, mbox):
> Cc: larsi <at> gnus.org, 37215 <at> debbugs.gnu.org
> From: Wolfgang Scherer <Wolfgang.Scherer <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#37215; Package
emacs.
(Fri, 14 Feb 2020 08:40:01 GMT)
Full text and
rfc822 format available.
Message #32 received at 37215 <at> debbugs.gnu.org (full text, mbox):
> Cc: larsi <at> gnus.org, 37215 <at> debbugs.gnu.org
> From: Wolfgang Scherer <Wolfgang.Scherer <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#37215; Package
emacs.
(Fri, 14 Feb 2020 09:25:01 GMT)
Full text and
rfc822 format available.
Message #35 received at 37215 <at> debbugs.gnu.org (full text, mbox):
> Date: Fri, 14 Feb 2020 10:39:22 +0200
> From: Eli Zaretskii <eliz <at> gnu.org>
> Cc: 37215 <at> debbugs.gnu.org, larsi <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#37215; Package
emacs.
(Sat, 15 Feb 2020 01:43:01 GMT)
Full text and
rfc822 format available.
Message #38 received at 37215 <at> debbugs.gnu.org (full text, mbox):
Am 14.02.20 um 09:33 schrieb Eli Zaretskii:
>> Cc: larsi <at> gnus.org, 37215 <at> debbugs.gnu.org
>> From: Wolfgang Scherer <Wolfgang.Scherer <at> 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).
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#37215; Package
emacs.
(Sat, 15 Feb 2020 07:45:01 GMT)
Full text and
rfc822 format available.
Message #41 received at 37215 <at> debbugs.gnu.org (full text, mbox):
> Cc: larsi <at> gnus.org, 37215 <at> debbugs.gnu.org, Dmitry Gutov <dgutov <at> yandex.ru>
> From: Wolfgang Scherer <Wolfgang.Scherer <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#37215; Package
emacs.
(Sat, 15 Feb 2020 12:56:02 GMT)
Full text and
rfc822 format available.
Message #44 received at 37215 <at> debbugs.gnu.org (full text, mbox):
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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#37215; Package
emacs.
(Sun, 16 Feb 2020 00:21:01 GMT)
Full text and
rfc822 format available.
Message #47 received at 37215 <at> debbugs.gnu.org (full text, mbox):
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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#37215; Package
emacs.
(Wed, 19 Feb 2020 23:03:01 GMT)
Full text and
rfc822 format available.
Message #50 received at 37215 <at> debbugs.gnu.org (full text, mbox):
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).
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#37215; Package
emacs.
(Wed, 19 Feb 2020 23:07:01 GMT)
Full text and
rfc822 format available.
Message #53 received at 37215 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
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.
[0001-Do-not-write-absolute-filenames-and-duplicate-string.patch (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#37215; Package
emacs.
(Fri, 21 Feb 2020 09:32:02 GMT)
Full text and
rfc822 format available.
Message #56 received at 37215 <at> debbugs.gnu.org (full text, mbox):
> Cc: larsi <at> gnus.org, 37215 <at> debbugs.gnu.org, dgutov <at> yandex.ru
> From: Wolfgang Scherer <Wolfgang.Scherer <at> 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?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#37215; Package
emacs.
(Fri, 21 Feb 2020 10:17:02 GMT)
Full text and
rfc822 format available.
Message #59 received at 37215 <at> debbugs.gnu.org (full text, mbox):
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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#37215; Package
emacs.
(Fri, 21 Feb 2020 20:33:02 GMT)
Full text and
rfc822 format available.
Message #62 received at 37215 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Am 21.02.20 um 10:31 schrieb Eli Zaretskii:
>> Cc: larsi <at> gnus.org, 37215 <at> debbugs.gnu.org, dgutov <at> yandex.ru
>> From: Wolfgang Scherer <Wolfgang.Scherer <at> 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.
>> +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 ...
[0001-Do-not-write-absolute-filenames-and-duplicate-string.patch (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#37215; Package
emacs.
(Fri, 21 Feb 2020 20:46:02 GMT)
Full text and
rfc822 format available.
Message #65 received at 37215 <at> debbugs.gnu.org (full text, mbox):
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 ;-)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#37215; Package
emacs.
(Fri, 21 Feb 2020 21:31:02 GMT)
Full text and
rfc822 format available.
Message #68 received at 37215 <at> debbugs.gnu.org (full text, mbox):
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
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#37215; Package
emacs.
(Fri, 21 Feb 2020 22:24:01 GMT)
Full text and
rfc822 format available.
Message #71 received at 37215 <at> debbugs.gnu.org (full text, mbox):
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.
Reply sent
to
Eli Zaretskii <eliz <at> gnu.org>:
You have taken responsibility.
(Sat, 22 Feb 2020 09:01:01 GMT)
Full text and
rfc822 format available.
Notification sent
to
Wolfgang Scherer <Wolfgang.Scherer <at> gmx.de>:
bug acknowledged by developer.
(Sat, 22 Feb 2020 09:01:02 GMT)
Full text and
rfc822 format available.
Message #76 received at 37215-done <at> debbugs.gnu.org (full text, mbox):
> Cc: larsi <at> gnus.org, 37215 <at> debbugs.gnu.org, dgutov <at> yandex.ru
> From: Wolfgang Scherer <Wolfgang.Scherer <at> 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.
> >> +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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#37215; Package
emacs.
(Fri, 28 Feb 2020 16:08:01 GMT)
Full text and
rfc822 format available.
Message #79 received at 37215 <at> debbugs.gnu.org (full text, mbox):
Some doc string tweaks committed to emacs-27 (4dec693f70); hope that's all right.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#37215; Package
emacs.
(Fri, 28 Feb 2020 16:26:02 GMT)
Full text and
rfc822 format available.
Message #82 received at 37215 <at> debbugs.gnu.org (full text, mbox):
> From: Mattias Engdegård <mattiase <at> acm.org>
> Date: Fri, 28 Feb 2020 17:07:14 +0100
> Cc: 37215 <at> 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.
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org.
(Sat, 28 Mar 2020 11:24:04 GMT)
Full text and
rfc822 format available.
This bug report was last modified 4 years and 23 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.