* 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 external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.