* bug#17630: 24.3.91; gv expander for a few places are incorrect @ 2014-05-29 11:36 Leo Liu 2014-05-29 13:18 ` Stefan Monnier 0 siblings, 1 reply; 8+ messages in thread From: Leo Liu @ 2014-05-29 11:36 UTC (permalink / raw) To: 17630 Hi Stefan, These are incorrect: (gv-define-simple-setter window-buffer set-window-buffer) (gv-define-simple-setter window-display-table set-window-display-table 'fix) (gv-define-simple-setter window-dedicated-p set-window-dedicated-p) (gv-define-simple-setter window-hscroll set-window-hscroll) (gv-define-simple-setter window-point set-window-point) (gv-define-simple-setter window-start set-window-start) The getter allows optional WINDOW arg but the setter requires WINDOW arg. For example: (setf (window-buffer) (get-buffer "abc")) expands incorrectly to (set-window-buffer (get-buffer "abc")) They should probably all be re-defined using gv-define-setter. Leo ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#17630: 24.3.91; gv expander for a few places are incorrect 2014-05-29 11:36 bug#17630: 24.3.91; gv expander for a few places are incorrect Leo Liu @ 2014-05-29 13:18 ` Stefan Monnier 2014-05-30 3:46 ` Leo Liu 0 siblings, 1 reply; 8+ messages in thread From: Stefan Monnier @ 2014-05-29 13:18 UTC (permalink / raw) To: Leo Liu; +Cc: 17630 > The getter allows optional WINDOW arg but the setter requires WINDOW > arg. For example: > (setf (window-buffer) (get-buffer "abc")) expands incorrectly to > (set-window-buffer (get-buffer "abc")) > They should probably all be re-defined using gv-define-setter. Indeed. Tho maybe a better fix is to get rid of the asymmetry between the getter and the setter. Stefan ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#17630: 24.3.91; gv expander for a few places are incorrect 2014-05-29 13:18 ` Stefan Monnier @ 2014-05-30 3:46 ` Leo Liu 2014-05-30 16:20 ` Stefan Monnier 0 siblings, 1 reply; 8+ messages in thread From: Leo Liu @ 2014-05-30 3:46 UTC (permalink / raw) To: Stefan Monnier; +Cc: 17630 On 2014-05-29 09:18 -0400, Stefan Monnier wrote: > Indeed. Tho maybe a better fix is to get rid of the asymmetry between > the getter and the setter. Yes but the situation is both odd and logical. Due to the naming of the setter the WINDOW arg naturally should appear first. For example: (set-window-parameter PARAMETER VALUE &optional WINDOW) is less clear. Making the WINDOW arg required in the getter has downsides as well. Leo ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#17630: 24.3.91; gv expander for a few places are incorrect 2014-05-30 3:46 ` Leo Liu @ 2014-05-30 16:20 ` Stefan Monnier 2014-05-31 3:04 ` Leo Liu 0 siblings, 1 reply; 8+ messages in thread From: Stefan Monnier @ 2014-05-30 16:20 UTC (permalink / raw) To: Leo Liu; +Cc: 17630 > Yes but the situation is both odd and logical. Due to the naming of the > setter the WINDOW arg naturally should appear first. For example: > (set-window-parameter PARAMETER VALUE &optional WINDOW) > is less clear. Making the WINDOW arg required in the getter has > downsides as well. Indeed. So I guess the better fix is to use gv-define-setter, indeed. BTW, IIRC these setter definitions come straight from the old CL code, so this bug has been with us for a while. Stefan ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#17630: 24.3.91; gv expander for a few places are incorrect 2014-05-30 16:20 ` Stefan Monnier @ 2014-05-31 3:04 ` Leo Liu 2014-05-31 14:32 ` Stefan Monnier 2014-05-31 14:37 ` Stefan Monnier 0 siblings, 2 replies; 8+ messages in thread From: Leo Liu @ 2014-05-31 3:04 UTC (permalink / raw) To: Stefan Monnier; +Cc: 17630 On 2014-05-30 12:20 -0400, Stefan Monnier wrote: > Indeed. So I guess the better fix is to use gv-define-setter, indeed. > BTW, IIRC these setter definitions come straight from the old CL code, > so this bug has been with us for a while. Indeed, though the return value of (setf (window-buffer ...) ...) is a regression. The fix is trivial. Do you mind installing it in emacs-24? === modified file 'lisp/emacs-lisp/gv.el' --- lisp/emacs-lisp/gv.el 2014-02-21 19:01:19 +0000 +++ lisp/emacs-lisp/gv.el 2014-05-31 02:50:44 +0000 @@ -340,13 +340,16 @@ (gv-define-simple-setter process-filter set-process-filter) (gv-define-simple-setter process-sentinel set-process-sentinel) (gv-define-simple-setter process-get process-put) -(gv-define-simple-setter window-buffer set-window-buffer) -(gv-define-simple-setter window-display-table set-window-display-table 'fix) -(gv-define-simple-setter window-dedicated-p set-window-dedicated-p) -(gv-define-simple-setter window-hscroll set-window-hscroll) (gv-define-simple-setter window-parameter set-window-parameter) -(gv-define-simple-setter window-point set-window-point) -(gv-define-simple-setter window-start set-window-start) +(gv-define-setter window-buffer (v &optional w) + `(progn (set-window-buffer ,w ,v) ,v)) +(gv-define-setter window-display-table (v &optional w) + `(progn (set-window-display-table ,w ,v) ,v)) +(gv-define-setter window-dedicated-p (v &optional w) + `(set-window-dedicated-p ,w ,v)) +(gv-define-setter window-hscroll (v &optional w) `(set-window-hscroll ,w ,v)) +(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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#17630: 24.3.91; gv expander for a few places are incorrect 2014-05-31 3:04 ` Leo Liu @ 2014-05-31 14:32 ` Stefan Monnier 2014-05-31 14:37 ` Stefan Monnier 1 sibling, 0 replies; 8+ messages in thread From: Stefan Monnier @ 2014-05-31 14:32 UTC (permalink / raw) To: Leo Liu; +Cc: 17630 > The fix is trivial. Do you mind installing it in emacs-24? I think it's OK, go ahead. Stefan ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#17630: 24.3.91; gv expander for a few places are incorrect 2014-05-31 3:04 ` Leo Liu 2014-05-31 14:32 ` Stefan Monnier @ 2014-05-31 14:37 ` Stefan Monnier 2014-05-31 15:47 ` Leo Liu 1 sibling, 1 reply; 8+ messages in thread From: Stefan Monnier @ 2014-05-31 14:37 UTC (permalink / raw) To: Leo Liu; +Cc: 17630 > +(gv-define-setter window-buffer (v &optional w) > + `(progn (set-window-buffer ,w ,v) ,v)) Actually, this is wrong. C-h f gv-define-setter says: The first arg in ARGLIST (the one that receives VAL) receives an expression which can do arbitrary things, whereas the other arguments are all guaranteed to be pure and copyable. So using ,v twice will cause havoc in things like (setf (window-buffer foo) (pop buffer)) -- Stefan ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#17630: 24.3.91; gv expander for a few places are incorrect 2014-05-31 14:37 ` Stefan Monnier @ 2014-05-31 15:47 ` Leo Liu 0 siblings, 0 replies; 8+ messages in thread From: Leo Liu @ 2014-05-31 15:47 UTC (permalink / raw) To: Stefan Monnier; +Cc: 17630-done Fixed in 24.4. On 2014-05-31 10:37 -0400, Stefan Monnier wrote: > So using ,v twice will cause havoc in things like > > (setf (window-buffer foo) (pop buffer)) The curse of multiple evaluation. I think I have corrected this. Thanks, Leo ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-05-31 15:47 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-05-29 11:36 bug#17630: 24.3.91; gv expander for a few places are incorrect Leo Liu 2014-05-29 13:18 ` Stefan Monnier 2014-05-30 3:46 ` Leo Liu 2014-05-30 16:20 ` Stefan Monnier 2014-05-31 3:04 ` Leo Liu 2014-05-31 14:32 ` Stefan Monnier 2014-05-31 14:37 ` Stefan Monnier 2014-05-31 15:47 ` Leo Liu
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).