* bug#26624: 26.0.50; Generalized variable `buffer-local-value' does't restore local flag
@ 2017-04-23 17:13 Philipp Stephani
2017-06-17 13:10 ` Philipp Stephani
0 siblings, 1 reply; 31+ messages in thread
From: Philipp Stephani @ 2017-04-23 17:13 UTC (permalink / raw)
To: 26624
In *scratch*, evaluate:
(defvar foo-test-var nil)
(with-temp-buffer
(list (list (buffer-local-value 'foo-test-var (current-buffer))
(local-variable-p 'foo-test-var)
(local-variable-if-set-p 'foo-test-var))
(cl-letf (((buffer-local-value 'foo-test-var (current-buffer)) 123))
(list (buffer-local-value 'foo-test-var (current-buffer))
(local-variable-p 'foo-test-var)
(local-variable-if-set-p 'foo-test-var)))
(list (buffer-local-value 'foo-test-var (current-buffer))
(local-variable-p 'foo-test-var)
(local-variable-if-set-p 'foo-test-var))))
The result is:
((nil nil nil) (123 t t) (nil t t))
But expected is:
((nil nil nil) (123 t t) (nil nil nil))
i.e. the local flag of the variable should be reset.
In GNU Emacs 26.0.50 (build 19, x86_64-apple-darwin16.5.0, NS appkit-1504.82 Version 10.12.4 (Build 16E195))
of 2017-04-23 built on p
Repository revision: a1f93c1dfa53dbe007faa09ab0c6e913e86e3ffe
Windowing system distributor 'Apple', version 10.3.1504
Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Configured using:
'configure --without-xml2 --with-modules --without-pop --with-mailutils
--enable-checking --enable-check-lisp-object-type 'CFLAGS=-ggdb3 -O0'
MAKEINFO=/usr/local/opt/texinfo/bin/makeinfo'
Configured features:
RSVG DBUS NOTIFY ACL GNUTLS ZLIB TOOLKIT_SCROLL_BARS NS MODULES
Important settings:
value of $LANG: de_DE.UTF-8
locale-coding-system: utf-8-unix
Major mode: Lisp Interaction
Minor modes in effect:
tooltip-mode: t
global-eldoc-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
auto-composition-mode: t
auto-encryption-mode: t
auto-compression-mode: t
line-number-mode: t
transient-mark-mode: t
Load-path shadows:
None found.
Features:
(shadow sort mail-extr emacsbug message subr-x puny seq byte-opt gv
bytecomp byte-compile cconv cl-loaddefs cl-lib dired dired-loaddefs
format-spec rfc822 mml easymenu mml-sec password-cache epa derived epg
epg-config gnus-util rmail rmail-loaddefs mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047
rfc2045 ietf-drums mm-util mail-prsvr mail-utils time-date tooltip eldoc
electric uniquify ediff-hook vc-hooks lisp-float-type 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
elisp-mode lisp-mode prog-mode register page menu-bar rfn-eshadow
isearch timer select scroll-bar mouse jit-lock font-lock syntax facemenu
font-core term/tty-colors frame cl-generic 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 charscript case-table epa-hook jka-cmpr-hook
help simple abbrev obarray minibuffer cl-preloaded nadvice loaddefs
button faces cus-face macroexp files text-properties overlay sha1 md5
base64 format env code-pages mule custom widget hashtable-print-readable
backquote dbusbind kqueue cocoa ns multi-tty make-network-process emacs)
Memory information:
((conses 16 203864 9733)
(symbols 48 19974 1)
(miscs 40 57 190)
(strings 32 18048 4861)
(string-bytes 1 589547)
(vectors 16 34976)
(vector-slots 8 701022 3277)
(floats 8 52 66)
(intervals 56 199 0)
(buffers 976 11))
^ permalink raw reply [flat|nested] 31+ messages in thread* bug#26624: 26.0.50; Generalized variable `buffer-local-value' does't restore local flag 2017-04-23 17:13 bug#26624: 26.0.50; Generalized variable `buffer-local-value' does't restore local flag Philipp Stephani @ 2017-06-17 13:10 ` Philipp Stephani 2017-06-17 18:48 ` npostavs 2017-06-18 4:17 ` Michael Heerdegen 0 siblings, 2 replies; 31+ messages in thread From: Philipp Stephani @ 2017-06-17 13:10 UTC (permalink / raw) To: 26624 [-- Attachment #1.1: Type: text/plain, Size: 1181 bytes --] Philipp Stephani <p.stephani2@gmail.com> schrieb am So., 23. Apr. 2017 um 19:14 Uhr: > > In *scratch*, evaluate: > > (defvar foo-test-var nil) > (with-temp-buffer > (list (list (buffer-local-value 'foo-test-var (current-buffer)) > (local-variable-p 'foo-test-var) > (local-variable-if-set-p 'foo-test-var)) > (cl-letf (((buffer-local-value 'foo-test-var (current-buffer)) > 123)) > (list (buffer-local-value 'foo-test-var (current-buffer)) > (local-variable-p 'foo-test-var) > (local-variable-if-set-p 'foo-test-var))) > (list (buffer-local-value 'foo-test-var (current-buffer)) > (local-variable-p 'foo-test-var) > (local-variable-if-set-p 'foo-test-var)))) > > The result is: > > ((nil nil nil) (123 t t) (nil t t)) > > But expected is: > > ((nil nil nil) (123 t t) (nil nil nil)) > > i.e. the local flag of the variable should be reset. > > It's possible to fix this (see attached patch), but at the expense of breaking other valid use cases such as (cl-incf (buffer-local-value ...)). Not sure whether the bug can be fixed at all without breaking other stuff. [-- Attachment #1.2: Type: text/html, Size: 1642 bytes --] [-- Attachment #2: 0001-Have-cl-letf-restore-buffer-local-status-Bug-26624.txt --] [-- Type: text/plain, Size: 3996 bytes --] From 3789b7b843ec417ae3be5d0f24409559a46bcc93 Mon Sep 17 00:00:00 2001 From: Philipp Stephani <phst@google.com> Date: Fri, 16 Jun 2017 22:55:52 +0200 Subject: [PATCH 1/2] Have `cl-letf' restore buffer-local status (Bug#26624) * lisp/emacs-lisp/gv.el (buffer-local-value): Record and restore whether the variable was buffer-local; used for `cl-letf'. * test/lisp/emacs-lisp/gv-tests.el (gv-tests--bug26624): Add unit test. --- lisp/emacs-lisp/gv.el | 15 +++++++++--- test/lisp/emacs-lisp/gv-tests.el | 51 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 3 deletions(-) create mode 100644 test/lisp/emacs-lisp/gv-tests.el diff --git a/lisp/emacs-lisp/gv.el b/lisp/emacs-lisp/gv.el index c5c12a6414..a614d675af 100644 --- a/lisp/emacs-lisp/gv.el +++ b/lisp/emacs-lisp/gv.el @@ -372,9 +372,18 @@ setf (gv-define-setter window-point (v &optional w) `(set-window-point ,w ,v)) (gv-define-setter window-start (v &optional w) `(set-window-start ,w ,v)) -(gv-define-setter buffer-local-value (val var buf) - (macroexp-let2 nil v val - `(with-current-buffer ,buf (set (make-local-variable ,var) ,v)))) +(gv-define-expander buffer-local-value + (lambda (do var buf) + (macroexp-let2* nil ((var var) (buf buf)) + (funcall do + `(if (local-variable-p ,var ,buf) + (buffer-local-value ,var ,buf) + #1='#:unbound) + (lambda (val) + `(with-current-buffer ,buf + (if (eq ,val #1#) + (kill-local-variable ,var) + (set (make-local-variable ,var) ,val)))))))) (gv-define-expander alist-get (lambda (do key alist &optional default remove) diff --git a/test/lisp/emacs-lisp/gv-tests.el b/test/lisp/emacs-lisp/gv-tests.el new file mode 100644 index 0000000000..b49c12ddf2 --- /dev/null +++ b/test/lisp/emacs-lisp/gv-tests.el @@ -0,0 +1,51 @@ +;;; gv-tests.el --- unit tests for gv.el -*- lexical-binding: t; -*- + +;; Copyright (C) 2017 Free Software Foundation, Inc. + +;; This file is part of GNU Emacs. + +;; GNU Emacs is free software: you can redistribute it and/or modify +;; it under the terms of the GNU General Public License as published by +;; the Free Software Foundation, either version 3 of the License, or +;; (at your option) any later version. + +;; GNU Emacs is distributed in the hope that it will be useful, +;; but WITHOUT ANY WARRANTY; without even the implied warranty of +;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +;; GNU General Public License for more details. + +;; You should have received a copy of the GNU General Public License +;; along with GNU Emacs. If not, see <http://www.gnu.org/licenses/>. + +;;; Commentary: + +;; Unit tests for lisp/emacs-lisp/gv.el. + +;;; Code: + +(defvar gv-tests--var 'default "Test variable.") + +(ert-deftest gv-tests--bug26624 () + "Checks that Bug#26624 is fixed." + (with-temp-buffer + (let ((var-calls 0) (buf-calls 0)) + (should (equal (buffer-local-value 'gv-tests--var (current-buffer)) + 'default)) + (should-not (local-variable-p 'gv-tests--var)) + (should-not (local-variable-if-set-p 'gv-tests--var)) + (cl-letf (((buffer-local-value + (progn (cl-incf var-calls) 'gv-tests--var) + (progn (cl-incf buf-calls) (current-buffer))) + 'unbound)) + (should (equal (buffer-local-value 'gv-tests--var (current-buffer)) + 'unbound)) + (should (local-variable-p 'gv-tests--var)) + (should (local-variable-if-set-p 'gv-tests--var))) + (should (equal (buffer-local-value 'gv-tests--var (current-buffer)) + 'default)) + (should-not (local-variable-p 'gv-tests--var)) + (should-not (local-variable-if-set-p 'gv-tests--var)) + (should (equal var-calls 1)) + (should (equal buf-calls 1))))) + +;;; gv-tests.el ends here -- 2.13.1 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* bug#26624: 26.0.50; Generalized variable `buffer-local-value' does't restore local flag 2017-06-17 13:10 ` Philipp Stephani @ 2017-06-17 18:48 ` npostavs 2017-06-18 4:17 ` Michael Heerdegen 1 sibling, 0 replies; 31+ messages in thread From: npostavs @ 2017-06-17 18:48 UTC (permalink / raw) To: Philipp Stephani; +Cc: 26624 Philipp Stephani <p.stephani2@gmail.com> writes: > It's possible to fix this (see attached patch), but at the expense of > breaking other valid use cases such as (cl-incf (buffer-local-value ...)). > Not sure whether the bug can be fixed at all without breaking other > stuff. I don't really understand the internals of the gv expander stuff, but maybe we could special case the fix to cl-letf? ^ permalink raw reply [flat|nested] 31+ messages in thread
* bug#26624: 26.0.50; Generalized variable `buffer-local-value' does't restore local flag 2017-06-17 13:10 ` Philipp Stephani 2017-06-17 18:48 ` npostavs @ 2017-06-18 4:17 ` Michael Heerdegen 2017-07-02 16:53 ` Philipp Stephani 1 sibling, 1 reply; 31+ messages in thread From: Michael Heerdegen @ 2017-06-18 4:17 UTC (permalink / raw) To: Philipp Stephani; +Cc: 26624 Philipp Stephani <p.stephani2@gmail.com> writes: > It's possible to fix this (see attached patch), but at the expense of > breaking other valid use cases such as (cl-incf (buffer-local-value > ...)). Not sure whether the bug can be fixed at all without breaking > other stuff. I have no solution, but some thoughts. The more I think about it, the more I come to the conclusion that `buffer-local-value' does not have a well defined according place. The function `buffer-local-value' is not injective: it maps different states to the same value because it can't express whether the VARIABLE's binding is buffer-local or not. But we need this information because we need to undo creating a buffer local binding in the setter when closing the `letf'. And the setter, accepting only a value for the binding, isn't surjective, because the argument doesn't hold any information of buffer-localness. Moreover, we want the setter to always create a buffer-local binding in one situation (setf), but this isn't true for the setter we need to use for `cl-letf'. We could widen the semantics of `cl-letf' to do what we want in this case, but I'm not sure if it's worth the trouble. Not if there are more cases like this. BTW, a completely different point of view would be to argument like this: the manual describes generalized variables as "one of the many places in Lisp memory where values can be stored". If variable X has no buffer local binding in some buffer B, that place in Lisp memory is the one that holds the global binding of X. That means that the place expression (buffer-local-binding X B) is equivalent to the place expression X. OTOH, If X has a buffer local binding in B, we can use X as getter expression and `setq' as setter, so the place expression (buffer-local-binding X B) is also equivalent to X. So it is always equivalent to just X and of no use. Michael. ^ permalink raw reply [flat|nested] 31+ messages in thread
* bug#26624: 26.0.50; Generalized variable `buffer-local-value' does't restore local flag 2017-06-18 4:17 ` Michael Heerdegen @ 2017-07-02 16:53 ` Philipp Stephani 2017-09-24 15:19 ` Philipp Stephani 2018-01-24 14:33 ` Michael Heerdegen 0 siblings, 2 replies; 31+ messages in thread From: Philipp Stephani @ 2017-07-02 16:53 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 26624 [-- Attachment #1: Type: text/plain, Size: 1495 bytes --] Michael Heerdegen <michael_heerdegen@web.de> schrieb am So., 18. Juni 2017 um 06:17 Uhr: > Philipp Stephani <p.stephani2@gmail.com> writes: > > > It's possible to fix this (see attached patch), but at the expense of > > breaking other valid use cases such as (cl-incf (buffer-local-value > > ...)). Not sure whether the bug can be fixed at all without breaking > > other stuff. > > I have no solution, but some thoughts. > > The more I think about it, the more I come to the conclusion that > `buffer-local-value' does not have a well defined according place. > > The function `buffer-local-value' is not injective: it maps different > states to the same value because it can't express whether the VARIABLE's > binding is buffer-local or not. But we need this information because we > need to undo creating a buffer local binding in the setter when closing > the `letf'. > > And the setter, accepting only a value for the binding, isn't > surjective, because the argument doesn't hold any information of > buffer-localness. Moreover, we want the setter to always create a > buffer-local binding in one situation (setf), but this isn't true for > the setter we need to use for `cl-letf'. > > We could widen the semantics of `cl-letf' to do what we want in this > case, but I'm not sure if it's worth the trouble. Not if there are more > cases like this. > > Thanks for this great analysis. Given this, it seems that the place definition for `buffer-local-value' should be removed from gv.el. [-- Attachment #2: Type: text/html, Size: 1996 bytes --] ^ permalink raw reply [flat|nested] 31+ messages in thread
* bug#26624: 26.0.50; Generalized variable `buffer-local-value' does't restore local flag 2017-07-02 16:53 ` Philipp Stephani @ 2017-09-24 15:19 ` Philipp Stephani 2017-09-24 15:36 ` Michael Heerdegen 2017-09-24 15:44 ` Noam Postavsky 2018-01-24 14:33 ` Michael Heerdegen 1 sibling, 2 replies; 31+ messages in thread From: Philipp Stephani @ 2017-09-24 15:19 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 26624 [-- Attachment #1.1: Type: text/plain, Size: 1657 bytes --] Philipp Stephani <p.stephani2@gmail.com> schrieb am So., 2. Juli 2017 um 18:53 Uhr: > Michael Heerdegen <michael_heerdegen@web.de> schrieb am So., 18. Juni > 2017 um 06:17 Uhr: > >> Philipp Stephani <p.stephani2@gmail.com> writes: >> >> > It's possible to fix this (see attached patch), but at the expense of >> > breaking other valid use cases such as (cl-incf (buffer-local-value >> > ...)). Not sure whether the bug can be fixed at all without breaking >> > other stuff. >> >> I have no solution, but some thoughts. >> >> The more I think about it, the more I come to the conclusion that >> `buffer-local-value' does not have a well defined according place. >> >> The function `buffer-local-value' is not injective: it maps different >> states to the same value because it can't express whether the VARIABLE's >> binding is buffer-local or not. But we need this information because we >> need to undo creating a buffer local binding in the setter when closing >> the `letf'. >> >> And the setter, accepting only a value for the binding, isn't >> surjective, because the argument doesn't hold any information of >> buffer-localness. Moreover, we want the setter to always create a >> buffer-local binding in one situation (setf), but this isn't true for >> the setter we need to use for `cl-letf'. >> >> We could widen the semantics of `cl-letf' to do what we want in this >> case, but I'm not sure if it's worth the trouble. Not if there are more >> cases like this. >> >> > Thanks for this great analysis. Given this, it seems that the place > definition for `buffer-local-value' should be removed from gv.el. > Here's a patch that does just that. [-- Attachment #1.2: Type: text/html, Size: 2456 bytes --] [-- Attachment #2: 0001-Remove-buffer-local-value-as-generalized-variable-Bug-.txt --] [-- Type: text/plain, Size: 2480 bytes --] From 4ad3058476bdc7f5fa7e60d91c240a63e1f5f73d Mon Sep 17 00:00:00 2001 From: Philipp Stephani <phst@google.com> Date: Fri, 16 Jun 2017 22:55:52 +0200 Subject: [PATCH] Remove `buffer-local-value' as generalized variable (Bug#26624) * lisp/emacs-lisp/gv.el (buffer-local-value): Remove. * lisp/files.el (file-name-non-special): Remove a stale comment. --- etc/NEWS | 7 +++++++ lisp/emacs-lisp/gv.el | 4 ---- lisp/files.el | 2 -- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/etc/NEWS b/etc/NEWS index aacdf79b57..5b87939cf7 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -51,6 +51,13 @@ sets the XTerm window title. The default is to set the window title. ** The FILENAME argument to 'file-name-base' is now mandatory and no longer defaults to 'buffer-file-name'. +** 'buffer-local-value' is no longer a generalized variable. This is +because the buffer-local value of a variable always consists of two +pieces: the actual value of the variable, and whether it is +buffer-local. But 'buffer-local-value' returns only one piece of +information, which is not enough for a generalized variable. In +particular, 'cl-letf' can't be made to work with 'buffer-local-value'. + \f * Lisp Changes in Emacs 27.1 diff --git a/lisp/emacs-lisp/gv.el b/lisp/emacs-lisp/gv.el index 892d6e9716..f504ca43b0 100644 --- a/lisp/emacs-lisp/gv.el +++ b/lisp/emacs-lisp/gv.el @@ -367,10 +367,6 @@ setf (gv-define-setter window-point (v &optional w) `(set-window-point ,w ,v)) (gv-define-setter window-start (v &optional w) `(set-window-start ,w ,v)) -(gv-define-setter buffer-local-value (val var buf) - (macroexp-let2 nil v val - `(with-current-buffer ,buf (set (make-local-variable ,var) ,v)))) - (gv-define-expander alist-get (lambda (do key alist &optional default remove testfn) (macroexp-let2 macroexp-copyable-p k key diff --git a/lisp/files.el b/lisp/files.el index fe7cb1a8a9..4bb611a3ba 100644 --- a/lisp/files.el +++ b/lisp/files.el @@ -7005,8 +7005,6 @@ file-name-non-special (when (and visit buffer-file-name) (setq buffer-file-name (concat "/:" buffer-file-name)))))) (`unquote-then-quote - ;; We can't use `cl-letf' with `(buffer-local-value)' here - ;; because it wouldn't work during bootstrapping. (let ((buffer (current-buffer))) ;; `unquote-then-quote' is only used for the ;; `verify-visited-file-modtime' action, which takes a buffer -- 2.14.1 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* bug#26624: 26.0.50; Generalized variable `buffer-local-value' does't restore local flag 2017-09-24 15:19 ` Philipp Stephani @ 2017-09-24 15:36 ` Michael Heerdegen 2017-09-24 15:44 ` Noam Postavsky 1 sibling, 0 replies; 31+ messages in thread From: Michael Heerdegen @ 2017-09-24 15:36 UTC (permalink / raw) To: Philipp Stephani; +Cc: 26624 Philipp Stephani <p.stephani2@gmail.com> writes: > Here's a patch that does just that. Thanks. Looks good to me. Michael. ^ permalink raw reply [flat|nested] 31+ messages in thread
* bug#26624: 26.0.50; Generalized variable `buffer-local-value' does't restore local flag 2017-09-24 15:19 ` Philipp Stephani 2017-09-24 15:36 ` Michael Heerdegen @ 2017-09-24 15:44 ` Noam Postavsky 2017-09-24 16:42 ` Philipp Stephani 2022-08-21 21:38 ` Lars Ingebrigtsen 1 sibling, 2 replies; 31+ messages in thread From: Noam Postavsky @ 2017-09-24 15:44 UTC (permalink / raw) To: Philipp Stephani; +Cc: Michael Heerdegen, 26624 Philipp Stephani <p.stephani2@gmail.com> writes: > * lisp/emacs-lisp/gv.el (buffer-local-value): Remove. Is it possible to just give an obsolete warning first? ^ permalink raw reply [flat|nested] 31+ messages in thread
* bug#26624: 26.0.50; Generalized variable `buffer-local-value' does't restore local flag 2017-09-24 15:44 ` Noam Postavsky @ 2017-09-24 16:42 ` Philipp Stephani 2017-09-24 17:43 ` Noam Postavsky 2022-08-21 21:38 ` Lars Ingebrigtsen 1 sibling, 1 reply; 31+ messages in thread From: Philipp Stephani @ 2017-09-24 16:42 UTC (permalink / raw) To: Noam Postavsky; +Cc: Michael Heerdegen, 26624 [-- Attachment #1: Type: text/plain, Size: 608 bytes --] Noam Postavsky <npostavs@users.sourceforge.net> schrieb am So., 24. Sep. 2017 um 17:44 Uhr: > Philipp Stephani <p.stephani2@gmail.com> writes: > > > * lisp/emacs-lisp/gv.el (buffer-local-value): Remove. > > Is it possible to just give an obsolete warning first? > I don't think it's possible in the sense of `make-obsolete' because the expander is not a named function. It would be possible to use `display-warning' within the body of the setter, but that would only annoy users. If necessary, we might add additional code to the `setf' macro to warn about this form in particular during byte compilation. [-- Attachment #2: Type: text/html, Size: 1012 bytes --] ^ permalink raw reply [flat|nested] 31+ messages in thread
* bug#26624: 26.0.50; Generalized variable `buffer-local-value' does't restore local flag 2017-09-24 16:42 ` Philipp Stephani @ 2017-09-24 17:43 ` Noam Postavsky 2017-09-29 7:50 ` Eli Zaretskii 0 siblings, 1 reply; 31+ messages in thread From: Noam Postavsky @ 2017-09-24 17:43 UTC (permalink / raw) To: Philipp Stephani; +Cc: Michael Heerdegen, 26624 Philipp Stephani <p.stephani2@gmail.com> writes: > Noam Postavsky <npostavs@users.sourceforge.net> schrieb am So., 24. > Sep. 2017 um 17:44 Uhr: > > Philipp Stephani <p.stephani2@gmail.com> writes: > > > * lisp/emacs-lisp/gv.el (buffer-local-value): Remove. > > Is it possible to just give an obsolete warning first? > > > I don't think it's possible in the sense of `make-obsolete' because > the expander is not a named function. > It would be possible to use `display-warning' within the body of the > setter, but that would only annoy users. > If necessary, we might add additional code to the `setf' macro to > warn about this form in particular during byte compilation. IMO, a compilation warning would be appropriate. ^ permalink raw reply [flat|nested] 31+ messages in thread
* bug#26624: 26.0.50; Generalized variable `buffer-local-value' does't restore local flag 2017-09-24 17:43 ` Noam Postavsky @ 2017-09-29 7:50 ` Eli Zaretskii 2017-09-29 20:55 ` Philipp Stephani 0 siblings, 1 reply; 31+ messages in thread From: Eli Zaretskii @ 2017-09-29 7:50 UTC (permalink / raw) To: Noam Postavsky; +Cc: michael_heerdegen, p.stephani2, 26624 > From: Noam Postavsky <npostavs@users.sourceforge.net> > Date: Sun, 24 Sep 2017 13:43:20 -0400 > Cc: Michael Heerdegen <michael_heerdegen@web.de>, 26624@debbugs.gnu.org > > Philipp Stephani <p.stephani2@gmail.com> writes: > > > Noam Postavsky <npostavs@users.sourceforge.net> schrieb am So., 24. > > Sep. 2017 um 17:44 Uhr: > > > > Philipp Stephani <p.stephani2@gmail.com> writes: > > > > > * lisp/emacs-lisp/gv.el (buffer-local-value): Remove. > > > > Is it possible to just give an obsolete warning first? > > > > > > I don't think it's possible in the sense of `make-obsolete' because > > the expander is not a named function. > > It would be possible to use `display-warning' within the body of the > > setter, but that would only annoy users. > > If necessary, we might add additional code to the `setf' macro to > > warn about this form in particular during byte compilation. > > IMO, a compilation warning would be appropriate. I agree. Removing some feature without due warning is not something we should do, except in very rare cases (which this one isn't). ^ permalink raw reply [flat|nested] 31+ messages in thread
* bug#26624: 26.0.50; Generalized variable `buffer-local-value' does't restore local flag 2017-09-29 7:50 ` Eli Zaretskii @ 2017-09-29 20:55 ` Philipp Stephani 2017-09-30 6:46 ` Eli Zaretskii 0 siblings, 1 reply; 31+ messages in thread From: Philipp Stephani @ 2017-09-29 20:55 UTC (permalink / raw) To: Eli Zaretskii, Noam Postavsky; +Cc: michael_heerdegen, 26624 [-- Attachment #1: Type: text/plain, Size: 1369 bytes --] Eli Zaretskii <eliz@gnu.org> schrieb am Fr., 29. Sep. 2017 um 09:51 Uhr: > > From: Noam Postavsky <npostavs@users.sourceforge.net> > > Date: Sun, 24 Sep 2017 13:43:20 -0400 > > Cc: Michael Heerdegen <michael_heerdegen@web.de>, 26624@debbugs.gnu.org > > > > Philipp Stephani <p.stephani2@gmail.com> writes: > > > > > Noam Postavsky <npostavs@users.sourceforge.net> schrieb am So., 24. > > > Sep. 2017 um 17:44 Uhr: > > > > > > Philipp Stephani <p.stephani2@gmail.com> writes: > > > > > > > * lisp/emacs-lisp/gv.el (buffer-local-value): Remove. > > > > > > Is it possible to just give an obsolete warning first? > > > > > > > > > I don't think it's possible in the sense of `make-obsolete' because > > > the expander is not a named function. > > > It would be possible to use `display-warning' within the body of the > > > setter, but that would only annoy users. > > > If necessary, we might add additional code to the `setf' macro to > > > warn about this form in particular during byte compilation. > > > > IMO, a compilation warning would be appropriate. > > I agree. Removing some feature without due warning is not something > we should do, except in very rare cases (which this one isn't). > I fully agree, but I don't know how to correctly deprecate a generalized variable. Should I add code to the byte compiler to deal with this case explicitly? [-- Attachment #2: Type: text/html, Size: 2283 bytes --] ^ permalink raw reply [flat|nested] 31+ messages in thread
* bug#26624: 26.0.50; Generalized variable `buffer-local-value' does't restore local flag 2017-09-29 20:55 ` Philipp Stephani @ 2017-09-30 6:46 ` Eli Zaretskii 2017-12-26 22:19 ` Michael Heerdegen 0 siblings, 1 reply; 31+ messages in thread From: Eli Zaretskii @ 2017-09-30 6:46 UTC (permalink / raw) To: Philipp Stephani; +Cc: michael_heerdegen, 26624, npostavs > From: Philipp Stephani <p.stephani2@gmail.com> > Date: Fri, 29 Sep 2017 20:55:41 +0000 > Cc: michael_heerdegen@web.de, 26624@debbugs.gnu.org > > I agree. Removing some feature without due warning is not something > we should do, except in very rare cases (which this one isn't). > > I fully agree, but I don't know how to correctly deprecate a generalized variable. Should I add code to the byte > compiler to deal with this case explicitly? If no other idea comes up, yes, I think so. ^ permalink raw reply [flat|nested] 31+ messages in thread
* bug#26624: 26.0.50; Generalized variable `buffer-local-value' does't restore local flag 2017-09-30 6:46 ` Eli Zaretskii @ 2017-12-26 22:19 ` Michael Heerdegen 2017-12-27 16:10 ` Eli Zaretskii 0 siblings, 1 reply; 31+ messages in thread From: Michael Heerdegen @ 2017-12-26 22:19 UTC (permalink / raw) To: Eli Zaretskii; +Cc: Philipp Stephani, 26624, npostavs Eli Zaretskii <eliz@gnu.org> writes: > > I agree. Removing some feature without due warning is not something > > we should do, except in very rare cases (which this one isn't). > > > > I fully agree, but I don't know how to correctly deprecate a > > generalized variable. Should I add code to the byte > > compiler to deal with this case explicitly? > > If no other idea comes up, yes, I think so. I guess it's not really worth the time to implement an infrastructure for gv-expander obsoletion, because we will probably make use of it only every 150 years (estimation). So it could be that nobody wants to do this for quite a while. Would it be acceptable if the gv setter of `buffer-local-value' would just print a warning (i.e., solve it "by hand")? Not perfect, admitted, but still much better than leaving this unfixed. Thanks, Michael. ^ permalink raw reply [flat|nested] 31+ messages in thread
* bug#26624: 26.0.50; Generalized variable `buffer-local-value' does't restore local flag 2017-12-26 22:19 ` Michael Heerdegen @ 2017-12-27 16:10 ` Eli Zaretskii 2017-12-27 19:54 ` Michael Heerdegen 0 siblings, 1 reply; 31+ messages in thread From: Eli Zaretskii @ 2017-12-27 16:10 UTC (permalink / raw) To: Michael Heerdegen; +Cc: p.stephani2, 26624, npostavs > From: Michael Heerdegen <michael_heerdegen@web.de> > Cc: Philipp Stephani <p.stephani2@gmail.com>, 26624@debbugs.gnu.org, npostavs@users.sourceforge.net > Date: Tue, 26 Dec 2017 23:19:33 +0100 > > I guess it's not really worth the time to implement an infrastructure > for gv-expander obsoletion, because we will probably make use of it only > every 150 years (estimation). So it could be that nobody wants to do > this for quite a while. > > Would it be acceptable if the gv setter of `buffer-local-value' would > just print a warning (i.e., solve it "by hand")? Not perfect, admitted, > but still much better than leaving this unfixed. Is it (better than leaving this unfixed)? In any case, I don't think I understand the suggestion in detail. Can you show a patch or an idea of a patch? Thanks. ^ permalink raw reply [flat|nested] 31+ messages in thread
* bug#26624: 26.0.50; Generalized variable `buffer-local-value' does't restore local flag 2017-12-27 16:10 ` Eli Zaretskii @ 2017-12-27 19:54 ` Michael Heerdegen 2017-12-27 20:27 ` Eli Zaretskii 0 siblings, 1 reply; 31+ messages in thread From: Michael Heerdegen @ 2017-12-27 19:54 UTC (permalink / raw) To: Eli Zaretskii; +Cc: p.stephani2, 26624, npostavs [-- Attachment #1: Type: text/plain, Size: 189 bytes --] Eli Zaretskii <eliz@gnu.org> writes: > In any case, I don't think I understand the suggestion in detail. Can > you show a patch or an idea of a patch? Maybe simply something like this? [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Obsolete-gv-setter-of-buffer-local-value.patch --] [-- Type: text/x-diff, Size: 803 bytes --] From 5bc85fadc201eb7d061fe585283f0f4e44f0d910 Mon Sep 17 00:00:00 2001 From: Michael Heerdegen <michael_heerdegen@web.de> Date: Wed, 27 Dec 2017 20:46:20 +0100 Subject: [PATCH] Obsolete gv-setter of `buffer-local-value' This solves Bug#26624. --- lisp/emacs-lisp/gv.el | 1 + 1 file changed, 1 insertion(+) diff --git a/lisp/emacs-lisp/gv.el b/lisp/emacs-lisp/gv.el index 777b955d90..f0aad6689d 100644 --- a/lisp/emacs-lisp/gv.el +++ b/lisp/emacs-lisp/gv.el @@ -370,6 +370,7 @@ setf (gv-define-setter window-start (v &optional w) `(set-window-start ,w ,v)) (gv-define-setter buffer-local-value (val var buf) + (byte-compile-warn "Warning: obsolete gv-setter: `buffer-local-value'") (macroexp-let2 nil v val `(with-current-buffer ,buf (set (make-local-variable ,var) ,v)))) -- 2.15.1 [-- Attachment #3: Type: text/plain, Size: 12 bytes --] Michael. ^ permalink raw reply related [flat|nested] 31+ messages in thread
* bug#26624: 26.0.50; Generalized variable `buffer-local-value' does't restore local flag 2017-12-27 19:54 ` Michael Heerdegen @ 2017-12-27 20:27 ` Eli Zaretskii 2017-12-29 14:08 ` Michael Heerdegen 0 siblings, 1 reply; 31+ messages in thread From: Eli Zaretskii @ 2017-12-27 20:27 UTC (permalink / raw) To: Michael Heerdegen; +Cc: p.stephani2, 26624, npostavs > From: Michael Heerdegen <michael_heerdegen@web.de> > Cc: p.stephani2@gmail.com, 26624@debbugs.gnu.org, npostavs@users.sourceforge.net > Date: Wed, 27 Dec 2017 20:54:28 +0100 > > diff --git a/lisp/emacs-lisp/gv.el b/lisp/emacs-lisp/gv.el > index 777b955d90..f0aad6689d 100644 > --- a/lisp/emacs-lisp/gv.el > +++ b/lisp/emacs-lisp/gv.el > @@ -370,6 +370,7 @@ setf > (gv-define-setter window-start (v &optional w) `(set-window-start ,w ,v)) > > (gv-define-setter buffer-local-value (val var buf) > + (byte-compile-warn "Warning: obsolete gv-setter: `buffer-local-value'") > (macroexp-let2 nil v val > `(with-current-buffer ,buf (set (make-local-variable ,var) ,v)))) Does this warning pop up during bootstrap, and if so, how many times? Thanks. ^ permalink raw reply [flat|nested] 31+ messages in thread
* bug#26624: 26.0.50; Generalized variable `buffer-local-value' does't restore local flag 2017-12-27 20:27 ` Eli Zaretskii @ 2017-12-29 14:08 ` Michael Heerdegen 2017-12-29 16:14 ` Eli Zaretskii 0 siblings, 1 reply; 31+ messages in thread From: Michael Heerdegen @ 2017-12-29 14:08 UTC (permalink / raw) To: Eli Zaretskii; +Cc: p.stephani2, 26624, npostavs Eli Zaretskii <eliz@gnu.org> writes: > Does this warning pop up during bootstrap, and if so, how many times? It currently would pop up five times: | ../lisp/electric.el:Warning: Warning: obsolete gv-setter: ‘buffer-local-value’ | -------------------- | ../lisp/electric.el:Warning: Warning: obsolete gv-setter: ‘buffer-local-value’ | -------------------- | electric.el:350:40:Warning: Warning: obsolete gv-setter: ‘buffer-local-value’ | -------------------- | electric.el:580:39:Warning: Warning: obsolete gv-setter: ‘buffer-local-value’ | -------------------- | elec-pair.el:608:38:Warning: Warning: obsolete gv-setter: ‘buffer-local-value’ Yes, these would need to be treated. Michael. ^ permalink raw reply [flat|nested] 31+ messages in thread
* bug#26624: 26.0.50; Generalized variable `buffer-local-value' does't restore local flag 2017-12-29 14:08 ` Michael Heerdegen @ 2017-12-29 16:14 ` Eli Zaretskii 2017-12-29 16:20 ` Philipp Stephani 0 siblings, 1 reply; 31+ messages in thread From: Eli Zaretskii @ 2017-12-29 16:14 UTC (permalink / raw) To: Michael Heerdegen; +Cc: p.stephani2, 26624, npostavs > From: Michael Heerdegen <michael_heerdegen@web.de> > Cc: p.stephani2@gmail.com, 26624@debbugs.gnu.org, npostavs@users.sourceforge.net > Date: Fri, 29 Dec 2017 15:08:41 +0100 > > Eli Zaretskii <eliz@gnu.org> writes: > > > Does this warning pop up during bootstrap, and if so, how many times? > > It currently would pop up five times: > > | ../lisp/electric.el:Warning: Warning: obsolete gv-setter: ‘buffer-local-value’ > | -------------------- > | ../lisp/electric.el:Warning: Warning: obsolete gv-setter: ‘buffer-local-value’ > | -------------------- > | electric.el:350:40:Warning: Warning: obsolete gv-setter: ‘buffer-local-value’ > | -------------------- > | electric.el:580:39:Warning: Warning: obsolete gv-setter: ‘buffer-local-value’ > | -------------------- > | elec-pair.el:608:38:Warning: Warning: obsolete gv-setter: ‘buffer-local-value’ > > > Yes, these would need to be treated. Can we treat them as part of fixing this issue? ^ permalink raw reply [flat|nested] 31+ messages in thread
* bug#26624: 26.0.50; Generalized variable `buffer-local-value' does't restore local flag 2017-12-29 16:14 ` Eli Zaretskii @ 2017-12-29 16:20 ` Philipp Stephani 0 siblings, 0 replies; 31+ messages in thread From: Philipp Stephani @ 2017-12-29 16:20 UTC (permalink / raw) To: Eli Zaretskii; +Cc: Michael Heerdegen, 26624, npostavs [-- Attachment #1: Type: text/plain, Size: 1738 bytes --] Eli Zaretskii <eliz@gnu.org> schrieb am Fr., 29. Dez. 2017 um 17:15 Uhr: > > From: Michael Heerdegen <michael_heerdegen@web.de> > > Cc: p.stephani2@gmail.com, 26624@debbugs.gnu.org, > npostavs@users.sourceforge.net > > Date: Fri, 29 Dec 2017 15:08:41 +0100 > > > > Eli Zaretskii <eliz@gnu.org> writes: > > > > > Does this warning pop up during bootstrap, and if so, how many times? > > > > It currently would pop up five times: > > > > | ../lisp/electric.el:Warning: Warning: obsolete gv-setter: > ‘buffer-local-value’ > > | -------------------- > > | ../lisp/electric.el:Warning: Warning: obsolete gv-setter: > ‘buffer-local-value’ > > | -------------------- > > | electric.el:350:40:Warning: Warning: obsolete gv-setter: > ‘buffer-local-value’ > > | -------------------- > > | electric.el:580:39:Warning: Warning: obsolete gv-setter: > ‘buffer-local-value’ > > | -------------------- > > | elec-pair.el:608:38:Warning: Warning: obsolete gv-setter: > ‘buffer-local-value’ > > > > > > Yes, these would need to be treated. > > Can we treat them as part of fixing this issue? > Yes, but I think changing them should be a separate commit because it's not straightforward. These are all modes that can be locally or globally enabled. I think typically such modes would be defined using `define-minor-mode` and `define-globalized-minor-mode`, but the electric modes are defined the other way round, i.e. the main modes are global, and then there are local modes that use `buffer-local-value` as mode variable. I'd suggest to turn this around to use `define-minor-mode` for the local modes and `define-globalized-minor-mode` for the global ones. Would that have any downsides? [-- Attachment #2: Type: text/html, Size: 2418 bytes --] ^ permalink raw reply [flat|nested] 31+ messages in thread
* bug#26624: 26.0.50; Generalized variable `buffer-local-value' does't restore local flag 2017-09-24 15:44 ` Noam Postavsky 2017-09-24 16:42 ` Philipp Stephani @ 2022-08-21 21:38 ` Lars Ingebrigtsen 2022-08-21 22:21 ` Lars Ingebrigtsen 1 sibling, 1 reply; 31+ messages in thread From: Lars Ingebrigtsen @ 2022-08-21 21:38 UTC (permalink / raw) To: Noam Postavsky; +Cc: Michael Heerdegen, Philipp Stephani, 26624 Noam Postavsky <npostavs@users.sourceforge.net> writes: > Philipp Stephani <p.stephani2@gmail.com> writes: > >> * lisp/emacs-lisp/gv.el (buffer-local-value): Remove. > > Is it possible to just give an obsolete warning first? I've added a mechanism for obsoletion, and I've now followed Michael's recommendation about buffer-local-value not being well-defined as a generalized variable, and obsoleted it in Emacs 29. I'm therefore closing this bug report. ^ permalink raw reply [flat|nested] 31+ messages in thread
* bug#26624: 26.0.50; Generalized variable `buffer-local-value' does't restore local flag 2022-08-21 21:38 ` Lars Ingebrigtsen @ 2022-08-21 22:21 ` Lars Ingebrigtsen 2022-08-22 6:53 ` Michael Heerdegen 0 siblings, 1 reply; 31+ messages in thread From: Lars Ingebrigtsen @ 2022-08-21 22:21 UTC (permalink / raw) To: Noam Postavsky; +Cc: Michael Heerdegen, Philipp Stephani, 26624 Lars Ingebrigtsen <larsi@gnus.org> writes: > I've added a mechanism for obsoletion, and I've now followed Michael's > recommendation about buffer-local-value not being well-defined as a > generalized variable, and obsoleted it in Emacs 29. It turns out that while not well-defined, it's useful here: (define-minor-mode electric-indent-local-mode "Toggle `electric-indent-mode' only in this buffer." :variable (buffer-local-value 'electric-indent-mode (current-buffer)) Rewriting this to avoid this is slightly cumbersome, it turns out. So I'm not sure it's worth obsoleting the form, and we just have to live with the somewhat odd semantics. ^ permalink raw reply [flat|nested] 31+ messages in thread
* bug#26624: 26.0.50; Generalized variable `buffer-local-value' does't restore local flag 2022-08-21 22:21 ` Lars Ingebrigtsen @ 2022-08-22 6:53 ` Michael Heerdegen 2022-08-22 10:10 ` Lars Ingebrigtsen 0 siblings, 1 reply; 31+ messages in thread From: Michael Heerdegen @ 2022-08-22 6:53 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: Philipp Stephani, 26624, Noam Postavsky Lars Ingebrigtsen <larsi@gnus.org> writes: > It turns out that while not well-defined, it's useful here: > > (define-minor-mode electric-indent-local-mode > "Toggle `electric-indent-mode' only in this buffer." > :variable (buffer-local-value 'electric-indent-mode (current-buffer)) > > Rewriting this to avoid this is slightly cumbersome, it turns out. Why not use the (GET . SET) syntax for :variable? Michael. ^ permalink raw reply [flat|nested] 31+ messages in thread
* bug#26624: 26.0.50; Generalized variable `buffer-local-value' does't restore local flag 2022-08-22 6:53 ` Michael Heerdegen @ 2022-08-22 10:10 ` Lars Ingebrigtsen 2022-08-22 21:43 ` Michael Heerdegen 0 siblings, 1 reply; 31+ messages in thread From: Lars Ingebrigtsen @ 2022-08-22 10:10 UTC (permalink / raw) To: Michael Heerdegen; +Cc: Philipp Stephani, 26624, Noam Postavsky Michael Heerdegen <michael_heerdegen@web.de> writes: >> (define-minor-mode electric-indent-local-mode >> "Toggle `electric-indent-mode' only in this buffer." >> :variable (buffer-local-value 'electric-indent-mode (current-buffer)) >> >> Rewriting this to avoid this is slightly cumbersome, it turns out. > > Why not use the (GET . SET) syntax for :variable? Let's see... would this work? :variable (electric-indent-mode . (lambda (val) (setq-local electric-indent-mode val))) ^ permalink raw reply [flat|nested] 31+ messages in thread
* bug#26624: 26.0.50; Generalized variable `buffer-local-value' does't restore local flag 2022-08-22 10:10 ` Lars Ingebrigtsen @ 2022-08-22 21:43 ` Michael Heerdegen 2022-08-23 10:28 ` Lars Ingebrigtsen 0 siblings, 1 reply; 31+ messages in thread From: Michael Heerdegen @ 2022-08-22 21:43 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: Philipp Stephani, 26624, Noam Postavsky Lars Ingebrigtsen <larsi@gnus.org> writes: > Let's see... would this work? > > :variable (electric-indent-mode . > (lambda (val) (setq-local electric-indent-mode val))) Looks good - similar specs are already used in other places. BTW: I find the definition of `electric-indent-local-mode' inelegant: the handling of the variable is split between the :variable spec and the body. The body enables the global mode and sets the global variable back to nil - quite hackish. Anyway, if this is needed in more places it cries for a `define-localized-mode'. Michael. ^ permalink raw reply [flat|nested] 31+ messages in thread
* bug#26624: 26.0.50; Generalized variable `buffer-local-value' does't restore local flag 2022-08-22 21:43 ` Michael Heerdegen @ 2022-08-23 10:28 ` Lars Ingebrigtsen 0 siblings, 0 replies; 31+ messages in thread From: Lars Ingebrigtsen @ 2022-08-23 10:28 UTC (permalink / raw) To: Michael Heerdegen; +Cc: Philipp Stephani, 26624, Noam Postavsky Michael Heerdegen <michael_heerdegen@web.de> writes: >> :variable (electric-indent-mode . >> (lambda (val) (setq-local electric-indent-mode val))) > > Looks good - similar specs are already used in other places. Now pushed to Emacs 29 (and I've made buffer-local-value obsolete again as a generalized variable). > BTW: I find the definition of `electric-indent-local-mode' inelegant: > the handling of the variable is split between the :variable spec and the > body. The body enables the global mode and sets the global variable > back to nil - quite hackish. Yes, it's not ideal at all. > Anyway, if this is needed in more places it cries for a > `define-localized-mode'. I hope it's not used a lot -- we'd rather have modes work in the opposite direction. That is, the mode is local, and then we have globalized versions of it. ^ permalink raw reply [flat|nested] 31+ messages in thread
* bug#26624: 26.0.50; Generalized variable `buffer-local-value' does't restore local flag 2017-07-02 16:53 ` Philipp Stephani 2017-09-24 15:19 ` Philipp Stephani @ 2018-01-24 14:33 ` Michael Heerdegen 2018-02-04 19:01 ` Philipp Stephani 1 sibling, 1 reply; 31+ messages in thread From: Michael Heerdegen @ 2018-01-24 14:33 UTC (permalink / raw) To: Philipp Stephani; +Cc: 26624 Philipp Stephani <p.stephani2@gmail.com> writes: > Thanks for this great analysis. Given this, it seems that the place > definition for `buffer-local-value' should be removed from gv.el. But hmm - surely there are other functions where `cl-letf' doesn't exactly restore the previous state - `alist-get', for example: #+begin_src emacs-lisp (setq my-alist '((x . 1))) (ignore (cl-letf (((alist-get 'y my-alist) 17)) my-alist)) my-alist ==> ((y) (x . 1)) #+end_src (admittedly, this is not as serious as the `buffer-local-value' case). Also, as another, different problematic case, the manual warns about `point' to be used with `cl-letf'. So, I wonder if, instead of removing the gv-setter definition for `buffer-local-value', we should instead add some more text about how `cl-letf' can have surprising effects to the manual. Michael. ^ permalink raw reply [flat|nested] 31+ messages in thread
* bug#26624: 26.0.50; Generalized variable `buffer-local-value' does't restore local flag 2018-01-24 14:33 ` Michael Heerdegen @ 2018-02-04 19:01 ` Philipp Stephani 2018-02-04 21:02 ` Michael Heerdegen 0 siblings, 1 reply; 31+ messages in thread From: Philipp Stephani @ 2018-02-04 19:01 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 26624 [-- Attachment #1: Type: text/plain, Size: 1434 bytes --] Michael Heerdegen <michael_heerdegen@web.de> schrieb am Mi., 24. Jan. 2018 um 15:33 Uhr: > Philipp Stephani <p.stephani2@gmail.com> writes: > > > Thanks for this great analysis. Given this, it seems that the place > > definition for `buffer-local-value' should be removed from gv.el. > > But hmm - surely there are other functions where `cl-letf' doesn't > exactly restore the previous state - `alist-get', for example: > > #+begin_src emacs-lisp > (setq my-alist '((x . 1))) > (ignore (cl-letf (((alist-get 'y my-alist) 17)) my-alist)) > my-alist > ==> ((y) (x . 1)) > #+end_src > > (admittedly, this is not as serious as the `buffer-local-value' case). > Also, as another, different problematic case, the manual warns about > `point' to be used with `cl-letf'. > > So, I wonder if, instead of removing the gv-setter definition for > `buffer-local-value', we should instead add some more text about how > `cl-letf' can have surprising effects to the manual. > > > I think we should spend significant efforts to avoid surprises. In this case, if it means we should remove `alist-get' as well from the forms supported by `cl-letf', then I think that's what we should do. The documentation for `cl-letf' clearly states: "On exit, either normally or because of a ‘throw’ or error, the PLACEs are set back to their original values." If it can't do that for some place form, it shouldn't be allowed. [-- Attachment #2: Type: text/html, Size: 1925 bytes --] ^ permalink raw reply [flat|nested] 31+ messages in thread
* bug#26624: 26.0.50; Generalized variable `buffer-local-value' does't restore local flag 2018-02-04 19:01 ` Philipp Stephani @ 2018-02-04 21:02 ` Michael Heerdegen 2018-02-11 17:54 ` Philipp Stephani 0 siblings, 1 reply; 31+ messages in thread From: Michael Heerdegen @ 2018-02-04 21:02 UTC (permalink / raw) To: Philipp Stephani; +Cc: 26624 Philipp Stephani <p.stephani2@gmail.com> writes: > #+begin_src emacs-lisp > (setq my-alist '((x . 1))) > (ignore (cl-letf (((alist-get 'y my-alist) 17)) my-alist)) > my-alist > ==> ((y) (x . 1)) > #+end_src > I think we should spend significant efforts to avoid surprises. In > this case, if it means we should remove `alist-get' as well from the > forms supported by `cl-letf', then I think that's what we should > do. The documentation for `cl-letf' clearly states: "On exit, either > normally or because of a ‘throw’ or error, the PLACEs are set back to > their original values." If it can't do that for some place form, it > shouldn't be allowed. But (alist-get value my-alist) doesn't change for any value (especially for y), so the alist, or the `alist-get' place expressions, aren't effectively changed. The object that represents the alist changes, however. Is that a problem or an internal implementation detail? Michael. ^ permalink raw reply [flat|nested] 31+ messages in thread
* bug#26624: 26.0.50; Generalized variable `buffer-local-value' does't restore local flag 2018-02-04 21:02 ` Michael Heerdegen @ 2018-02-11 17:54 ` Philipp Stephani 2018-02-11 20:56 ` Michael Heerdegen 0 siblings, 1 reply; 31+ messages in thread From: Philipp Stephani @ 2018-02-11 17:54 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 26624 [-- Attachment #1: Type: text/plain, Size: 2042 bytes --] Michael Heerdegen <michael_heerdegen@web.de> schrieb am So., 4. Feb. 2018 um 22:02 Uhr: > Philipp Stephani <p.stephani2@gmail.com> writes: > > > #+begin_src emacs-lisp > > (setq my-alist '((x . 1))) > > (ignore (cl-letf (((alist-get 'y my-alist) 17)) my-alist)) > > my-alist > > ==> ((y) (x . 1)) > > #+end_src > > > I think we should spend significant efforts to avoid surprises. In > > this case, if it means we should remove `alist-get' as well from the > > forms supported by `cl-letf', then I think that's what we should > > do. The documentation for `cl-letf' clearly states: "On exit, either > > normally or because of a ‘throw’ or error, the PLACEs are set back to > > their original values." If it can't do that for some place form, it > > shouldn't be allowed. > > But > > (alist-get value my-alist) > > doesn't change for any value (especially for y), so the alist, or the > `alist-get' place expressions, aren't effectively changed. The object > that represents the alist changes, however. Is that a problem or an > internal implementation detail? > > > Since it affects user-visible behavior, I wouldn't classify it as internal implementation detail. It seems to me that the approach that `cl-letf` takes is too naive: binding a generalized variable is never the same as setting it and later resetting it to the previous value, not even for simple dynamic symbols (consider unbound variables). Rather than having `(cl-letf ((place val)) body)` expand to (let ((oldval place)) (setf place val) (unwind-protect body (setf place oldval))) it should rather expand to (let ((old-state (internal-get-state place))) (setf place val) (unwind-protect body (internal-reset-state place old-state))) with suitably defined `internal-get-state` and `internal-reset-state`. For most use cases `internal-get-state` and `internal-reset-state` could just be `identity` and `setf`, but for the cases discussed here they would contain additional information. [-- Attachment #2: Type: text/html, Size: 2697 bytes --] ^ permalink raw reply [flat|nested] 31+ messages in thread
* bug#26624: 26.0.50; Generalized variable `buffer-local-value' does't restore local flag 2018-02-11 17:54 ` Philipp Stephani @ 2018-02-11 20:56 ` Michael Heerdegen 0 siblings, 0 replies; 31+ messages in thread From: Michael Heerdegen @ 2018-02-11 20:56 UTC (permalink / raw) To: Philipp Stephani; +Cc: 26624 Philipp Stephani <p.stephani2@gmail.com> writes: > it should rather expand to > > (let ((old-state (internal-get-state place))) > (setf place val) > (unwind-protect body > (internal-reset-state place old-state))) > > with suitably defined `internal-get-state` and > `internal-reset-state`. For most use cases `internal-get-state` and > `internal-reset-state` could just be `identity` and `setf `, but for > the cases discussed here they would contain additional information. Is that even well-defined? What happens when the code inside `letf' also alters this state? For example, code like #+begin_src emacs-lisp (let ((my-alist '((x 1)))) (cl-letf (((alist-get 'y my-alist) 2)) (push (cons 'y 17) my-alist)) my-alist) #+end_src or #+begin_src emacs-lisp (cl-letf (((buffer-local-value 'x my-buffer) 20)) ... (with-current-buffer my-buffer (set (make-local-variable 'x) 0)) ...) #+end_src what would "reset the state" mean? Michael. ^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2022-08-23 10:28 UTC | newest] Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-04-23 17:13 bug#26624: 26.0.50; Generalized variable `buffer-local-value' does't restore local flag Philipp Stephani 2017-06-17 13:10 ` Philipp Stephani 2017-06-17 18:48 ` npostavs 2017-06-18 4:17 ` Michael Heerdegen 2017-07-02 16:53 ` Philipp Stephani 2017-09-24 15:19 ` Philipp Stephani 2017-09-24 15:36 ` Michael Heerdegen 2017-09-24 15:44 ` Noam Postavsky 2017-09-24 16:42 ` Philipp Stephani 2017-09-24 17:43 ` Noam Postavsky 2017-09-29 7:50 ` Eli Zaretskii 2017-09-29 20:55 ` Philipp Stephani 2017-09-30 6:46 ` Eli Zaretskii 2017-12-26 22:19 ` Michael Heerdegen 2017-12-27 16:10 ` Eli Zaretskii 2017-12-27 19:54 ` Michael Heerdegen 2017-12-27 20:27 ` Eli Zaretskii 2017-12-29 14:08 ` Michael Heerdegen 2017-12-29 16:14 ` Eli Zaretskii 2017-12-29 16:20 ` Philipp Stephani 2022-08-21 21:38 ` Lars Ingebrigtsen 2022-08-21 22:21 ` Lars Ingebrigtsen 2022-08-22 6:53 ` Michael Heerdegen 2022-08-22 10:10 ` Lars Ingebrigtsen 2022-08-22 21:43 ` Michael Heerdegen 2022-08-23 10:28 ` Lars Ingebrigtsen 2018-01-24 14:33 ` Michael Heerdegen 2018-02-04 19:01 ` Philipp Stephani 2018-02-04 21:02 ` Michael Heerdegen 2018-02-11 17:54 ` Philipp Stephani 2018-02-11 20:56 ` Michael Heerdegen
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).