* bug#63949: 30.0.50; `vc-print-log´ does not erase buffer when called from *vc-change-log* buffer, at least for CVS logs @ 2023-06-07 21:04 Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-08 13:12 ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors ` (2 more replies) 0 siblings, 3 replies; 34+ messages in thread From: Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-07 21:04 UTC (permalink / raw) To: 63949 Start emacs -Q. Visit a CVS-controlled file. Then: C-x v l M-> C-x v l Notice the CVS log added once more after current end of log buffer. I tracked this down to the following snippet from `vc-do-command´: (save-current-buffer (unless (or (eq buffer t) (and (stringp buffer) (string= (buffer-name) buffer)) (eq buffer (current-buffer))) (vc-setup-buffer buffer)) That is, `vc-setup-buffer´ (which erases the buffer) is *not* called if the buffer where the VC command is to be executed in equals the current buffer. I guess that logic has a reason, so it probably should be some upper layer which should erase *vc-log-buffer*... For git, for example, function `vc-git-print-log´ calls `vc-setup-buffer´. Probably the other VC backends should follow suit? If this is the way to go, I could provide patches for VC backends CVS and RCS, pls let me know. In GNU Emacs 30.0.50 (build 2, x86_64-pc-linux-gnu, GTK+ Version 3.24.24, cairo version 1.16.0) of 2023-06-07 built on sappc2 Repository revision: a902156068ab071f93cc9bbd34cd320919b74064 Repository branch: master Windowing system distributor 'The X.Org Foundation', version 11.0.12011000 System Description: Debian GNU/Linux 11 (bullseye) Configured features: ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY INOTIFY PDUMPER PNG RSVG SECCOMP SOUND THREADS TIFF TOOLKIT_SCROLL_BARS X11 XDBE XIM XINPUT2 XPM GTK3 ZLIB Important settings: value of $LC_COLLATE: POSIX value of $LC_TIME: POSIX value of $LANG: en_US.UTF-8 value of $XMODIFIERS: @im=ibus locale-coding-system: utf-8-unix Major mode: Git-Log-View Minor modes in effect: tooltip-mode: t global-eldoc-mode: t show-paren-mode: t electric-indent-mode: t mouse-wheel-mode: t tool-bar-mode: t menu-bar-mode: t file-name-shadow-mode: t global-font-lock-mode: t font-lock-mode: t blink-cursor-mode: t buffer-read-only: t line-number-mode: t indent-tabs-mode: t transient-mark-mode: t auto-composition-mode: t auto-encryption-mode: t auto-compression-mode: t Load-path shadows: None found. Features: (shadow sort mail-extr emacsbug message mailcap yank-media puny dired dired-loaddefs rfc822 mml mml-sec password-cache epa derived epg rfc6068 epg-config gnus-util text-property-search mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils parse-time iso8601 time-date subr-x sh-script rx smie treesit cl-seq executable misearch multi-isearch vc-hg vc-bzr vc-src vc-sccs vc-svn vc-cvs vc-rcs add-log log-view pcvs-util cl-extra help-mode vc vc-git diff-mode easy-mmode vc-dispatcher cl-loaddefs cl-lib rmc iso-transl tooltip cconv eldoc paren electric uniquify ediff-hook vc-hooks lisp-float-type elisp-mode mwheel term/x-win x-win term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe tabulated-list replace newcomment text-mode lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow isearch easymenu timer select scroll-bar mouse jit-lock font-lock syntax font-core term/tty-colors frame minibuffer nadvice seq simple cl-generic indonesian philippine cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european ethiopic indian cyrillic chinese composite emoji-zwj charscript charprop case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button loaddefs theme-loaddefs faces cus-face macroexp files window text-properties overlay sha1 md5 base64 format env code-pages mule custom widget keymap hashtable-print-readable backquote threads dbusbind inotify lcms2 dynamic-setting system-font-setting font-render-setting cairo move-toolbar gtk x-toolkit xinput2 x multi-tty make-network-process emacs) Memory information: ((conses 16 61575 13745) (symbols 48 7721 0) (strings 32 23157 2232) (string-bytes 1 634764) (vectors 16 14390) (vector-slots 8 207631 12167) (floats 8 35 32) (intervals 56 586 0) (buffers 976 13)) ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#63949: 30.0.50; `vc-print-log´ does not erase buffer when called from *vc-change-log* buffer, at least for CVS logs 2023-06-07 21:04 bug#63949: 30.0.50; `vc-print-log´ does not erase buffer when called from *vc-change-log* buffer, at least for CVS logs Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-08 13:12 ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-08 20:21 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-08 22:44 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-08 23:09 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2 siblings, 1 reply; 34+ messages in thread From: Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-08 13:12 UTC (permalink / raw) To: Jens Schmidt; +Cc: 63949 Jens Schmidt <jschmidt4gnu@vodafonemail.de> writes: [...] > For git, for example, function `vc-git-print-log´ calls > `vc-setup-buffer´. Probably the other VC backends should follow suit? > > If this is the way to go, I could provide patches for VC backends CVS > and RCS, pls let me know. FWIW, I tested with mercurial (vc-hg.el) and it works correctly (as with git) but it have the same call to 'vc-setup-buffer' in 'vc-hg-print-log'. So I guess it would be logical that others backend "print-log" functions have this same call. (note: I could not test it but SVN and Bazaar print-log functions also have this call to 'vc-setup-buffer') -- Manuel Giraud ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#63949: 30.0.50; `vc-print-log´ does not erase buffer when called from *vc-change-log* buffer, at least for CVS logs 2023-06-08 13:12 ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-08 20:21 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-08 21:33 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 1 reply; 34+ messages in thread From: Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-08 20:21 UTC (permalink / raw) To: Manuel Giraud; +Cc: 63949 On 2023-06-08 15:12, Manuel Giraud wrote: > FWIW, I tested with mercurial (vc-hg.el) and it works correctly (as with > git) but it have the same call to 'vc-setup-buffer' in > 'vc-hg-print-log'. So I guess it would be logical that others backend > "print-log" functions have this same call. > > (note: I could not test it but SVN and Bazaar print-log functions also > have this call to 'vc-setup-buffer') Thanks for checking this out. Will provide patches for CVS and RCS in that direction. Probably I'll even do SCCS, which seems to lack the call to `vc-setup-buffer' as well. ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#63949: 30.0.50; `vc-print-log´ does not erase buffer when called from *vc-change-log* buffer, at least for CVS logs 2023-06-08 20:21 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-08 21:33 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-08 22:10 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 1 reply; 34+ messages in thread From: Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-08 21:33 UTC (permalink / raw) To: Manuel Giraud; +Cc: 63949 Root cause of this issue being that `vc-deduce-fileset' does not change buffer in Emacs 29 and newer, while it does in Emacs 27 and older. (Too lazy to check for Emacs 28.) So in Emacs 27 and older, a call to `vc-print-log' started from *vc-change-log* would change buffer to the vc parent buffer before calling the backend function, and thus `vc-do-command' would call `vc-setup-buffer' in the snippet mentioned above. ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#63949: 30.0.50; `vc-print-log´ does not erase buffer when called from *vc-change-log* buffer, at least for CVS logs 2023-06-08 21:33 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-08 22:10 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-09 6:09 ` Eli Zaretskii 0 siblings, 1 reply; 34+ messages in thread From: Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-08 22:10 UTC (permalink / raw) To: Manuel Giraud; +Cc: 63949 Caused by commit d494833d47968fcd97ba549654a259d6fb6c2eee. There is another subtle change to (most likely) that, which seems to affect all backends: When calling `vc-print-log' from *vc-change-buffer*, it now modifies the VC parent buffer to itself, which is of course incorrect. ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#63949: 30.0.50; `vc-print-log´ does not erase buffer when called from *vc-change-log* buffer, at least for CVS logs 2023-06-08 22:10 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-09 6:09 ` Eli Zaretskii 2023-06-09 20:27 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 1 reply; 34+ messages in thread From: Eli Zaretskii @ 2023-06-09 6:09 UTC (permalink / raw) To: Jens Schmidt; +Cc: 63949, manuel > Cc: 63949@debbugs.gnu.org > Date: Fri, 9 Jun 2023 00:10:57 +0200 > From: Jens Schmidt via "Bug reports for GNU Emacs, > the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org> > > Caused by commit d494833d47968fcd97ba549654a259d6fb6c2eee. Which means this was a regression in Emacs 28, and we should try to fix it in Emacs 29. > There is another subtle change to (most likely) that, which seems to > affect all backends: When calling `vc-print-log' from > *vc-change-buffer*, it now modifies the VC parent buffer to itself, > which is of course incorrect. What do you mean by "modifies the VC parent buffer to itself"? Do you mean that vc-print-log makes the *vc-change-buffer* be the VC parent buffer? If so, why is this incorrect? ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#63949: 30.0.50; `vc-print-log´ does not erase buffer when called from *vc-change-log* buffer, at least for CVS logs 2023-06-09 6:09 ` Eli Zaretskii @ 2023-06-09 20:27 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-10 6:01 ` Eli Zaretskii 0 siblings, 1 reply; 34+ messages in thread From: Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-09 20:27 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 63949, manuel On 2023-06-09 08:09, Eli Zaretskii wrote: > What do you mean by "modifies the VC parent buffer to itself"? Do > you mean that vc-print-log makes the *vc-change-buffer* be the VC > parent buffer? If so, why is this incorrect? Hope below recipe explains it. The general idea is to start a VC command first from the buffer of a VC-controlled file, then from the VC result buffer, again from the VC result buffer, etc. emacs -Q (from emacs-29) <visit git-controlled file, say README> ;; current buffer equals README C-x v l ;; current buffer equals *vc-change-log* C-h v vc-parent-buffer => #<buffer README> ;; current buffer equals *vc-change-log* C-x v l C-h v vc-parent-buffer => #<buffer *vc-change-log*> ;; current buffer equals *vc-change-log* C-x v l ;; git log of whole repository is displayed, not of README I backed out commit d494833d47968fcd97ba549654a259d6fb6c2eee from emacs-29 - both issues (the initial CVS-related one and the above one) are gone after having done so. Not all VC commands expose this problem. `vc-annotate' seems to work and even the VC parent buffer stays unchanged when invoking `vc-annotate' repeatedly. `vc-diff' gets the parent buffer wrong but seems to work when invoked from buffer *vc-diff* repeatedly even if parent buffer is wrong. ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#63949: 30.0.50; `vc-print-log´ does not erase buffer when called from *vc-change-log* buffer, at least for CVS logs 2023-06-09 20:27 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-10 6:01 ` Eli Zaretskii 2023-06-10 15:44 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 1 reply; 34+ messages in thread From: Eli Zaretskii @ 2023-06-10 6:01 UTC (permalink / raw) To: Jens Schmidt; +Cc: 63949, manuel > Date: Fri, 9 Jun 2023 22:27:37 +0200 > Cc: manuel@ledu-giraud.fr, 63949@debbugs.gnu.org > From: Jens Schmidt <jschmidt4gnu@vodafonemail.de> > > emacs -Q (from emacs-29) > <visit git-controlled file, say README> > > ;; current buffer equals README > C-x v l > ;; current buffer equals *vc-change-log* > C-h v vc-parent-buffer > => #<buffer README> > > ;; current buffer equals *vc-change-log* > C-x v l > C-h v vc-parent-buffer > => #<buffer *vc-change-log*> > > ;; current buffer equals *vc-change-log* > C-x v l > ;; git log of whole repository is displayed, not of README Thanks for the explanations. > Not all VC commands expose this problem. Why do you think it's a problem? I can justify this behavior, at least in some use cases. ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#63949: 30.0.50; `vc-print-log´ does not erase buffer when called from *vc-change-log* buffer, at least for CVS logs 2023-06-10 6:01 ` Eli Zaretskii @ 2023-06-10 15:44 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-10 15:55 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-10 17:23 ` Eli Zaretskii 0 siblings, 2 replies; 34+ messages in thread From: Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-10 15:44 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 63949, manuel On 2023-06-10 08:01, Eli Zaretskii wrote: > Why do you think it's a problem? I can justify this behavior, at > least in some use cases. Let's distinguish "VC-controlled buffers" like the buffer of a VC-controlled file or a *vc-dir* buffer or a dired buffer of VC-controlled directory. And "VC working buffers", like *vc-change-log*, *vc-log*, *vc-diff*. It is my understanding that `vc-parent-buffer' in a VC working buffer points to the VC-controlled buffer from which it originates. The rationale of that variable is to allow VC operations from a VC working buffer as if executed on the original VC-controlled buffer. So I can do C-x v l, pick a commit, do a C-x v ~ on that commit, then a C-x v =, and all these operations would automagically relate to the original VC-controlled buffer. At least I use that concept frequently. The documentation on `vc-parent-buffer', unfortunately, is out of date and does not necessarily support my understanding: ;; In a log entry buffer, this is a local variable ;; that points to the buffer for which it was made ;; (either a file, or a directory buffer). However, this has been working as described by me up to and including Emacs 27. So at least we can say that the fix fur bug#40967 has changed established behavior. And also the following code snippet from function `vc-deduce-fileset-1' seems to prove my point: ((and (buffer-live-p vc-parent-buffer) ;; FIXME: Why this test? --Stef (or (buffer-file-name vc-parent-buffer) (with-current-buffer vc-parent-buffer (or (derived-mode-p 'vc-dir-mode) (derived-mode-p 'dired-mode) (derived-mode-p 'diff-mode))))) (progn ;FIXME: Why not `with-current-buffer'? --Stef. (set-buffer vc-parent-buffer) (vc-deduce-fileset-1 not-state-changing allow-unregistered state-model-only-files))) Meaning: If the current buffer has a live vc-parent-buffer, this function switches to it and deduces the fileset from that. Plus it leaves the VC parent buffer current, which is important for follow-up code to keep the VC parent buffer unchanged. That logic used to work as intended (by me) up to Nathan's commit, which put a `with-current-buffer' around the whole function and rendered the `set-buffer' side effect pointless. Let's put it that way: The pre-28 logic of handling the VC parent buffer was not necessarily clean, as also pointed out by Stefan. But I think the concept of having a stable VC parent buffer across multiple VC operations is nice, and changing it midway according to rather unclear rules undesirable. Ideally, we would have a fix that handled indirect buffers and VC parent buffers (which are somewhat similar by concept) all consistently and nicely and in a stable way, at the same time fixing both issues that I have. I'll mediate about that... ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#63949: 30.0.50; `vc-print-log´ does not erase buffer when called from *vc-change-log* buffer, at least for CVS logs 2023-06-10 15:44 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-10 15:55 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-10 17:23 ` Eli Zaretskii 1 sibling, 0 replies; 34+ messages in thread From: Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-10 15:55 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 63949, manuel > I'll mediate about that... *meditate* ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#63949: 30.0.50; `vc-print-log´ does not erase buffer when called from *vc-change-log* buffer, at least for CVS logs 2023-06-10 15:44 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-10 15:55 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-10 17:23 ` Eli Zaretskii 2023-06-10 21:18 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-16 19:33 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 1 sibling, 2 replies; 34+ messages in thread From: Eli Zaretskii @ 2023-06-10 17:23 UTC (permalink / raw) To: Jens Schmidt; +Cc: 63949, manuel > Date: Sat, 10 Jun 2023 17:44:26 +0200 > Cc: manuel@ledu-giraud.fr, 63949@debbugs.gnu.org > From: Jens Schmidt <jschmidt4gnu@vodafonemail.de> > > On 2023-06-10 08:01, Eli Zaretskii wrote: > > > Why do you think it's a problem? I can justify this behavior, at > > least in some use cases. > > Let's distinguish "VC-controlled buffers" like the buffer of a > VC-controlled file or a *vc-dir* buffer or a dired buffer of > VC-controlled directory. And "VC working buffers", like > *vc-change-log*, *vc-log*, *vc-diff*. First, let me clarify what I alluded to as "a problem": I meant the fact that "C-x v l" in a *vc-change-log* buffer shows the log for the entire repository. How this is accomplished by setting vc-parent-buffer is an implementation detail. > It is my understanding that `vc-parent-buffer' in a VC working buffer > points to the VC-controlled buffer from which it originates. The > rationale of that variable is to allow VC operations from a VC working > buffer as if executed on the original VC-controlled buffer. So I can do > C-x v l, pick a commit, do a C-x v ~ on that commit, then a C-x v =, and > all these operations would automagically relate to the original > VC-controlled buffer. At least I use that concept frequently. If the backend is a VCS that records changes per-file, what you want will happen automatically, since "C-x v l" and other operations must in general refer to a file with those VCSes. For backends that record changes per-repository, why does it make sense that typing "C-x v l" from a buffer that already shows a log should produce the same log again? > The documentation on `vc-parent-buffer', unfortunately, is out of date > and does not necessarily support my understanding: > > ;; In a log entry buffer, this is a local variable > ;; that points to the buffer for which it was made > ;; (either a file, or a directory buffer). We need to define what we want vc-parent-buffer to mean, and then we should adjust the documentation and keep our promises about that variable in the future. > However, this has been working as described by me up to and including > Emacs 27. So at least we can say that the fix fur bug#40967 has changed > established behavior. Probably because no one realized that this must be the established behavior. > That logic used to work as intended (by me) up to Nathan's commit, which > put a `with-current-buffer' around the whole function and rendered the > `set-buffer' side effect pointless. > > Let's put it that way: The pre-28 logic of handling the VC parent buffer > was not necessarily clean, as also pointed out by Stefan. But I think > the concept of having a stable VC parent buffer across multiple VC > operations is nice, and changing it midway according to rather unclear > rules undesirable. > > Ideally, we would have a fix that handled indirect buffers and VC parent > buffers (which are somewhat similar by concept) all consistently and > nicely and in a stable way, at the same time fixing both issues that I > have. I'll mediate about that... If you can suggest changes to make the treatment of vc-parent-buffer more consistent, that would be good, I think. I'd also like to hear Dmitry's views on these issues. He was until now silent in this discussion, AFAICT. ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#63949: 30.0.50; `vc-print-log´ does not erase buffer when called from *vc-change-log* buffer, at least for CVS logs 2023-06-10 17:23 ` Eli Zaretskii @ 2023-06-10 21:18 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-11 5:06 ` Eli Zaretskii 2023-06-16 19:33 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 1 sibling, 1 reply; 34+ messages in thread From: Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-10 21:18 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 63949, manuel On 2023-06-10 19:23, Eli Zaretskii wrote: > If the backend is a VCS that records changes per-file, what you want > will happen automatically, since "C-x v l" and other operations must > in general refer to a file with those VCSes. For backends that > record changes per-repository, why does it make sense that typing > "C-x v l" from a buffer that already shows a log should produce the > same log again? To get it updated, for example because I perform some operation outside of Emacs. One of these habits ... > If you can suggest changes to make the treatment of vc-parent-buffer > more consistent, that would be good, I think. Here is another minor ugliness I forgot to mention so far which also should be defined and fixed, then: Whenever the `vc-parent-buffer' goes "out of sync" (as far as I am concerned) in some VC working buffer, its mode line shows, e.g. *vc-change-log* Top 1 (Git-Log-View from *vc-change-log*) or *vc-diff [...] (Diff ws from *vc-diff*) since mode line variable `vc-parent-buffer-name' goes likewise out of sync. > I'd also like to hear Dmitry's views on these issues. He was until > now silent in this discussion, AFAICT. Agreed. ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#63949: 30.0.50; `vc-print-log´ does not erase buffer when called from *vc-change-log* buffer, at least for CVS logs 2023-06-10 21:18 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-11 5:06 ` Eli Zaretskii 0 siblings, 0 replies; 34+ messages in thread From: Eli Zaretskii @ 2023-06-11 5:06 UTC (permalink / raw) To: Jens Schmidt; +Cc: 63949, manuel > Date: Sat, 10 Jun 2023 23:18:01 +0200 > Cc: manuel@ledu-giraud.fr, 63949@debbugs.gnu.org > From: Jens Schmidt <jschmidt4gnu@vodafonemail.de> > > On 2023-06-10 19:23, Eli Zaretskii wrote: > > > If the backend is a VCS that records changes per-file, what you want > > will happen automatically, since "C-x v l" and other operations must > > in general refer to a file with those VCSes. For backends that > > record changes per-repository, why does it make sense that typing > > "C-x v l" from a buffer that already shows a log should produce the > > same log again? > > To get it updated, for example because I perform some operation outside > of Emacs. One of these habits ... If there's more than one natural meaning of a command, the usual way we solve the dilemma is by using a prefix argument. ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#63949: 30.0.50; `vc-print-log´ does not erase buffer when called from *vc-change-log* buffer, at least for CVS logs 2023-06-10 17:23 ` Eli Zaretskii 2023-06-10 21:18 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-16 19:33 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-17 3:15 ` Dmitry Gutov 2023-06-18 2:42 ` Dmitry Gutov 1 sibling, 2 replies; 34+ messages in thread From: Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-16 19:33 UTC (permalink / raw) To: Eli Zaretskii, Dmitry Gutov; +Cc: 63949 On 2023-06-10 19:23, Eli Zaretskii wrote: > I'd also like to hear Dmitry's views on these issues. He was until > now silent in this discussion, AFAICT. @Dmitry: What's your view on these issues? Just making the handling of VC parent buffer more documented and "more consistent", whatever that means, or do you prefer some particular direction? Thanks ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#63949: 30.0.50; `vc-print-log´ does not erase buffer when called from *vc-change-log* buffer, at least for CVS logs 2023-06-16 19:33 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-17 3:15 ` Dmitry Gutov 2023-06-18 2:42 ` Dmitry Gutov 1 sibling, 0 replies; 34+ messages in thread From: Dmitry Gutov @ 2023-06-17 3:15 UTC (permalink / raw) To: Jens Schmidt, Eli Zaretskii; +Cc: 63949 On 16/06/2023 22:33, Jens Schmidt wrote: > On 2023-06-10 19:23, Eli Zaretskii wrote: > >> I'd also like to hear Dmitry's views on these issues. He was until >> now silent in this discussion, AFAICT. > > @Dmitry: What's your view on these issues? Just making the handling of > VC parent buffer more documented and "more consistent", whatever that > means, or do you prefer some particular direction? Sorry, been busy. I'll make sure to read the thread and comment on the issue tomorrow. ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#63949: 30.0.50; `vc-print-log´ does not erase buffer when called from *vc-change-log* buffer, at least for CVS logs 2023-06-16 19:33 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-17 3:15 ` Dmitry Gutov @ 2023-06-18 2:42 ` Dmitry Gutov 2023-06-18 5:35 ` Eli Zaretskii 1 sibling, 1 reply; 34+ messages in thread From: Dmitry Gutov @ 2023-06-18 2:42 UTC (permalink / raw) To: Jens Schmidt, Eli Zaretskii; +Cc: 63949 On 16/06/2023 22:33, Jens Schmidt wrote: > On 2023-06-10 19:23, Eli Zaretskii wrote: > >> I'd also like to hear Dmitry's views on these issues. He was until >> now silent in this discussion, AFAICT. > > @Dmitry: What's your view on these issues? Just making the handling of > VC parent buffer more documented and "more consistent", whatever that > means, or do you prefer some particular direction? I think the appropriate thing here is to back out of the change that caused the regression (d494833d47968fcd97ba549654a259d6fb6c2eee, as we've found out) and then try to re-fix it some other way (in master?). Or maybe adjust the current code such as when vc-deduce-fileset-1 does change the current buffer, vc-deduce-fileset retains that change. For example, using this ugly-ish (100% untested) patch: diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el index 91d3f6f70d3..91aae40a677 100644 --- a/lisp/vc/vc.el +++ b/lisp/vc/vc.el @@ -1121,10 +1121,13 @@ vc-deduce-fileset the returned list. BEWARE: this function may change the current buffer." - (with-current-buffer (or (buffer-base-buffer) (current-buffer)) - (vc-deduce-fileset-1 not-state-changing - allow-unregistered - state-model-only-files))) + (let (new-buf) + (with-current-buffer (or (buffer-base-buffer) (current-buffer)) + (vc-deduce-fileset-1 not-state-changing + allow-unregistered + state-model-only-files) + (setq new-buf (current-buffer))) + (set-buffer new-buf))) (defun vc-deduce-fileset-1 (not-state-changing allow-unregistered The fact that some backends do call vc-setup-buffer inside vc-xx-print-log and some dont', also bears investigation. But the question I would like to have answered is, can we drop this call from all of them? Rather than trying to add it everywhere. ^ permalink raw reply related [flat|nested] 34+ messages in thread
* bug#63949: 30.0.50; `vc-print-log´ does not erase buffer when called from *vc-change-log* buffer, at least for CVS logs 2023-06-18 2:42 ` Dmitry Gutov @ 2023-06-18 5:35 ` Eli Zaretskii 2023-06-18 9:11 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 1 reply; 34+ messages in thread From: Eli Zaretskii @ 2023-06-18 5:35 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 63949, jschmidt4gnu > Date: Sun, 18 Jun 2023 05:42:44 +0300 > Cc: 63949@debbugs.gnu.org > From: Dmitry Gutov <dmitry@gutov.dev> > > On 16/06/2023 22:33, Jens Schmidt wrote: > > On 2023-06-10 19:23, Eli Zaretskii wrote: > > > >> I'd also like to hear Dmitry's views on these issues. He was until > >> now silent in this discussion, AFAICT. > > > > @Dmitry: What's your view on these issues? Just making the handling of > > VC parent buffer more documented and "more consistent", whatever that > > means, or do you prefer some particular direction? > > I think the appropriate thing here is to back out of the change that > caused the regression (d494833d47968fcd97ba549654a259d6fb6c2eee, as > we've found out) and then try to re-fix it some other way (in master?). > > Or maybe adjust the current code such as when vc-deduce-fileset-1 does > change the current buffer, vc-deduce-fileset retains that change. > > For example, using this ugly-ish (100% untested) patch: Whatever you decide, please do it soon, as I'd like to release another pretest of Emacs 29. Thanks. ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#63949: 30.0.50; `vc-print-log´ does not erase buffer when called from *vc-change-log* buffer, at least for CVS logs 2023-06-18 5:35 ` Eli Zaretskii @ 2023-06-18 9:11 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-18 12:00 ` Dmitry Gutov 0 siblings, 1 reply; 34+ messages in thread From: Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-18 9:11 UTC (permalink / raw) To: Eli Zaretskii, Dmitry Gutov; +Cc: 63949 On 2023-06-18 07:35, Eli Zaretskii wrote: > Whatever you decide, please do it soon, as I'd like to release > another pretest of Emacs 29. No need to change anything for Emacs 29, I think. The issues that I encountered are not that serious, and whatever gets implemented to rectify them and commit d494833d47968fcd97ba549654a259d6fb6c2eee in a consistent way would be too much for Emacs 29, anyway. ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#63949: 30.0.50; `vc-print-log´ does not erase buffer when called from *vc-change-log* buffer, at least for CVS logs 2023-06-18 9:11 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-18 12:00 ` Dmitry Gutov [not found] ` <e695eaa4-2f39-1b20-1cd4-fe7fdaeb3d61@vodafonemail.de> 0 siblings, 1 reply; 34+ messages in thread From: Dmitry Gutov @ 2023-06-18 12:00 UTC (permalink / raw) To: Jens Schmidt, Eli Zaretskii; +Cc: 63949 On 18/06/2023 12:11, Jens Schmidt wrote: > On 2023-06-18 07:35, Eli Zaretskii wrote: > >> Whatever you decide, please do it soon, as I'd like to release another >> pretest of Emacs 29. > > No need to change anything for Emacs 29, I think. The issues that I > encountered are not that serious, and whatever gets implemented to > rectify them and commit d494833d47968fcd97ba549654a259d6fb6c2eee in a > consistent way would be too much for Emacs 29, anyway. Could you test the patch I showed? ^ permalink raw reply [flat|nested] 34+ messages in thread
[parent not found: <e695eaa4-2f39-1b20-1cd4-fe7fdaeb3d61@vodafonemail.de>]
* bug#63949: 30.0.50; `vc-print-log´ does not erase buffer when called from *vc-change-log* buffer, at least for CVS logs [not found] ` <e695eaa4-2f39-1b20-1cd4-fe7fdaeb3d61@vodafonemail.de> @ 2023-06-20 2:41 ` Dmitry Gutov 2023-06-21 13:03 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 1 reply; 34+ messages in thread From: Dmitry Gutov @ 2023-06-20 2:41 UTC (permalink / raw) To: Jens Schmidt; +Cc: 63949, Eli Zaretskii (Please keep the bug address in Cc). On 18/06/2023 16:21, Jens Schmidt wrote: > On 2023-06-18 14:00, Dmitry Gutov wrote: > >> Could you test the patch I showed? > > Will do, pls just give me some time here... Thanks. If we can verify that the part broken by the bisected revision, is fixed here, we could push that to Emacs 29 and then clean up for Emacs 30 more thoroughly. In the meantime, here's the updated patch. The previous one discarded the return value :( diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el index 91d3f6f70d3..c8b2b3ac11d 100644 --- a/lisp/vc/vc.el +++ b/lisp/vc/vc.el @@ -1121,10 +1121,15 @@ vc-deduce-fileset the returned list. BEWARE: this function may change the current buffer." - (with-current-buffer (or (buffer-base-buffer) (current-buffer)) - (vc-deduce-fileset-1 not-state-changing - allow-unregistered - state-model-only-files))) + (let (new-buf res) + (with-current-buffer (or (buffer-base-buffer) (current-buffer)) + (setq res + (vc-deduce-fileset-1 not-state-changing + allow-unregistered + state-model-only-files)) + (setq new-buf (current-buffer))) + (set-buffer new-buf) + res)) (defun vc-deduce-fileset-1 (not-state-changing allow-unregistered ^ permalink raw reply related [flat|nested] 34+ messages in thread
* bug#63949: 30.0.50; `vc-print-log´ does not erase buffer when called from *vc-change-log* buffer, at least for CVS logs 2023-06-20 2:41 ` Dmitry Gutov @ 2023-06-21 13:03 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-21 14:37 ` Dmitry Gutov 0 siblings, 1 reply; 34+ messages in thread From: Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-21 13:03 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 63949, Eli Zaretskii On 2023-06-20 04:41, Dmitry Gutov wrote: > (Please keep the bug address in Cc). Will do. > Thanks. If we can verify that the part broken by the bisected > revision, is fixed here, we could push that to Emacs 29 and then > clean up for Emacs 30 more thoroughly. Both of my issues (described on "Wed, 7 Jun 2023 23:04:50 +0200" and on "Fri, 9 Jun 2023 22:27:37 +0200") are fixed by your latest, second patch. Tested on emacs-29. Thanks! ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#63949: 30.0.50; `vc-print-log´ does not erase buffer when called from *vc-change-log* buffer, at least for CVS logs 2023-06-21 13:03 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-21 14:37 ` Dmitry Gutov 2023-06-21 15:30 ` Eli Zaretskii 0 siblings, 1 reply; 34+ messages in thread From: Dmitry Gutov @ 2023-06-21 14:37 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 63949, Jens Schmidt On 21/06/2023 16:03, Jens Schmidt wrote: > On 2023-06-20 04:41, Dmitry Gutov wrote: > >> (Please keep the bug address in Cc). > > Will do. > >> Thanks. If we can verify that the part broken by the bisected >> revision, is fixed here, we could push that to Emacs 29 and then clean >> up for Emacs 30 more thoroughly. > > Both of my issues (described on "Wed, 7 Jun 2023 23:04:50 +0200" and on > "Fri, 9 Jun 2023 22:27:37 +0200") are fixed by your latest, second patch. > > Tested on emacs-29. Excellent, thank you. Eli, I suggest we push it to emacs-29 now. There could be ways to make it prettier, but probably not by much -- at least I don't see any obvious ones if we're keeping the indirect buffer-related functionality. Anyway, we can look into that later in Emacs 30. ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#63949: 30.0.50; `vc-print-log´ does not erase buffer when called from *vc-change-log* buffer, at least for CVS logs 2023-06-21 14:37 ` Dmitry Gutov @ 2023-06-21 15:30 ` Eli Zaretskii 2023-06-24 3:08 ` Dmitry Gutov 0 siblings, 1 reply; 34+ messages in thread From: Eli Zaretskii @ 2023-06-21 15:30 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 63949, jschmidt4gnu > Date: Wed, 21 Jun 2023 17:37:55 +0300 > Cc: 63949@debbugs.gnu.org, Jens Schmidt <jschmidt4gnu@vodafonemail.de> > From: Dmitry Gutov <dmitry@gutov.dev> > > On 21/06/2023 16:03, Jens Schmidt wrote: > > Tested on emacs-29. > > Excellent, thank you. > > Eli, I suggest we push it to emacs-29 now. Fine by me, thanks. > There could be ways to make it prettier, but probably not by much -- at > least I don't see any obvious ones if we're keeping the indirect > buffer-related functionality. Anyway, we can look into that later in > Emacs 30. Right. ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#63949: 30.0.50; `vc-print-log´ does not erase buffer when called from *vc-change-log* buffer, at least for CVS logs 2023-06-21 15:30 ` Eli Zaretskii @ 2023-06-24 3:08 ` Dmitry Gutov 2023-06-26 19:54 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-29 21:37 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 2 replies; 34+ messages in thread From: Dmitry Gutov @ 2023-06-24 3:08 UTC (permalink / raw) To: Jens Schmidt; +Cc: 63949, Eli Zaretskii [-- Attachment #1: Type: text/plain, Size: 745 bytes --] On 21/06/2023 18:30, Eli Zaretskii wrote: >> Date: Wed, 21 Jun 2023 17:37:55 +0300 >> Cc:63949@debbugs.gnu.org, Jens Schmidt<jschmidt4gnu@vodafonemail.de> >> From: Dmitry Gutov<dmitry@gutov.dev> >> >> On 21/06/2023 16:03, Jens Schmidt wrote: >>> Tested on emacs-29. >> Excellent, thank you. >> >> Eli, I suggest we push it to emacs-29 now. > Fine by me, thanks. And done. >> There could be ways to make it prettier, but probably not by much -- at >> least I don't see any obvious ones if we're keeping the indirect >> buffer-related functionality. Anyway, we can look into that later in >> Emacs 30. > Right. I'm attaching a patch which should work fine, though could be a tad risky for 29. Jens (and possibly others), could you try it out? [-- Attachment #2: vc-deduce-fileset.diff --] [-- Type: text/x-patch, Size: 1369 bytes --] diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el index 410fe5c01e1..24e786eb218 100644 --- a/lisp/vc/vc.el +++ b/lisp/vc/vc.el @@ -1121,19 +1121,8 @@ vc-deduce-fileset the returned list. BEWARE: this function may change the current buffer." - (let (new-buf res) - (with-current-buffer (or (buffer-base-buffer) (current-buffer)) - (setq res - (vc-deduce-fileset-1 not-state-changing - allow-unregistered - state-model-only-files)) - (setq new-buf (current-buffer))) - (set-buffer new-buf) - res)) - -(defun vc-deduce-fileset-1 (not-state-changing - allow-unregistered - state-model-only-files) + (when (buffer-base-buffer) + (set-buffer (buffer-base-buffer))) (let (backend) (cond ((derived-mode-p 'vc-dir-mode) @@ -1158,7 +1147,7 @@ vc-deduce-fileset-1 (derived-mode-p 'diff-mode))))) (progn ;FIXME: Why not `with-current-buffer'? --Stef. (set-buffer vc-parent-buffer) - (vc-deduce-fileset-1 not-state-changing allow-unregistered state-model-only-files))) + (vc-deduce-fileset not-state-changing allow-unregistered state-model-only-files))) ((and (not buffer-file-name) (setq backend (vc-responsible-backend default-directory))) (list backend nil)) ^ permalink raw reply related [flat|nested] 34+ messages in thread
* bug#63949: 30.0.50; `vc-print-log´ does not erase buffer when called from *vc-change-log* buffer, at least for CVS logs 2023-06-24 3:08 ` Dmitry Gutov @ 2023-06-26 19:54 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-29 21:37 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 1 sibling, 0 replies; 34+ messages in thread From: Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-26 19:54 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 63949, Eli Zaretskii On 2023-06-24 05:08, Dmitry Gutov wrote: > Jens (and possibly others), could you try it out? Thanks, will do, might take some time again ... ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#63949: 30.0.50; `vc-print-log´ does not erase buffer when called from *vc-change-log* buffer, at least for CVS logs 2023-06-24 3:08 ` Dmitry Gutov 2023-06-26 19:54 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-29 21:37 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-30 5:57 ` Eli Zaretskii 1 sibling, 1 reply; 34+ messages in thread From: Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-29 21:37 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 63949, Eli Zaretskii On 2023-06-24 05:08, Dmitry Gutov wrote: > And done. Thanks. > I'm attaching a patch which should work fine, though could be a tad > risky for 29. I like the patch - I think it is more in line with what the function does/seems to do originally. > Jens (and possibly others), could you try it out? And it works, too. It covers my test cases. I also tried to run some tests with indirect buffers, and they came out as I would have expected them to come out. It would have been nice to have the opinion of the original author of commit d494833d47968fcd97ba549654a259d6fb6c2eee on this, but he doesn't seem to be active any longer. ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#63949: 30.0.50; `vc-print-log´ does not erase buffer when called from *vc-change-log* buffer, at least for CVS logs 2023-06-29 21:37 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-30 5:57 ` Eli Zaretskii 2023-07-17 19:53 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 1 reply; 34+ messages in thread From: Eli Zaretskii @ 2023-06-30 5:57 UTC (permalink / raw) To: Jens Schmidt, Nathan Moreau; +Cc: 63949, dmitry > Date: Thu, 29 Jun 2023 23:37:58 +0200 > Cc: 63949@debbugs.gnu.org, Eli Zaretskii <eliz@gnu.org> > From: Jens Schmidt <jschmidt4gnu@vodafonemail.de> > > It would have been nice to have the opinion of the original author of > commit d494833d47968fcd97ba549654a259d6fb6c2eee on this, but he doesn't > seem to be active any longer. We never CC'ed him. Let's try harder: Nathan, any comments on this discussion and the proposed changes? ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#63949: 30.0.50; `vc-print-log´ does not erase buffer when called from *vc-change-log* buffer, at least for CVS logs 2023-06-30 5:57 ` Eli Zaretskii @ 2023-07-17 19:53 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-07-18 0:55 ` Dmitry Gutov 0 siblings, 1 reply; 34+ messages in thread From: Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-07-17 19:53 UTC (permalink / raw) To: Eli Zaretskii, Nathan Moreau; +Cc: 63949, dmitry On 2023-06-30 07:57, Eli Zaretskii wrote: > We never CC'ed him. Actually, I contacted him off-list some weeks already before your mail, without getting any reply, either. To summarize: - Nathan's changes related to indirect buffers regressed some less-used scenarios in vc. - Dmitry provided a first fix for that which landed in emacs-29 and master already. - Plus a second, IMO more elegant and in Dmitry's opinion slightly more risky patch that also fixes the regression while keeping (AFAICT) Nathan's intended behavior: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=63949#86 Probably we could have that second patch at least in master? Thanks ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#63949: 30.0.50; `vc-print-log´ does not erase buffer when called from *vc-change-log* buffer, at least for CVS logs 2023-07-17 19:53 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-07-18 0:55 ` Dmitry Gutov 2023-07-18 11:08 ` Eli Zaretskii 0 siblings, 1 reply; 34+ messages in thread From: Dmitry Gutov @ 2023-07-18 0:55 UTC (permalink / raw) To: Jens Schmidt, Eli Zaretskii, Nathan Moreau; +Cc: 63949-done On 17/07/2023 22:53, Jens Schmidt wrote: > Probably we could have that second patch at least in master? Yes, of course. Now pushed to master. Thanks for the report, testing and the reminders. :-) Closing! Now if emacs-29 pretest ends up going on for several months more, we could consider backporting the second patch. Or maybe do that after, for 29.2... ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#63949: 30.0.50; `vc-print-log´ does not erase buffer when called from *vc-change-log* buffer, at least for CVS logs 2023-07-18 0:55 ` Dmitry Gutov @ 2023-07-18 11:08 ` Eli Zaretskii 0 siblings, 0 replies; 34+ messages in thread From: Eli Zaretskii @ 2023-07-18 11:08 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 63949, nathan.moreau, jschmidt4gnu > Date: Tue, 18 Jul 2023 03:55:59 +0300 > Cc: 63949-done@debbugs.gnu.org > From: Dmitry Gutov <dmitry@gutov.dev> > > Now if emacs-29 pretest ends up going on for several months more I could do without salt on my wounds, thank you. ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#63949: 30.0.50; `vc-print-log´ does not erase buffer when called from *vc-change-log* buffer, at least for CVS logs 2023-06-07 21:04 bug#63949: 30.0.50; `vc-print-log´ does not erase buffer when called from *vc-change-log* buffer, at least for CVS logs Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-08 13:12 ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-08 22:44 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-09 6:41 ` Eli Zaretskii 2023-06-08 23:09 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2 siblings, 1 reply; 34+ messages in thread From: Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-08 22:44 UTC (permalink / raw) To: 63949 [-- Attachment #1: Type: text/plain, Size: 804 bytes --] Here come five patches for this issue, based on emacs-29: 0001-Always-erase-log-buffer-before-calling-vc-print-log.patch 0002-Always-erase-log-buffer-before-calling-vc-print-log.patch 0003-Always-erase-log-buffer-before-calling-vc-print-log.patch Name should be self-explaining, patches uncritical. 0004-Fix-documentation-bug-and-remove-obsolete-fixmes.patch `vc-deduce-fileset' uses `with-current-buffer' to protect the current buffer, which has not been reflected in comments. 0005-Avoid-setting-circular-vc-parent-buffer.patch This one fixes the issue related to VC parent buffer described in the previous update. The "local" change seems to be logical (a buffer should not be the VC parent buffer of itself), but I'm not quite sure about any adverse "global" consequences. [-- Attachment #2: 0001-Always-erase-log-buffer-before-calling-vc-print-log.patch --] [-- Type: text/x-patch, Size: 1094 bytes --] From 9e3cb8038ea5bea3e2f129f0271a0cf3ec948159 Mon Sep 17 00:00:00 2001 From: Jens Schmidt <jschmidt4gnu@vodafonemail.de> Date: Thu, 8 Jun 2023 23:36:46 +0200 Subject: [PATCH 1/5] Always erase log buffer before calling `vc-*-print-log' When calling `vc-print-log' from buffer *vc-change-log* itself for backends SCCS, RCS, and CVS, the log information is inserted or appended to the *vc-change-log* buffer again without previously erasing it. * lisp/vc/vc-sccs.el (vc-sccs-print-log): Add call to `vc-setup-buffer'. (Bug#63949) --- lisp/vc/vc-sccs.el | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lisp/vc/vc-sccs.el b/lisp/vc/vc-sccs.el index 03e9d12b76f..e5430753a64 100644 --- a/lisp/vc/vc-sccs.el +++ b/lisp/vc/vc-sccs.el @@ -310,6 +310,8 @@ vc-sccs-print-log "Print commit log associated with FILES into specified BUFFER. Remaining arguments are ignored." (setq files (vc-expand-dirs files 'SCCS)) + (save-current-buffer + (vc-setup-buffer buffer)) (vc-sccs-do-command buffer 0 "prs" (mapcar #'vc-master-name files)) (when limit 'limit-unsupported)) -- 2.30.2 [-- Attachment #3: 0002-Always-erase-log-buffer-before-calling-vc-print-log.patch --] [-- Type: text/x-patch, Size: 1111 bytes --] From 4cb5eaa2166103d645fa77972b456dd6a4fad282 Mon Sep 17 00:00:00 2001 From: Jens Schmidt <jschmidt4gnu@vodafonemail.de> Date: Thu, 8 Jun 2023 23:37:45 +0200 Subject: [PATCH 2/5] Always erase log buffer before calling `vc-*-print-log' When calling `vc-print-log' from buffer *vc-change-log* itself for backends SCCS, RCS, and CVS, the log information is inserted or appended to the *vc-change-log* buffer again without previously erasing it. * lisp/vc/vc-cvs.el (vc-cvs-print-log): Add call to `vc-setup-buffer'. (Bug#63949) --- lisp/vc/vc-cvs.el | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lisp/vc/vc-cvs.el b/lisp/vc/vc-cvs.el index 6e0246ea762..cf74ac31154 100644 --- a/lisp/vc/vc-cvs.el +++ b/lisp/vc/vc-cvs.el @@ -550,6 +550,8 @@ vc-cvs-modify-change-comment (defun vc-cvs-print-log (files buffer &optional _shortlog _start-revision limit) "Print commit log associated with FILES into specified BUFFER. Remaining arguments are ignored." + (save-current-buffer + (vc-setup-buffer buffer)) ;; It's just the catenation of the individual logs. (vc-cvs-command buffer -- 2.30.2 [-- Attachment #4: 0003-Always-erase-log-buffer-before-calling-vc-print-log.patch --] [-- Type: text/x-patch, Size: 1114 bytes --] From 4eb1a951229d96026ca58f965c652ab8e8392812 Mon Sep 17 00:00:00 2001 From: Jens Schmidt <jschmidt4gnu@vodafonemail.de> Date: Thu, 8 Jun 2023 23:38:45 +0200 Subject: [PATCH 3/5] Always erase log buffer before calling `vc-*-print-log' When calling `vc-print-log' from buffer *vc-change-log* itself for backends SCCS, RCS, and CVS, the log information is inserted or appended to the *vc-change-log* buffer again without previously erasing it. * lisp/vc/vc-rcs.el (vc-rcs-print-log): Add call to `vc-setup-buffer'. (Bug#63949) --- lisp/vc/vc-rcs.el | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lisp/vc/vc-rcs.el b/lisp/vc/vc-rcs.el index c2112b76ad3..05846a7de35 100644 --- a/lisp/vc/vc-rcs.el +++ b/lisp/vc/vc-rcs.el @@ -548,6 +548,8 @@ vc-rcs-print-log Remaining arguments are ignored. If FILE is a directory the operation is applied to all registered files beneath it." + (save-current-buffer + (vc-setup-buffer buffer)) (vc-do-command (or buffer "*vc*") 0 "rlog" (mapcar #'vc-master-name (vc-expand-dirs files 'RCS))) (with-current-buffer (or buffer "*vc*") -- 2.30.2 [-- Attachment #5: 0004-Fix-documentation-bug-and-remove-obsolete-fixmes.patch --] [-- Type: text/x-patch, Size: 1513 bytes --] From 8f62cd7bbe336ba0668afb72cbbbd8c46c15376a Mon Sep 17 00:00:00 2001 From: Jens Schmidt <jschmidt4gnu@vodafonemail.de> Date: Thu, 8 Jun 2023 23:46:33 +0200 Subject: [PATCH 4/5] Fix documentation bug and remove obsolete fixmes Function `vc-deduce-fileset' preserves current buffer since commit d494833d47968fcd97ba549654a259d6fb6c2eee. Change documentation accordingly. * lisp/vc/vc.el (vc-deduce-fileset): Change doc string. (vc-deduce-fileset-1): Remove now-obsolete fixme comment. (Bug#63949) --- lisp/vc/vc.el | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el index 1144a23f317..6b99eba9d3b 100644 --- a/lisp/vc/vc.el +++ b/lisp/vc/vc.el @@ -1120,7 +1120,7 @@ vc-deduce-fileset `vc-checkout-model'. Otherwise, these 3 members may be omitted from the returned list. -BEWARE: this function may change the current buffer." +This function preserves the current buffer." (with-current-buffer (or (buffer-base-buffer) (current-buffer)) (vc-deduce-fileset-1 not-state-changing allow-unregistered @@ -1151,7 +1151,7 @@ vc-deduce-fileset-1 (or (derived-mode-p 'vc-dir-mode) (derived-mode-p 'dired-mode) (derived-mode-p 'diff-mode))))) - (progn ;FIXME: Why not `with-current-buffer'? --Stef. + (progn (set-buffer vc-parent-buffer) (vc-deduce-fileset-1 not-state-changing allow-unregistered state-model-only-files))) ((and (not buffer-file-name) -- 2.30.2 [-- Attachment #6: 0005-Avoid-setting-circular-vc-parent-buffer.patch --] [-- Type: text/x-patch, Size: 1476 bytes --] From 9f3fc16ac6eb2fbf6c73aef5e47569dba2cb5d80 Mon Sep 17 00:00:00 2001 From: Jens Schmidt <jschmidt4gnu@vodafonemail.de> Date: Fri, 9 Jun 2023 00:24:28 +0200 Subject: [PATCH 5/5] Avoid setting circular `vc-parent-buffer' Otherwise, calling e.g. `vc-print-log' from buffer *vc-change-log* sets `vc-parent-buffer' to itself. * lisp/vc/vc-dispatcher.el (vc-setup-buffer): Avoid setting circular `vc-parent-buffer'. (Bug#63949) --- lisp/vc/vc-dispatcher.el | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lisp/vc/vc-dispatcher.el b/lisp/vc/vc-dispatcher.el index fd5f655a0f6..ac4d3a78afd 100644 --- a/lisp/vc/vc-dispatcher.el +++ b/lisp/vc/vc-dispatcher.el @@ -186,10 +186,13 @@ vc-setup-buffer ;; want any of its output to appear from now on. (when oldproc (delete-process oldproc))) (kill-all-local-variables) - (setq-local vc-parent-buffer camefrom) - (setq-local vc-parent-buffer-name - (concat " from " (buffer-name camefrom))) - (setq default-directory olddir) + ;; Do not set the VC parent buffer and related variables to + ;; ourselves. (Bug#63949) + (unless (equal (current-buffer) camefrom) + (setq-local vc-parent-buffer camefrom) + (setq-local vc-parent-buffer-name + (concat " from " (buffer-name camefrom))) + (setq default-directory olddir)) (let ((buffer-undo-list t) (inhibit-read-only t)) (erase-buffer)))) -- 2.30.2 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* bug#63949: 30.0.50; `vc-print-log´ does not erase buffer when called from *vc-change-log* buffer, at least for CVS logs 2023-06-08 22:44 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-09 6:41 ` Eli Zaretskii 2023-06-09 18:44 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 1 reply; 34+ messages in thread From: Eli Zaretskii @ 2023-06-09 6:41 UTC (permalink / raw) To: Jens Schmidt; +Cc: 63949 > Date: Fri, 9 Jun 2023 00:44:39 +0200 > From: Jens Schmidt via "Bug reports for GNU Emacs, > the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org> > > Here come five patches for this issue, based on emacs-29: > > 0001-Always-erase-log-buffer-before-calling-vc-print-log.patch > 0002-Always-erase-log-buffer-before-calling-vc-print-log.patch > 0003-Always-erase-log-buffer-before-calling-vc-print-log.patch Please add comments there explaining the significance of the call to vc-setup-buffer and its effect, depending on what is the current buffer. > 0005-Avoid-setting-circular-vc-parent-buffer.patch > > This one fixes the issue related to VC parent buffer described in the > previous update. The "local" change seems to be logical (a buffer > should not be the VC parent buffer of itself), but I'm not quite sure > about any adverse "global" consequences. This code is very old, so at the very least we need to track its origin and understand why it was added, before discussing whether it should be removed. AFAICT, this code was introduced in commit 1a2f456b73bab4a711a51c8f84abf1d9f63a3b90, 30 years ago, and there's some explanation of the rationale in the log message. Thanks. ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#63949: 30.0.50; `vc-print-log´ does not erase buffer when called from *vc-change-log* buffer, at least for CVS logs 2023-06-09 6:41 ` Eli Zaretskii @ 2023-06-09 18:44 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 0 replies; 34+ messages in thread From: Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-09 18:44 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 63949 > This code is very old, so at the very least we need to track its > origin and understand why it was added, before discussing whether it > should be removed. [...] Totally agreed. Pls ignore my patches for the time being, I just hacked them together before really thinking and getting to the root cause of my issues. I'm trying to get into contact with Nathan, the author of the enhancement that caused the regression, to see whether he can provide more information than what is contained in his bug#40967. ^ permalink raw reply [flat|nested] 34+ messages in thread
* bug#63949: 30.0.50; `vc-print-log´ does not erase buffer when called from *vc-change-log* buffer, at least for CVS logs 2023-06-07 21:04 bug#63949: 30.0.50; `vc-print-log´ does not erase buffer when called from *vc-change-log* buffer, at least for CVS logs Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-08 13:12 ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-08 22:44 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-08 23:09 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2 siblings, 0 replies; 34+ messages in thread From: Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-08 23:09 UTC (permalink / raw) To: 63949 Not sure about my previous patches 0004 and 0005. IMHO that commit d494833d47968fcd97ba549654a259d6fb6c2eee which introduced all these issues looks a bit flaky. (It does not even have a NEWS entry!) I think I understand what it's doing and the intention seems noble, even if it covers only a subset of VC operations. But that `with-current-buffer' around `vc-deduce-fileset', where that function explicitly declares that it changes buffer, seems to ask for more trouble. Probably somebody with deeper VC insight should review both commit d494833d47968fcd97ba549654a259d6fb6c2eee and my patches. ^ permalink raw reply [flat|nested] 34+ messages in thread
end of thread, other threads:[~2023-07-18 11:08 UTC | newest] Thread overview: 34+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-06-07 21:04 bug#63949: 30.0.50; `vc-print-log´ does not erase buffer when called from *vc-change-log* buffer, at least for CVS logs Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-08 13:12 ` Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-08 20:21 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-08 21:33 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-08 22:10 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-09 6:09 ` Eli Zaretskii 2023-06-09 20:27 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-10 6:01 ` Eli Zaretskii 2023-06-10 15:44 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-10 15:55 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-10 17:23 ` Eli Zaretskii 2023-06-10 21:18 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-11 5:06 ` Eli Zaretskii 2023-06-16 19:33 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-17 3:15 ` Dmitry Gutov 2023-06-18 2:42 ` Dmitry Gutov 2023-06-18 5:35 ` Eli Zaretskii 2023-06-18 9:11 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-18 12:00 ` Dmitry Gutov [not found] ` <e695eaa4-2f39-1b20-1cd4-fe7fdaeb3d61@vodafonemail.de> 2023-06-20 2:41 ` Dmitry Gutov 2023-06-21 13:03 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-21 14:37 ` Dmitry Gutov 2023-06-21 15:30 ` Eli Zaretskii 2023-06-24 3:08 ` Dmitry Gutov 2023-06-26 19:54 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-29 21:37 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-30 5:57 ` Eli Zaretskii 2023-07-17 19:53 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-07-18 0:55 ` Dmitry Gutov 2023-07-18 11:08 ` Eli Zaretskii 2023-06-08 22:44 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-09 6:41 ` Eli Zaretskii 2023-06-09 18:44 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors 2023-06-08 23:09 ` Jens Schmidt via Bug reports for GNU Emacs, the Swiss army knife of text editors
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).