* bug#75730: 31.0.50; describe-symbol causing saveplace.el's defuns to evaluate
@ 2025-01-21 15:30 Jordan Ellis Coppard via Bug reports for GNU Emacs, the Swiss army knife of text editors
2025-01-25 10:42 ` Eli Zaretskii
0 siblings, 1 reply; 13+ messages in thread
From: Jordan Ellis Coppard via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2025-01-21 15:30 UTC (permalink / raw)
To: 75730
Hi there,
There's a long email from myself regarding save-place-mode clobbering
Tramp's cache here:
https://lists.gnu.org/r/tramp-devel/2025-01/msg00012.html
While I was looking at how save-place-mode works I noticed that C-h o
save-place-mode RET (or any other symbol it defines) causes the actual
minor-mode logic for save-place-mode to evaluate. The result is it
parsed the places file, it populated save-place-alist and so forth.
Visiting the file directly via C-x C-f doesn't do this and the side
effects of doing so I think constitute a bug.
A user probably didn't or doesn't want C-h o executing minor-mode BODY
functions when all they asked for is a description of the symbol. If
this must occur (as I understand it to gather a complete picture of
other symbols defined etc) it should definitely not clobber the users
state.
A practical example of this right now is save-place-mode clobbering
Tramp's cache data, and thus Tramp completions too.
To reproduce: ensure that emacs -Q will read the places file you are
already using, otherwise create one it will be able to see with this
example contents:
;;; -*- coding: utf-8; mode: lisp-data -*-
(("/podmancp:no_such_foo:/home/lorem/ipsum.txt" . 1))
Perform emacs -Q and then...
M-: (require 'tramp)
M-: (tramp-cache-print tramp-cache-data)
;; OUTPUT: "((tramp-file-name \"cache\" nil nil nil nil nil nil)
(\"tramp-version\" \"2.8.0-pre\"))"
C-h o save-place-loaded RET
;; OUTPUT: t
M-: (tramp-cache-print tramp-cache-data)
Observe that the second run of tramp-cache-print will include the bogus
podmancp entry from the places file. Aside: this would perhaps be useful
except that container `no_such_foo` might not exist anymore and M-x
tramp-cleanup-all-connections or other attempts to remove it do nothing.
A user must know that specifically C-h o has caused save-place-mode to
clobber their completions.
Closing Emacs, deleting Tramp's default cache file, and reopning it
works until maybe you're typing a minibuffer completion and happen to
expand save-place- or visit saveplace.el whereby your Tramp completions
(populated in-part from Tramp's cache data) are now clobbered; or
perhaps you have (save-place-mode) in your init.el in which case they
are always clobbered. One can manually remove them from places which
isn't friendly I'd wager; otherwise manually removing projects from
project.el's project file or Tramp's cache file etc would be the
suggested approach instead of us having commands to do so.
Besides those issues I'm confused as to why save-place's functions are
actually executing when all I asked was to describe the symbol
save-place-loaded (another aside: due to this it will ALWAYS be t).
Indeed you can use any symbol saveplace.el defines and it will execute,
reading the save-place file (default: places) and populating
save-place-alist. Since Tramp hooks into file handlers by syntax this
clobbers Tramp if any Tramp-like file names appear (like /podmancp or
/ssh etc). Carefully maintaining and keeping in-sync a list of regexp's
to ignore (said utility functions are in saveplace.el) is not a solution
here because it masks the symptom (amongst other reasons: human
forgetfulness or lazyness, this bug reports contents etc).
The documentation for define-minor-mode states that BODY is executed
when the minor mode is enabled or disabled; but it hasn't been enabled
or disabled here. Is it because describe-symbol is enabling the minor
mode to gather all symbols?
In addition to all the above this also means any user who falls into
this trap has effectively stumbled into save-place-mode sideeffects for
their entire Emacs session even though save-place-mode is nil which
causes strange behaviour I would argue is a bug.
/Jordan
In GNU Emacs 31.0.50 (build 1, aarch64-apple-darwin23.6.0, NS
appkit-2487.70 Version 14.7.2 (Build 23H311)) of 2025-01-20 built on
yote.local
Repository revision: 9093a0f824d709af15a29da528259dbca30f5c29
Windowing system distributor 'Apple', version 10.3.2487
System Description: macOS 14.7.2
Configured using:
'configure --prefix=/opt/local --disable-silent-rules --without-dbus
--without-gconf --without-libotf --without-m17n-flt --with-libgmp
--with-gnutls --with-xml2 --with-modules --with-sqlite3 --with-webp
--infodir /opt/local/share/info/emacs --disable-gc-mark-trace --with-ns
--with-lcms2 --without-harfbuzz --without-imagemagick --without-xaw3d
--with-rsvg --with-xwidgets --with-native-compilation=aot
--with-tree-sitter 'CFLAGS=-pipe -O2 -march=native -mtune=native
-Wno-attributes
-isysroot/Library/Developer/CommandLineTools/SDKs/MacOSX14.sdk -arch
arm64' 'CPPFLAGS=-I/opt/local/include
-isysroot/Library/Developer/CommandLineTools/SDKs/MacOSX14.sdk'
'LDFLAGS=-L/opt/local/lib -Wl,-headerpad_max_install_names -Wl,-no_pie
-Wl,-rpath /opt/local/lib/gcc14 -Wl,-rpath /opt/local/lib
-Wl,-syslibroot,/Library/Developer/CommandLineTools/SDKs/MacOSX14.sdk
-arch arm64''
Configured features:
ACL GIF GLIB GMP GNUTLS JPEG LCMS2 LIBXML2 MODULES NATIVE_COMP NOTIFY
KQUEUE NS PDUMPER PNG RSVG SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS
TREE_SITTER WEBP XIM XWIDGETS ZLIB
Important settings:
value of $LC_CTYPE: en_US.UTF-8
value of $LANG: en_AU.UTF-8
locale-coding-system: utf-8
Major mode: Fundamental
Minor modes in effect:
mlscroll-mode: t
vertico-mode: t
marginalia-mode: t
eat-eshell-mode: t
tabspaces-mode: t
popper-echo-mode: t
popper-mode: t
global-git-commit-mode: t
magit-auto-revert-mode: t
global-auto-revert-mode: t
corfu-popupinfo-mode: t
global-corfu-mode: t
corfu-mode: t
meow-global-mode: t
meow-mode: t
meow-normal-mode: t
delete-selection-mode: t
meow-esc-mode: t
apheleia-global-mode: t
apheleia-mode: t
which-key-mode: t
electric-pair-mode: t
elpaca-use-package-mode: t
override-global-mode: t
display-battery-mode: t
recentf-mode: t
savehist-mode: t
global-visual-wrap-prefix-mode: t
visual-wrap-prefix-mode: t
global-hl-line-mode: t
tooltip-mode: t
global-eldoc-mode: t
show-paren-mode: t
electric-indent-mode: t
mouse-wheel-mode: t
tool-bar-mode: t
tab-bar-mode: t
menu-bar-mode: t
file-name-shadow-mode: t
context-menu-mode: t
global-font-lock-mode: t
font-lock-mode: t
minibuffer-regexp-mode: t
column-number-mode: 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:
/Users/tsujp/.config/emacs/elpaca/builds/modus-themes/theme-loaddefs
hides
/Applications/MacPorts/Emacs.app/Contents/Resources/lisp/theme-loaddefs
/Users/tsujp/.config/emacs/elpaca/builds/transient/transient hides
/Applications/MacPorts/Emacs.app/Contents/Resources/lisp/transient
Features:
(shadow sort display-line-numbers whitespace mail-extr emacsbug help-fns
radix-tree tramp-cmds inspector edebug debug backtrace tree-inspector
treeview keycast dap-mode dap-tasks dap-launch lsp-docker yaml posframe
dap-overlays lsp-mode lsp-protocol xref spinner network-stream nsm
markdown-mode lv ht f s ewoc embark-consult consult magit-bookmark
bookmark embark-org embark ffap htmlize zig-mode reformatter time
mlscroll vertico marginalia esh-mode esh-var esh-cmd esh-ext esh-proc
esh-opt esh-io esh-arg esh-module esh-module-loaddefs esh-util eat
term/xterm xterm tramp trampver tramp-integration tramp-message
tramp-compat parse-time iso8601 tramp-loaddefs term disp-table ehelp
tabspaces dired-x popper-echo popper org-transclusion
org-transclusion-font-lock org-transclusion-src-lines text-clone
org-element org-persist avl-tree generator org-remark
org-remark-global-tracking just-mode magit-submodule magit-blame
magit-stash magit-reflog magit-bisect magit-push magit-pull magit-fetch
magit-clone magit-remote magit-commit magit-sequence magit-notes
magit-worktree magit-tag magit-merge magit-branch magit-reset
magit-files magit-refs magit-status magit package url-handlers
magit-repos magit-apply magit-wip magit-log magit-diff smerge-mode diff
git-commit log-edit message sendmail yank-media puny dired
dired-loaddefs rfc822 mml mml-sec epa epg rfc6068 epg-config gnus-util
mm-decode mm-bodies mm-encode mail-parse rfc2231 rfc2047 rfc2045 mm-util
ietf-drums mail-prsvr mailabbrev mail-utils gmm-utils mailheader
pcvs-util add-log magit-core magit-autorevert autorevert filenotify
magit-margin magit-transient magit-process with-editor shell server
magit-mode elp transient browse-url xdg magit-git magit-base which-func
imenu vc-git diff-mode track-changes files-x info magit-section comp
comp-cstr comp-run comp-common derived benchmark cursor-sensor crm dash
corfu-popupinfo corfu cus-start edmacro orderless compat meow meow-tutor
meow-cheatsheet meow-cheatsheet-layout meow-core meow-shims delsel
meow-esc meow-command array meow-thing meow-visual meow-keypad
meow-beacon meow-helpers meow-util color meow-keymap meow-face meow-var
avy modus-vivendi-theme modus-themes exec-path-from-shell
zig-mode-autoloads reformatter-autoloads htmlize-autoloads
embark-consult-autoloads embark-autoloads popper-autoloads
dap-mode-autoloads bui-autoloads lsp-treemacs-autoloads
treemacs-autoloads ace-window-autoloads pfuture-autoloads
hydra-autoloads cfrs-autoloads posframe-autoloads lsp-docker-autoloads
yaml-autoloads apheleia-autoloads lsp-mode-autoloads f-autoloads
s-autoloads ht-autoloads spinner-autoloads markdown-mode-autoloads
lv-autoloads keycast-autoloads tree-inspector-autoloads
treeview-autoloads inspector-autoloads diff-hl-autoloads
org-transclusion-autoloads org-remark-autoloads consult-autoloads
tabspaces-autoloads just-mode-autoloads eat-autoloads magit-autoloads
pcase magit-section-autoloads dash-autoloads transient-autoloads
with-editor-autoloads marginalia-autoloads corfu-autoloads
vertico-autoloads orderless-autoloads meow-autoloads avy-autoloads
mlscroll-autoloads modus-themes-autoloads exec-path-from-shell-autoloads
which-key kmacro org-id org-refile org-element-ast inline elec-pair
org-inlinetask org ob ob-tangle ob-ref ob-lob ob-table ob-exp org-macro
org-src sh-script smie executable ob-comint org-pcomplete pcomplete
org-list org-footnote org-faces org-entities time-date noutline outline
org-version ob-emacs-lisp ob-core ob-eval org-cycle org-table ol
org-fold org-fold-core org-keys oc org-loaddefs find-func cal-menu
calendar cal-loaddefs org-compat org-macs flymake project compile
text-property-search comint ansi-osc ansi-color ring warnings thingatpt
go-ts-mode rx treesit vc vc-dispatcher elpaca-use-package use-package
use-package-delight use-package-diminish use-package-bind-key bind-key
easy-mmode elpaca-use-package-autoloads elpaca-log elpaca-ui
elpaca-menu-elpa elpaca-menu-melpa url url-proxy url-privacy url-expand
url-methods url-history url-cookie generate-lisp-file url-domsuf
url-util url-parse auth-source eieio eieio-core password-cache json map
byte-opt url-vars mailcap elpaca-menu-org elpaca elpaca-process
elpaca-autoloads format-spec battery dbus xml subr-x cl-extra help-mode
recentf tree-widget savehist visual-wrap hl-line cl-macs gv
use-package-ensure cl-seq use-package-core bytecomp byte-compile
cus-edit pp cus-load icons wid-edit cl-loaddefs cl-lib rmc iso-transl
tooltip cconv eldoc paren electric uniquify ediff-hook vc-hooks
lisp-float-type elisp-mode mwheel term/ns-win ns-win ucs-normalize
mule-util term/common-win 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 xwidget-internal kqueue cocoa ns lcms2 multi-tty
make-network-process tty-child-frames native-compile emacs)
Memory information:
((conses 16 918954 745661) (symbols 48 69367 66) (strings 32 345052 35504)
(string-bytes 1 8102223) (vectors 16 69625) (vector-slots 8 896262
178568) (floats 8 508 223)
(intervals 56 897 448) (buffers 992 17))
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#75730: 31.0.50; describe-symbol causing saveplace.el's defuns to evaluate
2025-01-21 15:30 bug#75730: 31.0.50; describe-symbol causing saveplace.el's defuns to evaluate Jordan Ellis Coppard via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2025-01-25 10:42 ` Eli Zaretskii
2025-01-25 12:10 ` Ship Mints
2025-01-25 12:12 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
0 siblings, 2 replies; 13+ messages in thread
From: Eli Zaretskii @ 2025-01-25 10:42 UTC (permalink / raw)
To: Jordan Ellis Coppard, Stefan Monnier; +Cc: 75730
> Date: Wed, 22 Jan 2025 00:30:41 +0900
> From: Jordan Ellis Coppard via "Bug reports for GNU Emacs,
> the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
>
> While I was looking at how save-place-mode works I noticed that C-h o
> save-place-mode RET (or any other symbol it defines) causes the actual
> minor-mode logic for save-place-mode to evaluate. The result is it
> parsed the places file, it populated save-place-alist and so forth.
> Visiting the file directly via C-x C-f doesn't do this and the side
> effects of doing so I think constitute a bug.
>
> A user probably didn't or doesn't want C-h o executing minor-mode BODY
> functions when all they asked for is a description of the symbol. If
> this must occur (as I understand it to gather a complete picture of
> other symbols defined etc) it should definitely not clobber the users
> state.
I don't see the minor mode being turned on by "C-h o". What I do see
is that the :set function of save-place-abbreviate-file-names fills up
save-place-alist (and maybe does other things).
Stefan, do we have features to avoid that? Perhaps some value of
:initialize attribute could help?
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#75730: 31.0.50; describe-symbol causing saveplace.el's defuns to evaluate
2025-01-25 10:42 ` Eli Zaretskii
@ 2025-01-25 12:10 ` Ship Mints
2025-01-25 12:12 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
1 sibling, 0 replies; 13+ messages in thread
From: Ship Mints @ 2025-01-25 12:10 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 75730, Stefan Monnier, Jordan Ellis Coppard
[-- Attachment #1: Type: text/plain, Size: 1552 bytes --]
Tramp connections could also be made during that process (and to hosts not
available at the time), via abbreviate-file-name's file name handler logic?
That could delay/hang people's Emacs startup sessions or when they run
customize.
On Sat, Jan 25, 2025 at 5:43 AM Eli Zaretskii <eliz@gnu.org> wrote:
> > Date: Wed, 22 Jan 2025 00:30:41 +0900
> > From: Jordan Ellis Coppard via "Bug reports for GNU Emacs,
> > the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> >
> > While I was looking at how save-place-mode works I noticed that C-h o
> > save-place-mode RET (or any other symbol it defines) causes the actual
> > minor-mode logic for save-place-mode to evaluate. The result is it
> > parsed the places file, it populated save-place-alist and so forth.
> > Visiting the file directly via C-x C-f doesn't do this and the side
> > effects of doing so I think constitute a bug.
> >
> > A user probably didn't or doesn't want C-h o executing minor-mode BODY
> > functions when all they asked for is a description of the symbol. If
> > this must occur (as I understand it to gather a complete picture of
> > other symbols defined etc) it should definitely not clobber the users
> > state.
>
> I don't see the minor mode being turned on by "C-h o". What I do see
> is that the :set function of save-place-abbreviate-file-names fills up
> save-place-alist (and maybe does other things).
>
> Stefan, do we have features to avoid that? Perhaps some value of
> :initialize attribute could help?
>
>
>
>
[-- Attachment #2: Type: text/html, Size: 2184 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#75730: 31.0.50; describe-symbol causing saveplace.el's defuns to evaluate
2025-01-25 10:42 ` Eli Zaretskii
2025-01-25 12:10 ` Ship Mints
@ 2025-01-25 12:12 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2025-01-25 12:18 ` Ship Mints
2025-02-01 11:20 ` Eli Zaretskii
1 sibling, 2 replies; 13+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2025-01-25 12:12 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 75730, Jordan Ellis Coppard
> I don't see the minor mode being turned on by "C-h o". What I do see
> is that the :set function of save-place-abbreviate-file-names fills up
> save-place-alist (and maybe does other things).
>
> Stefan, do we have features to avoid that? Perhaps some value of
> :initialize attribute could help?
Nothing specific, no, but I think here the setter just shouldn't call
`save-place-load-alist-from-file` when `save-place-loaded` is nil.
Stefan
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#75730: 31.0.50; describe-symbol causing saveplace.el's defuns to evaluate
2025-01-25 12:12 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2025-01-25 12:18 ` Ship Mints
2025-02-01 11:20 ` Eli Zaretskii
1 sibling, 0 replies; 13+ messages in thread
From: Ship Mints @ 2025-01-25 12:18 UTC (permalink / raw)
To: Stefan Monnier; +Cc: Eli Zaretskii, 75730, Jordan Ellis Coppard
[-- Attachment #1: Type: text/plain, Size: 883 bytes --]
Aha, I spoke to soon about Tramp. Looks like someone put in (non-essential
t) so that takes care of that.
In any case, it seems would be better would be to defer the translation
work rather than do it in the defcustom.
On Sat, Jan 25, 2025 at 7:13 AM Stefan Monnier via Bug reports for GNU
Emacs, the Swiss army knife of text editors <bug-gnu-emacs@gnu.org> wrote:
> > I don't see the minor mode being turned on by "C-h o". What I do see
> > is that the :set function of save-place-abbreviate-file-names fills up
> > save-place-alist (and maybe does other things).
> >
> > Stefan, do we have features to avoid that? Perhaps some value of
> > :initialize attribute could help?
>
> Nothing specific, no, but I think here the setter just shouldn't call
> `save-place-load-alist-from-file` when `save-place-loaded` is nil.
>
>
> Stefan
>
>
>
>
>
[-- Attachment #2: Type: text/html, Size: 1578 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#75730: 31.0.50; describe-symbol causing saveplace.el's defuns to evaluate
2025-01-25 12:12 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2025-01-25 12:18 ` Ship Mints
@ 2025-02-01 11:20 ` Eli Zaretskii
2025-02-01 12:11 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2025-02-03 15:21 ` Jordan Ellis Coppard via Bug reports for GNU Emacs, the Swiss army knife of text editors
1 sibling, 2 replies; 13+ messages in thread
From: Eli Zaretskii @ 2025-02-01 11:20 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 75730, jc+o.emacs
> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Jordan Ellis Coppard <jc+o.emacs@wz.ht>, 75730@debbugs.gnu.org
> Date: Sat, 25 Jan 2025 07:12:12 -0500
>
> > I don't see the minor mode being turned on by "C-h o". What I do see
> > is that the :set function of save-place-abbreviate-file-names fills up
> > save-place-alist (and maybe does other things).
> >
> > Stefan, do we have features to avoid that? Perhaps some value of
> > :initialize attribute could help?
>
> Nothing specific, no, but I think here the setter just shouldn't call
> `save-place-load-alist-from-file` when `save-place-loaded` is nil.
Like below?
Jordan, can you see if the below patch solves your problem?
diff --git a/lisp/saveplace.el b/lisp/saveplace.el
index c2e68f3..984b1c6 100644
--- a/lisp/saveplace.el
+++ b/lisp/saveplace.el
@@ -155,7 +155,9 @@ save-place-abbreviate-file-names
:type 'boolean
:set (lambda (sym val)
(set-default sym val)
- (or save-place-loaded (save-place-load-alist-from-file))
+ (or save-place-loaded
+ (and save-place-mode
+ (save-place-load-alist-from-file)))
(let ((fun (if val #'abbreviate-file-name #'expand-file-name))
;; Don't expand file names for non-existing remote connections.
(non-essential t))
^ permalink raw reply related [flat|nested] 13+ messages in thread
* bug#75730: 31.0.50; describe-symbol causing saveplace.el's defuns to evaluate
2025-02-01 11:20 ` Eli Zaretskii
@ 2025-02-01 12:11 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2025-02-01 12:47 ` Eli Zaretskii
2025-02-03 15:21 ` Jordan Ellis Coppard via Bug reports for GNU Emacs, the Swiss army knife of text editors
1 sibling, 1 reply; 13+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2025-02-01 12:11 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 75730, jc+o.emacs
[-- Attachment #1: Type: text/plain, Size: 979 bytes --]
>> Nothing specific, no, but I think here the setter just shouldn't call
>> `save-place-load-alist-from-file` when `save-place-loaded` is nil.
>
> Like below?
> Jordan, can you see if the below patch solves your problem?
>
> diff --git a/lisp/saveplace.el b/lisp/saveplace.el
> index c2e68f3..984b1c6 100644
> --- a/lisp/saveplace.el
> +++ b/lisp/saveplace.el
> @@ -155,7 +155,9 @@ save-place-abbreviate-file-names
> :type 'boolean
> :set (lambda (sym val)
> (set-default sym val)
> - (or save-place-loaded (save-place-load-alist-from-file))
> + (or save-place-loaded
> + (and save-place-mode
> + (save-place-load-alist-from-file)))
> (let ((fun (if val #'abbreviate-file-name #'expand-file-name))
> ;; Don't expand file names for non-existing remote connections.
> (non-essential t))
Not sure. I was thinking of something like the patch below instead.
Stefan
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: saveplace.patch --]
[-- Type: text/x-diff, Size: 4599 bytes --]
diff --git a/lisp/saveplace.el b/lisp/saveplace.el
index c2e68f39730..b886b9d1891 100644
--- a/lisp/saveplace.el
+++ b/lisp/saveplace.el
@@ -101,16 +101,16 @@
:type 'boolean)
(defun save-place-load-alist-from-file ()
- (if (not save-place-loaded)
- (progn
+ (unless save-place-loaded
(setq save-place-loaded t)
+ ;; FIXME: Obey `save-place-abbreviate-file-names'?
(let ((file (expand-file-name save-place-file)))
;; make sure that the alist does not get overwritten, and then
;; load it if it exists:
- (if (file-readable-p file)
+ (when (file-readable-p file)
;; don't want to use find-file because we have been
;; adding hooks to it.
- (with-current-buffer (get-buffer-create " *Saved Places*")
+ (with-temp-buffer
(delete-region (point-min) (point-max))
;; Make sure our 'coding:' cookie in the save-place
;; file will take effect, in case the caller binds
@@ -138,10 +138,8 @@
(if (>= count save-place-limit)
(setcdr s nil)
(setq count (1+ count)))
- (setq s (cdr s))))))
-
- (kill-buffer (current-buffer))))
- nil))))
+ (setq s (cdr s))))))))
+ (save-place--normalize-alist))))
(defcustom save-place-abbreviate-file-names nil
"If non-nil, abbreviate file names before saving them.
@@ -154,27 +152,31 @@ save-place-abbreviate-file-names
either `setopt' or \\[customize-variable] to set this option."
:type 'boolean
:set (lambda (sym val)
- (set-default sym val)
- (or save-place-loaded (save-place-load-alist-from-file))
- (let ((fun (if val #'abbreviate-file-name #'expand-file-name))
- ;; Don't expand file names for non-existing remote connections.
- (non-essential t))
- (setq save-place-alist
- (cl-delete-duplicates
- (cl-loop for (k . v) in save-place-alist
- collect
- (cons (funcall fun k)
- (if (listp v)
- (cl-loop for (k1 . v1) in v
- collect
- (cons k1 (funcall fun v1)))
- v)))
- :key #'car
- :from-end t
- :test #'equal)))
- val)
+ (let ((old (if (default-boundp sym) (default-value sym))))
+ (set-default sym val)
+ (if (or (equal old val) (not save-place-loaded))
+ nil ;Nothing to do.
+ (save-place--normalize-alist))))
:version "28.1")
+(defun save-place--normalize-alist ()
+ (let ((fun (if val #'abbreviate-file-name #'expand-file-name))
+ ;; Don't expand file names for non-existing remote connections.
+ (non-essential t))
+ (setq save-place-alist
+ (cl-delete-duplicates
+ (cl-loop for (k . v) in save-place-alist
+ collect
+ (cons (funcall fun k)
+ (if (listp v)
+ (cl-loop for (k1 . v1) in v
+ collect
+ (cons k1 (funcall fun v1)))
+ v)))
+ :key #'car
+ :from-end t
+ :test #'equal))))
+
(defcustom save-place-save-skipped t
"If non-nil, remember files matching `save-place-skip-check-regexp'.
@@ -342,7 +344,7 @@ save-place-forget-unreadable-files
(defun save-place-alist-to-file ()
(let ((file (expand-file-name save-place-file))
(coding-system-for-write 'utf-8))
- (with-current-buffer (get-buffer-create " *Saved Places*")
+ (with-temp-buffer
(delete-region (point-min) (point-max))
(when save-place-forget-unreadable-files
(save-place-forget-unreadable-files))
@@ -361,8 +363,7 @@ save-place-alist-to-file
(condition-case nil
;; Don't use write-file; we don't want this buffer to visit it.
(write-region (point-min) (point-max) file)
- (file-error (message "Saving places: can't write %s" file)))
- (kill-buffer (current-buffer))))))
+ (file-error (message "Saving places: can't write %s" file)))))))
(defun save-places-to-alist ()
;; go through buffer-list, saving places to alist if save-place-mode
^ permalink raw reply related [flat|nested] 13+ messages in thread
* bug#75730: 31.0.50; describe-symbol causing saveplace.el's defuns to evaluate
2025-02-01 12:11 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2025-02-01 12:47 ` Eli Zaretskii
2025-02-02 23:30 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
0 siblings, 1 reply; 13+ messages in thread
From: Eli Zaretskii @ 2025-02-01 12:47 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 75730, jc+o.emacs
> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: jc+o.emacs@wz.ht, 75730@debbugs.gnu.org
> Date: Sat, 01 Feb 2025 07:11:23 -0500
>
> >> Nothing specific, no, but I think here the setter just shouldn't call
> >> `save-place-load-alist-from-file` when `save-place-loaded` is nil.
> >
> > Like below?
> > Jordan, can you see if the below patch solves your problem?
> >
> > diff --git a/lisp/saveplace.el b/lisp/saveplace.el
> > index c2e68f3..984b1c6 100644
> > --- a/lisp/saveplace.el
> > +++ b/lisp/saveplace.el
> > @@ -155,7 +155,9 @@ save-place-abbreviate-file-names
> > :type 'boolean
> > :set (lambda (sym val)
> > (set-default sym val)
> > - (or save-place-loaded (save-place-load-alist-from-file))
> > + (or save-place-loaded
> > + (and save-place-mode
> > + (save-place-load-alist-from-file)))
> > (let ((fun (if val #'abbreviate-file-name #'expand-file-name))
> > ;; Don't expand file names for non-existing remote connections.
> > (non-essential t))
>
> Not sure. I was thinking of something like the patch below instead.
Your patch mixes real changes with many stylistic changes, so it's
hard for me to understand the important differences between the two
approaches. Can you explain in plain English the idea behind your
proposal and how it differs from mine?
Thanks.
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#75730: 31.0.50; describe-symbol causing saveplace.el's defuns to evaluate
2025-02-01 12:47 ` Eli Zaretskii
@ 2025-02-02 23:30 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2025-02-03 12:14 ` Eli Zaretskii
0 siblings, 1 reply; 13+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2025-02-02 23:30 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 75730, jc+o.emacs
>> Not sure. I was thinking of something like the patch below instead.
>
> Your patch mixes real changes with many stylistic changes, so it's
> hard for me to understand the important differences between the two
> approaches. Can you explain in plain English the idea behind your
> proposal and how it differs from mine?
Just delay the normalization to the moment that the alist needs to
be read (whereas currently we read the alist as soon as we know how to
normalize it).
Stefan
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#75730: 31.0.50; describe-symbol causing saveplace.el's defuns to evaluate
2025-02-02 23:30 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2025-02-03 12:14 ` Eli Zaretskii
2025-02-03 21:35 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
0 siblings, 1 reply; 13+ messages in thread
From: Eli Zaretskii @ 2025-02-03 12:14 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 75730, jc+o.emacs
> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: jc+o.emacs@wz.ht, 75730@debbugs.gnu.org
> Date: Sun, 02 Feb 2025 18:30:04 -0500
>
> >> Not sure. I was thinking of something like the patch below instead.
> >
> > Your patch mixes real changes with many stylistic changes, so it's
> > hard for me to understand the important differences between the two
> > approaches. Can you explain in plain English the idea behind your
> > proposal and how it differs from mine?
>
> Just delay the normalization to the moment that the alist needs to
> be read (whereas currently we read the alist as soon as we know how to
> normalize it).
OK, then I have no objections to installing your changes, thanks.
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#75730: 31.0.50; describe-symbol causing saveplace.el's defuns to evaluate
2025-02-01 11:20 ` Eli Zaretskii
2025-02-01 12:11 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2025-02-03 15:21 ` Jordan Ellis Coppard via Bug reports for GNU Emacs, the Swiss army knife of text editors
2025-02-03 17:03 ` Eli Zaretskii
1 sibling, 1 reply; 13+ messages in thread
From: Jordan Ellis Coppard via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2025-02-03 15:21 UTC (permalink / raw)
To: Eli Zaretskii, Stefan Monnier; +Cc: 75730
On 1/2/2025 8:20 pm, Eli Zaretskii wrote:
> Jordan, can you see if the below patch solves your problem?
>
> diff --git a/lisp/saveplace.el b/lisp/saveplace.el
> index c2e68f3..984b1c6 100644
> --- a/lisp/saveplace.el
> +++ b/lisp/saveplace.el
> @@ -155,7 +155,9 @@ save-place-abbreviate-file-names
> :type 'boolean
> :set (lambda (sym val)
> (set-default sym val)
> - (or save-place-loaded (save-place-load-alist-from-file))
> + (or save-place-loaded
> + (and save-place-mode
> + (save-place-load-alist-from-file)))
> (let ((fun (if val #'abbreviate-file-name #'expand-file-name))
> ;; Don't expand file names for non-existing remote connections.
> (non-essential t))
I can try this in a bit (trying to get eglot to work with a project over
tramp currently).
I should just be able to emacs -Q and re-evaluate the defun your patch
is in (with your changes, and before doing anything that could trigger
saveplace) for my assessment to be valid right?
Or asked another way: do I have to recompile Emacs from scratch with
your patch applied for said assessment to be valid?
/Jordan
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#75730: 31.0.50; describe-symbol causing saveplace.el's defuns to evaluate
2025-02-03 15:21 ` Jordan Ellis Coppard via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2025-02-03 17:03 ` Eli Zaretskii
0 siblings, 0 replies; 13+ messages in thread
From: Eli Zaretskii @ 2025-02-03 17:03 UTC (permalink / raw)
To: Jordan Ellis Coppard; +Cc: monnier, 75730
> Date: Tue, 4 Feb 2025 00:21:50 +0900
> Cc: 75730@debbugs.gnu.org
> From: Jordan Ellis Coppard <jc+o.emacs@wz.ht>
>
> On 1/2/2025 8:20 pm, Eli Zaretskii wrote:
> > Jordan, can you see if the below patch solves your problem?
> >
> > diff --git a/lisp/saveplace.el b/lisp/saveplace.el
> > index c2e68f3..984b1c6 100644
> > --- a/lisp/saveplace.el
> > +++ b/lisp/saveplace.el
> > @@ -155,7 +155,9 @@ save-place-abbreviate-file-names
> > :type 'boolean
> > :set (lambda (sym val)
> > (set-default sym val)
> > - (or save-place-loaded (save-place-load-alist-from-file))
> > + (or save-place-loaded
> > + (and save-place-mode
> > + (save-place-load-alist-from-file)))
> > (let ((fun (if val #'abbreviate-file-name #'expand-file-name))
> > ;; Don't expand file names for non-existing remote connections.
> > (non-essential t))
>
> I can try this in a bit (trying to get eglot to work with a project over
> tramp currently).
>
> I should just be able to emacs -Q and re-evaluate the defun your patch
> is in (with your changes, and before doing anything that could trigger
> saveplace) for my assessment to be valid right?
>
> Or asked another way: do I have to recompile Emacs from scratch with
> your patch applied for said assessment to be valid?
No you need to apply the patch to saveplace.el, then re-evaluate the
patched function (or byte-compile the patched saveplace.el and restart
Emacs).
^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#75730: 31.0.50; describe-symbol causing saveplace.el's defuns to evaluate
2025-02-03 12:14 ` Eli Zaretskii
@ 2025-02-03 21:35 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
0 siblings, 0 replies; 13+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2025-02-03 21:35 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 75730-done, jc+o.emacs
> OK, then I have no objections to installing your changes, thanks.
Thanks, pushed, closing,
Stefan
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2025-02-03 21:35 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-21 15:30 bug#75730: 31.0.50; describe-symbol causing saveplace.el's defuns to evaluate Jordan Ellis Coppard via Bug reports for GNU Emacs, the Swiss army knife of text editors
2025-01-25 10:42 ` Eli Zaretskii
2025-01-25 12:10 ` Ship Mints
2025-01-25 12:12 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2025-01-25 12:18 ` Ship Mints
2025-02-01 11:20 ` Eli Zaretskii
2025-02-01 12:11 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2025-02-01 12:47 ` Eli Zaretskii
2025-02-02 23:30 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2025-02-03 12:14 ` Eli Zaretskii
2025-02-03 21:35 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2025-02-03 15:21 ` Jordan Ellis Coppard via Bug reports for GNU Emacs, the Swiss army knife of text editors
2025-02-03 17:03 ` Eli Zaretskii
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).