* Bug: buffer local variables handled wrong [9.0.5 (release_9.0.5-497-g5bc540 @ /home/hs/.emacs.d/lib/org-mode/lisp/)]
@ 2017-06-03 11:48 Stefan-W. Hahn
2017-06-03 20:46 ` Nicolas Goaziou
0 siblings, 1 reply; 9+ messages in thread
From: Stefan-W. Hahn @ 2017-06-03 11:48 UTC (permalink / raw)
To: emacs-orgmode
Good day,
Emacs : GNU Emacs 26.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.18.9)
of 2017-05-20
Package: Org mode version 9.0.5 (release_9.0.5-497-g5bc540 @ /home/hs/.emacs.d/lib/org-mode/lisp/)
I use a minor mode (moccur-edit-mode, seems a little bit old) which
initializes one variable in this way:
,----
| (defvar moccur-edit-old-content)
| (make-local-variable 'moccur-edit-old-content)
`----
This leads to following result in (buffer-local-variables):
,----
| ... (moccur-edit-file-overlays) moccur-edit-old-content (company-prefix) ...
`----
I think this is correct and happens not only by the used minor-mode.
When doing org-capture now I got a lisp error:
Debugger entered--Lisp error: listp moccur-edit-old-content
This error comes from org-clone-local-variables, because there the
prediction for local variables is always to be a list.
I traced all other code points where (buffer-local-variables) is used:
,----
| grep --color -nH -e buffer-local-var *.el
| 1. org-agenda.el:2158: (let ((save (buffer-local-variables)))
| 2. org.el:9401: (buffer-local-variables)))))
| 3. org.el:9406: (dolist (pair (buffer-local-variables from-buffer))
| 4. org-element.el:4091: (t (let ((local-variables (buffer-local-variables)))
| 5. ox.el:2646: (dolist (entry (buffer-local-variables (buffer-base-buffer)) vars)
`----
The code 2., 4. and 5. are correct, they use (consp v) or (symbolp v) to
decide what to do.
The code 1. and 3. are wrong. They both work directly with (car v) or
(cdr v).
For 1. and 3. I would like to suggest the following corrections:
modified lisp/org-agenda.el
@@ -2159,11 +2159,12 @@ org-agenda-mode
(kill-all-local-variables)
(mapc 'make-local-variable org-agenda-local-vars)
(dolist (elem save)
- (let ((var (car elem))
- (val (cdr elem)))
- (when (and val
- (member var org-agenda-local-vars))
- (set var val)))))
+ (if (consp elem)
+ (let ((var (car elem))
+ (val (cdr elem)))
+ (when (and val
+ (member var org-agenda-local-vars))
+ (set var val))))))
(setq-local org-agenda-this-buffer-is-sticky t))
(org-agenda-sticky
;; Creating a sticky Agenda buffer for the first time
modified lisp/org.el
@@ -9404,11 +9404,12 @@ org-clone-local-variables
"Clone local variables from FROM-BUFFER.
Optional argument REGEXP selects variables to clone."
(dolist (pair (buffer-local-variables from-buffer))
- (let ((name (car pair)))
- (when (and (symbolp name)
- (not (memq name org-unique-local-variables))
- (or (null regexp) (string-match regexp (symbol-name name))))
- (set (make-local-variable name) (cdr pair))))))
+ (if (consp pair)
+ (let ((name (car pair)))
+ (when (and (symbolp name)
+ (not (memq name org-unique-local-variables))
+ (or (null regexp) (string-match regexp (symbol-name name))))
+ (set (make-local-variable name) (cdr pair)))))))
;;;###autoload
(defun org-run-like-in-org-mode (cmd)
With kind regards,
Stefan
--
Stefan-W. Hahn It is easy to make things.
It is hard to make things simple.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: Bug: buffer local variables handled wrong [9.0.5 (release_9.0.5-497-g5bc540 @ /home/hs/.emacs.d/lib/org-mode/lisp/)]
2017-06-03 11:48 Bug: buffer local variables handled wrong [9.0.5 (release_9.0.5-497-g5bc540 @ /home/hs/.emacs.d/lib/org-mode/lisp/)] Stefan-W. Hahn
@ 2017-06-03 20:46 ` Nicolas Goaziou
2017-06-04 6:57 ` Stefan-W. Hahn
0 siblings, 1 reply; 9+ messages in thread
From: Nicolas Goaziou @ 2017-06-03 20:46 UTC (permalink / raw)
To: Stefan-W. Hahn; +Cc: emacs-orgmode
Hello,
"Stefan-W. Hahn" <stefan.hahn@s-hahn.de> writes:
> I use a minor mode (moccur-edit-mode, seems a little bit old) which
> initializes one variable in this way:
>
> ,----
> | (defvar moccur-edit-old-content)
> | (make-local-variable 'moccur-edit-old-content)
> `----
>
> This leads to following result in (buffer-local-variables):
>
> ,----
> | ... (moccur-edit-file-overlays) moccur-edit-old-content (company-prefix) ...
> `----
>
> I think this is correct and happens not only by the used minor-mode.
>
> When doing org-capture now I got a lisp error:
>
> Debugger entered--Lisp error: listp moccur-edit-old-content
>
> This error comes from org-clone-local-variables, because there the
> prediction for local variables is always to be a list.
Fixed. Thank you for the report and the analysis.
Regards,
--
Nicolas Goaziou
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Bug: buffer local variables handled wrong [9.0.5 (release_9.0.5-497-g5bc540 @ /home/hs/.emacs.d/lib/org-mode/lisp/)]
2017-06-03 20:46 ` Nicolas Goaziou
@ 2017-06-04 6:57 ` Stefan-W. Hahn
2017-06-04 7:19 ` Nicolas Goaziou
0 siblings, 1 reply; 9+ messages in thread
From: Stefan-W. Hahn @ 2017-06-04 6:57 UTC (permalink / raw)
To: Nicolas Goaziou; +Cc: emacs-orgmode
Mail von Nicolas Goaziou, Sat, 03 Jun 2017 at 22:46:01 +0200:
Good morning,
> >
> > This error comes from org-clone-local-variables, because there the
> > prediction for local variables is always to be a list.
>
> Fixed. Thank you for the report and the analysis.
Thanks.
Because for me pcase is magic, I tried to understand your corretion of the
code.
To understand it I expanded the pcase form with (macrostep-expand), e.g. in
org-agenda-mode:
,----
| (defun org-agenda-mode ()
| ...
| (mapc #'make-local-variable org-agenda-local-vars)
| (dolist (elem save)
| (if
| (consp elem)
| (let*
| ((x
| (car elem))
| (x
| (cdr elem)))
| (if
| (consp x)
| (let*
| ((x
| (car x))
| (x
| (cdr x)))
| (if
| (null x)
| (let
| ((val x)
| (var x))
| (when
| (and val
| (memq var org-agenda-local-vars))
| (set var val)))
| nil))
| nil))
| nil)))
| (setq-local org-agenda-this-buffer-is-sticky t))
| ...
`----
for me it looks not correct, because var and val get expanded to (cdr
(cdr elem)).
Also the second one
,----
| (defun org-clone-local-variables (from-buffer &optional regexp)
| ...
| (dolist (pair (buffer-local-variables from-buffer))
| (if
| (consp pair)
| (let*
| ((x
| (car pair))
| (x
| (cdr pair)))
| (if
| (consp x)
| (let*
| ((x
| (cdr x)))
| (if
| (null x)
| (let
| ((name x))
| (when
| (and
| (not
| (memq name org-unique-local-variables))
| (or
| (null regexp)
| (string-match-p regexp
| (symbol-name name))))
| (set
| (make-local-variable name)
| (cdr pair))))
| nil))
| nil))
| nil)))
`----
here name get expanded to (cdr (cdr pair)).
For me both cases don't look correct or do I missinterpret something?
With kind regards,
Stefan
--
Stefan-W. Hahn It is easy to make things.
It is hard to make things simple.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: Bug: buffer local variables handled wrong [9.0.5 (release_9.0.5-497-g5bc540 @ /home/hs/.emacs.d/lib/org-mode/lisp/)]
2017-06-04 6:57 ` Stefan-W. Hahn
@ 2017-06-04 7:19 ` Nicolas Goaziou
2017-06-04 8:08 ` Stefan-W. Hahn
0 siblings, 1 reply; 9+ messages in thread
From: Nicolas Goaziou @ 2017-06-04 7:19 UTC (permalink / raw)
To: Stefan-W. Hahn; +Cc: emacs-orgmode
Hello,
"Stefan-W. Hahn" <stefan.hahn@s-hahn.de> writes:
> For me both cases don't look correct or do I missinterpret something?
No, you're right. I fixed it. Thank you.
Regards,
--
Nicolas Goaziou
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Bug: buffer local variables handled wrong [9.0.5 (release_9.0.5-497-g5bc540 @ /home/hs/.emacs.d/lib/org-mode/lisp/)]
2017-06-04 7:19 ` Nicolas Goaziou
@ 2017-06-04 8:08 ` Stefan-W. Hahn
2017-06-04 8:24 ` Nicolas Goaziou
0 siblings, 1 reply; 9+ messages in thread
From: Stefan-W. Hahn @ 2017-06-04 8:08 UTC (permalink / raw)
To: Nicolas Goaziou; +Cc: emacs-orgmode
Mail von Nicolas Goaziou, Sun, 04 Jun 2017 at 09:19:09 +0200:
Good morning again,
> Hello,
>
> "Stefan-W. Hahn" <stefan.hahn@s-hahn.de> writes:
>
> > For me both cases don't look correct or do I missinterpret something?
>
> No, you're right. I fixed it. Thank you.
I looked at it, but sorry, I think this also is not right, it expands to:
,----
| (defun org-agenda-mode ()
| ...
| (mapc #'make-local-variable org-agenda-local-vars)
| (dolist (elem save)
| (if
| (consp elem)
| (let*
| ((x
| (car elem))
| (x
| (cdr elem)))
| (let
| ((val x)
| (var x))
| (when
| (and val
| (memq var org-agenda-local-vars))
| (set var val))))
| nil)))
| (setq-local org-agenda-this-buffer-is-sticky t))
| (org-agenda-sticky
`----
and the second one:
,----
| (defun org-clone-local-variables (from-buffer &optional regexp)
| ...
| (dolist (pair (buffer-local-variables from-buffer))
| (if
| (consp pair)
| (let*
| ((x
| (car pair))
| (x
| (cdr pair)))
| (if
| (consp x)
| (let*
| ((x
| (cdr x)))
| (if
| (null x)
| (let
| ((name x))
| (when
| (and
| (not
| (memq name org-unique-local-variables))
| (or
| (null regexp)
| (string-match-p regexp
| (symbol-name name))))
| (set
| (make-local-variable name)
| (cdr pair))))
| nil))
| nil))
| nil)))
`----
Both looking wrong for me. Sorry.
With kind regards,
Stefan
--
Stefan-W. Hahn It is easy to make things.
It is hard to make things simple.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: Bug: buffer local variables handled wrong [9.0.5 (release_9.0.5-497-g5bc540 @ /home/hs/.emacs.d/lib/org-mode/lisp/)]
2017-06-04 8:08 ` Stefan-W. Hahn
@ 2017-06-04 8:24 ` Nicolas Goaziou
2017-06-04 9:18 ` Stefan-W. Hahn
0 siblings, 1 reply; 9+ messages in thread
From: Nicolas Goaziou @ 2017-06-04 8:24 UTC (permalink / raw)
To: Stefan-W. Hahn; +Cc: emacs-orgmode
"Stefan-W. Hahn" <stefan.hahn@s-hahn.de> writes:
> I looked at it, but sorry, I think this also is not right, it expands to:
>
> ,----
> | (defun org-agenda-mode ()
> | ...
> | (mapc #'make-local-variable org-agenda-local-vars)
> | (dolist (elem save)
> | (if
> | (consp elem)
> | (let*
> | ((x
> | (car elem))
> | (x
> | (cdr elem)))
> | (let
> | ((val x)
> | (var x))
> | (when
> | (and val
> | (memq var org-agenda-local-vars))
> | (set var val))))
> | nil)))
> | (setq-local org-agenda-this-buffer-is-sticky t))
> | (org-agenda-sticky
> `----
>
> and the second one:
>
> ,----
> | (defun org-clone-local-variables (from-buffer &optional regexp)
> | ...
> | (dolist (pair (buffer-local-variables from-buffer))
> | (if
> | (consp pair)
> | (let*
> | ((x
> | (car pair))
> | (x
> | (cdr pair)))
> | (if
> | (consp x)
> | (let*
> | ((x
> | (cdr x)))
> | (if
> | (null x)
> | (let
> | ((name x))
> | (when
> | (and
> | (not
> | (memq name org-unique-local-variables))
> | (or
> | (null regexp)
> | (string-match-p regexp
> | (symbol-name name))))
> | (set
> | (make-local-variable name)
> | (cdr pair))))
> | nil))
> | nil))
> | nil)))
> `----
>
> Both looking wrong for me. Sorry.
What do you think is wrong?
In particular
(let (res)
(dolist (pair (buffer-local-variables))
(pcase pair
(`(,var . ,val)
(push (list 'set var val) res))))
res)
expands to
((set flyspell-word-cache-result _)
(set flyspell-word-cache-end -1)
(set undo-auto--last-boundary-cause (2 #<buffer *scratch*>))
(set syntax-ppss-last (1 0 nil nil nil nil nil 0 nil nil nil))
(set syntax-propertize--done 139)
(set flyspell-changes nil)
(set deactivate-mark nil)
(set flyspell-pre-point 139)
(set auto-revert-notify-modified-p nil)
...
)
which looks correct.
Regards,
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: Bug: buffer local variables handled wrong [9.0.5 (release_9.0.5-497-g5bc540 @ /home/hs/.emacs.d/lib/org-mode/lisp/)]
2017-06-04 8:24 ` Nicolas Goaziou
@ 2017-06-04 9:18 ` Stefan-W. Hahn
2017-06-04 9:35 ` Nicolas Goaziou
0 siblings, 1 reply; 9+ messages in thread
From: Stefan-W. Hahn @ 2017-06-04 9:18 UTC (permalink / raw)
To: Nicolas Goaziou; +Cc: emacs-orgmode
Mail von Nicolas Goaziou, Sun, 04 Jun 2017 at 10:24:57 +0200:
Hello,
> "Stefan-W. Hahn" <stefan.hahn@s-hahn.de> writes:
>
> > I looked at it, but sorry, I think this also is not right, it expands to:
> >
> > Both looking wrong for me. Sorry.
>
> What do you think is wrong?
>
> In particular
>
> (let (res)
> (dolist (pair (buffer-local-variables))
> (pcase pair
> (`(,var . ,val)
> (push (list 'set var val) res))))
> res)
>
> expands to
>
> ((set flyspell-word-cache-result _)
> (set flyspell-word-cache-end -1)
> (set undo-auto--last-boundary-cause (2 #<buffer *scratch*>))
> (set syntax-ppss-last (1 0 nil nil nil nil nil 0 nil nil nil))
> (set syntax-propertize--done 139)
> (set flyspell-changes nil)
> (set deactivate-mark nil)
> (set flyspell-pre-point 139)
> (set auto-revert-notify-modified-p nil)
> ...
> )
>
> which looks correct.
Obviously you are right, I get the ame result when evaluating it.
What I don't understand is, if I expand the pcase with (macrostep-expand) I
get the following:
,----
| (let (res)
| (dolist (pair (buffer-local-variables))
| (if
| (consp pair)
| (let*
| ((x
| (car pair))
| (x
| (cdr pair)))
| (let
| ((val x)
| (var x))
| (push
| (list 'set var val)
| res)))
| nil))
| res)
`----
And this is obviously wrong.
With kind regards,
Stefan
--
Stefan-W. Hahn It is easy to make things.
It is hard to make things simple.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: Bug: buffer local variables handled wrong [9.0.5 (release_9.0.5-497-g5bc540 @ /home/hs/.emacs.d/lib/org-mode/lisp/)]
2017-06-04 9:18 ` Stefan-W. Hahn
@ 2017-06-04 9:35 ` Nicolas Goaziou
2017-06-04 9:52 ` Stefan-W. Hahn
0 siblings, 1 reply; 9+ messages in thread
From: Nicolas Goaziou @ 2017-06-04 9:35 UTC (permalink / raw)
To: Stefan-W. Hahn; +Cc: emacs-orgmode
"Stefan-W. Hahn" <stefan.hahn@s-hahn.de> writes:
> What I don't understand is, if I expand the pcase with (macrostep-expand) I
> get the following:
>
> ,----
> | (let (res)
> | (dolist (pair (buffer-local-variables))
> | (if
> | (consp pair)
> | (let*
> | ((x
> | (car pair))
> | (x
> | (cdr pair)))
> | (let
> | ((val x)
> | (var x))
> | (push
> | (list 'set var val)
> | res)))
> | nil))
> | res)
> `----
>
> And this is obviously wrong.
This is not obviously wrong. You may be thinking that both `x' symbols
are the same, but they are not. E.g.,
(let ((s1 (make-symbol "x"))
(s2 (make-symbol "x")))
(list (list s1 s2) (eq s1 s2)))
=>
((x x) nil)
Regards,
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: Bug: buffer local variables handled wrong [9.0.5 (release_9.0.5-497-g5bc540 @ /home/hs/.emacs.d/lib/org-mode/lisp/)]
2017-06-04 9:35 ` Nicolas Goaziou
@ 2017-06-04 9:52 ` Stefan-W. Hahn
0 siblings, 0 replies; 9+ messages in thread
From: Stefan-W. Hahn @ 2017-06-04 9:52 UTC (permalink / raw)
To: Nicolas Goaziou; +Cc: emacs-orgmode
Mail von Nicolas Goaziou, Sun, 04 Jun 2017 at 11:35:34 +0200:
Hello,
> "Stefan-W. Hahn" <stefan.hahn@s-hahn.de> writes:
>
> > What I don't understand is, if I expand the pcase with (macrostep-expand) I
> > get the following:
> >
> > ,----
> > | (let (res)
> > | (dolist (pair (buffer-local-variables))
> > | (if
> > | (consp pair)
> > | (let*
> > | ((x
> > | (car pair))
> > | (x
> > | (cdr pair)))
> > | (let
> > | ((val x)
> > | (var x))
> > | (push
> > | (list 'set var val)
> > | res)))
> > | nil))
> > | res)
> > `----
> >
> > And this is obviously wrong.
>
> This is not obviously wrong. You may be thinking that both `x' symbols
> are the same, but they are not. E.g.,
>
> (let ((s1 (make-symbol "x"))
> (s2 (make-symbol "x")))
> (list (list s1 s2) (eq s1 s2)))
>
> =>
>
> ((x x) nil)
This I understand, but cannot see it in the macrostep-expanded code.
I will try learn more to understand this.
So it is obvoius now that you are a lisp magician of 8th grade. :-)
Thanks and kind regards,
Stefan
--
Stefan-W. Hahn It is easy to make things.
It is hard to make things simple.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-06-04 9:52 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-06-03 11:48 Bug: buffer local variables handled wrong [9.0.5 (release_9.0.5-497-g5bc540 @ /home/hs/.emacs.d/lib/org-mode/lisp/)] Stefan-W. Hahn
2017-06-03 20:46 ` Nicolas Goaziou
2017-06-04 6:57 ` Stefan-W. Hahn
2017-06-04 7:19 ` Nicolas Goaziou
2017-06-04 8:08 ` Stefan-W. Hahn
2017-06-04 8:24 ` Nicolas Goaziou
2017-06-04 9:18 ` Stefan-W. Hahn
2017-06-04 9:35 ` Nicolas Goaziou
2017-06-04 9:52 ` Stefan-W. Hahn
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs/org-mode.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).