* Inheriting some local variables from source code block editing buffers @ 2018-04-29 17:19 Göktuğ Kayaalp 2018-04-29 22:09 ` Bastien 2018-05-14 5:44 ` Göktuğ Kayaalp 0 siblings, 2 replies; 28+ messages in thread From: Göktuğ Kayaalp @ 2018-04-29 17:19 UTC (permalink / raw) To: emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 689 bytes --] Hello, when editing a source block, passing some local variables from the original buffer into the buffer in which the source code is edited might be useful. My use case is passing the value of ‘lexical-binding’ when editing Elisp source code blocks in my literate .emacs so that I don't mistakenly evaluate code in a non-lexical environment thinking it's the opposite instead. I've implemented this with the patch attached to this message, if it's deemed useful, I can revise it for inclusion upstream (just wanted to see what you think about the idea with this one). -- İ. Göktuğ Kayaalp <https://www.gkayaalp.com/> 024C 30DD 597D 142B 49AC 40EB 465C D949 B101 2427 [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: the patch --] [-- Type: text/x-diff, Size: 2363 bytes --] diff --git a/lisp/org-src.el b/lisp/org-src.el index 850525b8d..248b46462 100644 --- a/lisp/org-src.el +++ b/lisp/org-src.el @@ -196,6 +196,14 @@ For example, there is no ocaml-mode in Emacs, but the mode to use is (string "Language name") (symbol "Major mode")))) +(defcustom org-src-inherited-local-variables '() + "Local variables to be copied into source code editing buffers. +This variable contains a list of symbols, whose values in the Org +mode buffer that contains the source block are to be interited by +the source code editing buffer." + :group 'org-edit-structure + :type '(repeat symbol)) + (defcustom org-src-block-faces nil "Alist of faces to be used for source-block. Each element is a cell of the format @@ -947,9 +955,13 @@ name of the sub-editing buffer." (unless (and (memq type '(example-block src-block)) (org-src--on-datum-p element)) (user-error "Not in a source or example block")) - (let* ((lang + (let* ((org-buffer (current-buffer)) + (lang (if (eq type 'src-block) (org-element-property :language element) "example")) + (buffer-name (or edit-buffer-name + (org-src--construct-edit-buffer-name + (buffer-name) lang))) (lang-f (and (eq type 'src-block) (org-src--get-lang-mode lang))) (babel-info (and (eq type 'src-block) (org-babel-get-src-block-info 'light))) @@ -957,10 +969,7 @@ name of the sub-editing buffer." (when (and (eq type 'src-block) (not (functionp lang-f))) (error "No such language mode: %s" lang-f)) (org-src--edit-element - element - (or edit-buffer-name - (org-src--construct-edit-buffer-name (buffer-name) lang)) - lang-f + element buffer-name lang-f (and (null code) (lambda () (org-escape-code-in-region (point-min) (point-max)))) (and code (org-unescape-code-in-string code))) @@ -973,6 +982,9 @@ name of the sub-editing buffer." (let ((edit-prep-func (intern (concat "org-babel-edit-prep:" lang)))) (when (fboundp edit-prep-func) (funcall edit-prep-func babel-info)))) + (dolist (localvar org-src-inherited-local-variables) + (set (make-local-variable localvar) + (with-current-buffer org-buffer (eval localvar)))) t))) (defun org-edit-inline-src-code () ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: Inheriting some local variables from source code block editing buffers 2018-04-29 17:19 Inheriting some local variables from source code block editing buffers Göktuğ Kayaalp @ 2018-04-29 22:09 ` Bastien 2018-05-01 3:30 ` Göktuğ Kayaalp 2018-05-14 5:44 ` Göktuğ Kayaalp 1 sibling, 1 reply; 28+ messages in thread From: Bastien @ 2018-04-29 22:09 UTC (permalink / raw) To: Göktuğ Kayaalp; +Cc: emacs-orgmode Hi Göktuğ, thanks for your patch. Kayaalp <self@gkayaalp.com> writes: > when editing a source block, passing some local variables from the > original buffer into the buffer in which the source code is edited might > be useful. My use case is passing the value of ‘lexical-binding’ when > editing Elisp source code blocks in my literate .emacs so that I don't > mistakenly evaluate code in a non-lexical environment thinking it's the > opposite instead. Yes, I see how this can be useful in this case. Do you have other examples on local variables that would be useful to pass on when editing code in Org Src buffers? > I've implemented this with the patch attached to this message, if it's > deemed useful, I can revise it for inclusion upstream (just wanted to > see what you think about the idea with this one). Note that in the particular case of lexical-binding and emacs-lisp blocks, there is the :lexical block parameter -- of course, this is for execution, not edition, but you can use it as a clue on whether the block supposes lexical binding or not. In general, instead of inheriting values from the Org's buffer, I'd allow users to set the values independantly -- for example, the cdr of elements in `org-babel-load-languages', instead of being `t' all the time (because nil makes no sense to me), could contain an alist of variables and their local values when editing and... executing. This is: *if* we find cases that seem generic enough. What do you think? -- Bastien ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Inheriting some local variables from source code block editing buffers 2018-04-29 22:09 ` Bastien @ 2018-05-01 3:30 ` Göktuğ Kayaalp 2018-05-01 8:43 ` Nicolas Goaziou 0 siblings, 1 reply; 28+ messages in thread From: Göktuğ Kayaalp @ 2018-05-01 3:30 UTC (permalink / raw) To: Bastien; +Cc: emacs-orgmode On 2018-04-30 00:09 +02, Bastien <bzg@gnu.org> wrote: > Hi Göktuğ, > > thanks for your patch. You're welcome! > Kayaalp <self@gkayaalp.com> writes: > Do you have other examples on local variables that would be useful to > pass on when editing code in Org Src buffers? Currently I only pass on lexical-binding and truncate-lines, and I do not have another concrete example in my configuration (I do not use source blocks for anything other than shell and elisp currently). But I have listed some hypothetical use cases further down. > In general, instead of inheriting values from the Org's buffer, I'd > allow users to set the values independantly -- for example, the cdr > of elements in `org-babel-load-languages', instead of being `t' all > the time (because nil makes no sense to me), could contain an alist > of variables and their local values when editing and... executing. This might be a better way indeed. But this setting is then global, i.e. one can't have the file A.org have lexical-binding on, but B.org off (which might be necessary for say an older org file that depends on lexical binding). > This is: *if* we find cases that seem generic enough. > > What do you think? One case I can think of is to set variables like fill-column when editing inline LaTeX, HTML, &c blocks, and also, those like c-file-style, where say when writing a paper the author wants to use k&r style, but when writing a literate source prefers gnu style. Maybe a good way to achieve this would be to have the way you suggest to set defaults for Babel, but allow to define such bindings also in individual org mode files, either via the local variables or with a specific #+keyword like: #+edit_special_bindings: lexical-binding:t # or #+edit_special_bindings: c-file-style:gnu fill-column:80 which is better IMO because there is no need to declare separately which variables to copy, and is more granular. Also, in this case, a shortcut syntax for inheriting the buffer local value of a variable can be useful: ==== x.org === # -*- fill-column: 65 -*- #+edit_special_bindings: c-file-style:gnu fill-column* This can be useful when one needs/wants to keep a consistent style in a given file. -- İ. Göktuğ Kayaalp <https://www.gkayaalp.com/> 024C 30DD 597D 142B 49AC 40EB 465C D949 B101 2427 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Inheriting some local variables from source code block editing buffers 2018-05-01 3:30 ` Göktuğ Kayaalp @ 2018-05-01 8:43 ` Nicolas Goaziou 2018-05-01 8:45 ` Nicolas Goaziou ` (2 more replies) 0 siblings, 3 replies; 28+ messages in thread From: Nicolas Goaziou @ 2018-05-01 8:43 UTC (permalink / raw) To: Göktuğ Kayaalp; +Cc: Bastien, emacs-orgmode Hello, Göktuğ Kayaalp <self@gkayaalp.com> writes: > One case I can think of is to set variables like fill-column when > editing inline LaTeX, HTML, &c blocks, and also, those like > c-file-style, where say when writing a paper the author wants to use k&r > style, but when writing a literate source prefers gnu style. > > Maybe a good way to achieve this would be to have the way you suggest to > set defaults for Babel, but allow to define such bindings also in > individual org mode files, either via the local variables or with a > specific #+keyword like: > > #+edit_special_bindings: lexical-binding:t > # or > #+edit_special_bindings: c-file-style:gnu fill-column:80 > > which is better IMO because there is no need to declare separately which > variables to copy, and is more granular. Also, in this case, a shortcut > syntax for inheriting the buffer local value of a variable can be > useful: > > ==== x.org === > # -*- fill-column: 65 -*- > #+edit_special_bindings: c-file-style:gnu fill-column* > > This can be useful when one needs/wants to keep a consistent style in a > given file. I think this machinery is not necessary. First add a call to `hack-local-variables-apply' somewhere in `org-src--edit-element'. Then, just use regular file-local variables ,e.g., #+begin_src emacs-lisp (foo) ;; Local Variables: ;; fill-column: 99 ;; End: #+end_src Regards, -- Nicolas Goaziou ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Inheriting some local variables from source code block editing buffers 2018-05-01 8:43 ` Nicolas Goaziou @ 2018-05-01 8:45 ` Nicolas Goaziou 2018-05-01 11:41 ` Göktuğ Kayaalp 2018-05-01 11:45 ` Göktuğ Kayaalp 2 siblings, 0 replies; 28+ messages in thread From: Nicolas Goaziou @ 2018-05-01 8:45 UTC (permalink / raw) To: Göktuğ Kayaalp; +Cc: Bastien, emacs-orgmode Nicolas Goaziou <mail@nicolasgoaziou.fr> writes: > First add a call to `hack-local-variables-apply' somewhere in > `org-src--edit-element'. I meant `hack-local-variables'. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Inheriting some local variables from source code block editing buffers 2018-05-01 8:43 ` Nicolas Goaziou 2018-05-01 8:45 ` Nicolas Goaziou @ 2018-05-01 11:41 ` Göktuğ Kayaalp 2018-05-01 11:55 ` Nicolas Goaziou 2018-05-01 11:45 ` Göktuğ Kayaalp 2 siblings, 1 reply; 28+ messages in thread From: Göktuğ Kayaalp @ 2018-05-01 11:41 UTC (permalink / raw) To: Nicolas Goaziou; +Cc: Bastien, emacs-orgmode On 2018-05-01 10:43 +02, Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote: > I think this machinery is not necessary. > > First add a call to `hack-local-variables-apply' somewhere in > `org-src--edit-element'. > > Then, just use regular file-local variables ,e.g., > > #+begin_src emacs-lisp > (foo) > ;; Local Variables: > ;; fill-column: 99 > ;; End: > #+end_src But in my case (which is quite common I think) this would require adding those local variables sections to each code block. My Emacs config alone has upwards of 170 code blocks, which means same three lines repeated 170 times adding up to extra 510 lines, just for setting one variable consistently. -- İ. Göktuğ Kayaalp <https://www.gkayaalp.com/> 024C 30DD 597D 142B 49AC 40EB 465C D949 B101 2427 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Inheriting some local variables from source code block editing buffers 2018-05-01 11:41 ` Göktuğ Kayaalp @ 2018-05-01 11:55 ` Nicolas Goaziou [not found] ` <ygmvac7lcl1.fsf@gkayaalp.com> 2018-05-01 16:37 ` Aaron Ecay 0 siblings, 2 replies; 28+ messages in thread From: Nicolas Goaziou @ 2018-05-01 11:55 UTC (permalink / raw) To: Göktuğ Kayaalp; +Cc: Bastien, emacs-orgmode Göktuğ Kayaalp <self@gkayaalp.com> writes: > But in my case (which is quite common I think) this would require adding > those local variables sections to each code block. My Emacs config > alone has upwards of 170 code blocks, which means same three lines > repeated 170 times adding up to extra 510 lines, just for setting one > variable consistently. No, because you can use Noweb syntax wherever you need these local variables set: <<local-variables>> ^ permalink raw reply [flat|nested] 28+ messages in thread
[parent not found: <ygmvac7lcl1.fsf@gkayaalp.com>]
* Re: Inheriting some local variables from source code block editing buffers [not found] ` <ygmvac7lcl1.fsf@gkayaalp.com> @ 2018-05-01 14:00 ` Nicolas Goaziou 0 siblings, 0 replies; 28+ messages in thread From: Nicolas Goaziou @ 2018-05-01 14:00 UTC (permalink / raw) To: Göktuğ Kayaalp; +Cc: emacs-orgmode Göktuğ Kayaalp <self@gkayaalp.com> writes: > On 2018-05-01 13:55 +02, Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote: > >> No, because you can use Noweb syntax wherever you need these local >> variables set: >> >> <<local-variables>> > > This seems to only expand when tangling, When tangling, exporting and evaluating. > not while editing via C-c '. Correct. > What I propose is about setting up the editing buffer (e.g. making > sure in the org-edit-special lexical-binding is the same as in the > org-mode buffer so that the results from live evaluation and from when > tangling and loading the file agree). Lexical binding is not a good example, because there's already a header argument to control this: #+begin_src emacs-lisp :lexical t ... #+end_src and you can set header arguments document-wise, or globally. Other file local variables you could set in an Org buffer do not usually make sense in programming languages. I'm Cc'ing Emacs Org mailing list. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Inheriting some local variables from source code block editing buffers 2018-05-01 11:55 ` Nicolas Goaziou [not found] ` <ygmvac7lcl1.fsf@gkayaalp.com> @ 2018-05-01 16:37 ` Aaron Ecay [not found] ` <ygmin87kwua.fsf@gkayaalp.com> 1 sibling, 1 reply; 28+ messages in thread From: Aaron Ecay @ 2018-05-01 16:37 UTC (permalink / raw) To: Nicolas Goaziou, Göktuğ Kayaalp; +Cc: Bastien, emacs-orgmode 2018ko maiatzak 1an, Nicolas Goaziou-ek idatzi zuen: > > No, because you can use Noweb syntax wherever you need these local > variables set: > > <<local-variables>> This would still require adding something to each source block, which I think Göktuğ wants to avoid doing. With respect to the original proposal, I donʼt think itʼs a good idea to inherit the values from the org buffer. I can imagine that one might want to have different values of (e.g.) fill-column in org-mode vs emacs-lisp-mode. The approach that should be taken should be to use a header argument to specify an alist of variables and values to set in src block edit buffers. Then the usual methods (info "(org) Using header arguments") could be used to control the feature globally, per-buffer, per-language, per-subtree, and per-individual source block. If this isnʼt desired as a core feature I wouldnʼt see an impediment to having it be part of org-contrib, though it could also be distributed via (GNU/M)ELPA; it would be Göktuğʼs choice I guess. Aaron PS My view is the lexical-binding case is important enough that this feature should be included in core, and that core should default to lexical-binding: t in elisp src blocks -- Aaron Ecay ^ permalink raw reply [flat|nested] 28+ messages in thread
[parent not found: <ygmin87kwua.fsf@gkayaalp.com>]
* Re: Inheriting some local variables from source code block editing buffers [not found] ` <ygmin87kwua.fsf@gkayaalp.com> @ 2018-05-01 19:35 ` Aaron Ecay 2018-05-01 20:04 ` Göktuğ Kayaalp 0 siblings, 1 reply; 28+ messages in thread From: Aaron Ecay @ 2018-05-01 19:35 UTC (permalink / raw) To: Göktuğ Kayaalp; +Cc: emacs-orgmode 2018ko maiatzak 1an, Göktuğ Kayaalp-ek idatzi zuen: [...] >> The approach that should be taken should be to use a header argument to >> specify an alist of variables and values to set in src block edit buffers. >> Then the usual methods (info "(org) Using header arguments") could be used >> to control the feature globally, per-buffer, per-language, per-subtree, >> and per-individual source block. > > I agree. But note that what I want to do is to set some buffer local > variables in the special block editing buffer. The header argument > ":lexical yes" seems to not affect the editing buffer Thinking about it some more, lexical binding is not a good case for this feature, since it has to be set not only in the edit buffer, but also when C-c C-c is used in the org-mode buffer to evaluate the src block. The :lexical header arg (which I did not know about) works for the latter but not the former. To complete the picture, Someone™ needs to implement a org-babel-edit-prep:emacs-lisp function which looks for :lexical yes in the header arguments and sets the value in the edit buffer appropriately. If lexical-binding is the major motivating factor, then maybe the above is enough. My original suggestion was for something like: #+begin_src emacs-lisp :edit-vars ((fill-column 72) (other-var other-val)) ... #+end_src This would set the variables in the edit buffer in the (hopefully) obvious way. It is not implemented yet, though. Aaron PS Itʼs best to use “reply all” and include the org mode mailing list in replies, to make sure everyone following the thread sees all the messages. -- Aaron Ecay ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Inheriting some local variables from source code block editing buffers 2018-05-01 19:35 ` Aaron Ecay @ 2018-05-01 20:04 ` Göktuğ Kayaalp 2018-05-01 20:53 ` Aaron Ecay 0 siblings, 1 reply; 28+ messages in thread From: Göktuğ Kayaalp @ 2018-05-01 20:04 UTC (permalink / raw) To: Aaron Ecay; +Cc: emacs-orgmode On 2018-05-01 20:35 +01, Aaron Ecay <aaronecay@gmail.com> wrote: > Thinking about it some more, lexical binding is not a good case for > this feature, since it has to be set not only in the edit buffer, but > also when C-c C-c is used in the org-mode buffer to evaluate the src > block. The :lexical header arg (which I did not know about) works for > the latter but not the former. To complete the picture, Someone™ needs > to implement a org-babel-edit-prep:emacs-lisp function which looks for > :lexical yes in the header arguments and sets the value in the edit > buffer appropriately. I can (and plan to) take on that task once a design is agreed upon. I don't really know much about Org internals, but I think I can figure out. > If lexical-binding is the major motivating factor, then maybe the above > is enough. My original suggestion was for something like: > > #+begin_src emacs-lisp :edit-vars ((fill-column 72) (other-var other-val)) > ... > #+end_src > > This would set the variables in the edit buffer in the (hopefully) > obvious way. It is not implemented yet, though. I do like that syntax. A minor addition might be for the case when one wants to pass the value of a variable local to the org buffer to the editing buffer: -*- fill-column: 65; indent-tabs-mode: nil -*- #+edit-vars: (indent-tabs-mode) #+begin_src emacs-lisp :edit-vars (fill-column (lexical-binding t)) ;; here, when editing, fill-column is 65, lexical binding is t ;; and indent-tabs-mode is nil #+end_src > PS Itʼs best to use “reply all” and include the org mode mailing list in > replies, to make sure everyone following the thread sees all the > messages. Whoops! Sorry, yeah, I have been hitting the wrong keybinding for the last couple of messages I think... -- İ. Göktuğ Kayaalp <https://www.gkayaalp.com/> 024C 30DD 597D 142B 49AC 40EB 465C D949 B101 2427 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Inheriting some local variables from source code block editing buffers 2018-05-01 20:04 ` Göktuğ Kayaalp @ 2018-05-01 20:53 ` Aaron Ecay 2018-05-01 22:12 ` Göktuğ Kayaalp 0 siblings, 1 reply; 28+ messages in thread From: Aaron Ecay @ 2018-05-01 20:53 UTC (permalink / raw) To: Göktuğ Kayaalp; +Cc: emacs-orgmode 2018ko maiatzak 1an, Göktuğ Kayaalp-ek idatzi zuen: > > On 2018-05-01 20:35 +01, Aaron Ecay <aaronecay@gmail.com> wrote: >> Thinking about it some more, lexical binding is not a good case for >> this feature, since it has to be set not only in the edit buffer, but >> also when C-c C-c is used in the org-mode buffer to evaluate the src >> block. The :lexical header arg (which I did not know about) works for >> the latter but not the former. To complete the picture, Someone™ needs >> to implement a org-babel-edit-prep:emacs-lisp function which looks for >> :lexical yes in the header arguments and sets the value in the edit >> buffer appropriately. > > I can (and plan to) take on that task once a design is agreed upon. I > don't really know much about Org internals, but I think I can figure > out. That is excellent news :) If you run into anything you canʼt figure out then let us know. > >> If lexical-binding is the major motivating factor, then maybe the above >> is enough. My original suggestion was for something like: >> >> #+begin_src emacs-lisp :edit-vars ((fill-column 72) (other-var other-val)) >> ... >> #+end_src >> >> This would set the variables in the edit buffer in the (hopefully) >> obvious way. It is not implemented yet, though. > > I do like that syntax. A minor addition might be for the case when one > wants to pass the value of a variable local to the org buffer to the > editing buffer: That looks fine to me. > > -*- fill-column: 65; indent-tabs-mode: nil -*- > #+edit-vars: (indent-tabs-mode) A minor note, the above line should read: #+header: :edit-vars (indent-tabs-mode) > #+begin_src emacs-lisp :edit-vars (fill-column (lexical-binding t)) But because of the nature of the variable (a lisp list), it can only be set once. So you can have only one of: #+header :edit-vars ... OR #+begin_src emacs-lisp :edit-vars ... OR #+property: header-args:emacs-lisp :edit-vars ... OR :PROPERTIES: :header-args:emacs-lisp: :edit-vars ... :END: But they canʼt be combined. AFAIR, :var is the only header argument that can be meaningfully specified more than once. -- Aaron Ecay ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Inheriting some local variables from source code block editing buffers 2018-05-01 20:53 ` Aaron Ecay @ 2018-05-01 22:12 ` Göktuğ Kayaalp 2018-05-01 22:19 ` Aaron Ecay 2018-05-01 22:26 ` Göktuğ Kayaalp 0 siblings, 2 replies; 28+ messages in thread From: Göktuğ Kayaalp @ 2018-05-01 22:12 UTC (permalink / raw) To: Aaron Ecay; +Cc: emacs-orgmode On 2018-05-01 21:53 +01, Aaron Ecay <aaronecay@gmail.com> wrote: > That is excellent news :) If you run into anything you canʼt figure out > then let us know. I will probably be able to start working on this next weekend (tho there is some stuff that can inevitably slow me down this week). In the mean time other people can comment both on this and on where to put the resulting feature. > But because of the nature of the variable (a lisp list), it can only be > set once. So you can have only one of: > [...] > But they canʼt be combined. AFAIR, :var is the only header argument > that can be meaningfully specified more than once. Okay, I'll read up on these, both code and manuals. So we've agreed that what we want is a new header argument, ‘:edit-vars’, whose value is a form similar to a varlist, where - a form (var val) means bind var to val in the editing buffer, - a symbol var means bind var in the editing buffer to the buffer-local value of it in the relevant x.org buffer, as in (setq (make-local-variable var) (with-current-buffer "x.org" var)) Do you confirm? Also, what do you think about :edit-bindings or :edit-locals instead of :edit-vars? :var is a completely different thing, and :edit-vars may cause confusion, given the similarity of the name. -- İ. Göktuğ Kayaalp <https://www.gkayaalp.com/> 024C 30DD 597D 142B 49AC 40EB 465C D949 B101 2427 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Inheriting some local variables from source code block editing buffers 2018-05-01 22:12 ` Göktuğ Kayaalp @ 2018-05-01 22:19 ` Aaron Ecay 2018-05-01 22:26 ` Göktuğ Kayaalp 1 sibling, 0 replies; 28+ messages in thread From: Aaron Ecay @ 2018-05-01 22:19 UTC (permalink / raw) To: Göktuğ Kayaalp; +Cc: emacs-orgmode 2018ko maiatzak 1an, Göktuğ Kayaalp-ek idatzi zuen: [...] > So we've agreed > that what we want is a new header argument, ‘:edit-vars’, whose value is > a form similar to a varlist, where > > - a form (var val) means bind var to val in the editing buffer, > > - a symbol var means bind var in the editing buffer to the buffer-local > value of it in the relevant x.org buffer, as in (setq > (make-local-variable var) (with-current-buffer "x.org" var)) > > Do you confirm? Thatʼs how I understand the proposal, and it seems good to me. > Also, what do you think about :edit-bindings or :edit-locals instead > of :edit-vars? :var is a completely different thing, and :edit-vars > may cause confusion, given the similarity of the name. Agreed. I prefer -bindings, but I suppose itʼs not terribly important: either of your proposed names is a big improvement. -- Aaron Ecay ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Inheriting some local variables from source code block editing buffers 2018-05-01 22:12 ` Göktuğ Kayaalp 2018-05-01 22:19 ` Aaron Ecay @ 2018-05-01 22:26 ` Göktuğ Kayaalp 2018-05-02 10:16 ` Nicolas Goaziou 1 sibling, 1 reply; 28+ messages in thread From: Göktuğ Kayaalp @ 2018-05-01 22:26 UTC (permalink / raw) To: Aaron Ecay; +Cc: emacs-orgmode On 2018-05-02 01:12 +03, Göktuğ Kayaalp <self@gkayaalp.com> wrote: > Okay, I'll read up on these, both code and manuals. So we've agreed > that what we want is a new header argument, ‘:edit-vars’, whose value is > a form similar to a varlist, where > > - a form (var val) means bind var to val in the editing buffer, > > - a symbol var means bind var in the editing buffer to the buffer-local > value of it in the relevant x.org buffer, as in (setq > (make-local-variable var) (with-current-buffer "x.org" var)) > > Do you confirm? Also, what do you think about :edit-bindings or > :edit-locals instead of :edit-vars? :var is a completely different > thing, and :edit-vars may cause confusion, given the similarity of the > name. Also, another question remains: how do we extend this to #+begin_export blocks? But that's unclear to me maybe because I don't know in detail how header arguments work. Ideally this feature would work for _any block_ editable by ‘org-edit-special’ (i.e. C-c '), and again ideally using the same syntax. -- İ. Göktuğ Kayaalp <https://www.gkayaalp.com/> 024C 30DD 597D 142B 49AC 40EB 465C D949 B101 2427 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Inheriting some local variables from source code block editing buffers 2018-05-01 22:26 ` Göktuğ Kayaalp @ 2018-05-02 10:16 ` Nicolas Goaziou 2018-05-02 19:52 ` Göktuğ Kayaalp 0 siblings, 1 reply; 28+ messages in thread From: Nicolas Goaziou @ 2018-05-02 10:16 UTC (permalink / raw) To: Göktuğ Kayaalp; +Cc: Aaron Ecay, emacs-orgmode Hello, Göktuğ Kayaalp <self@gkayaalp.com> writes: > Also, another question remains: how do we extend this to #+begin_export > blocks? But that's unclear to me maybe because I don't know in detail > how header arguments work. Ideally this feature would work for _any > block_ editable by ‘org-edit-special’ (i.e. C-c '), and again ideally > using the same syntax. Export blocks, like any other element, accept #+header: keyword, e.g., #+header: whatever #+begin_export latex ... #+end_export Note that, however, such header arguments are ignored during export. Regards, -- Nicolas Goaziou ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Inheriting some local variables from source code block editing buffers 2018-05-02 10:16 ` Nicolas Goaziou @ 2018-05-02 19:52 ` Göktuğ Kayaalp 0 siblings, 0 replies; 28+ messages in thread From: Göktuğ Kayaalp @ 2018-05-02 19:52 UTC (permalink / raw) To: Nicolas Goaziou; +Cc: Aaron Ecay, emacs-orgmode On 2018-05-02 12:16 +02, Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote: > Export blocks, like any other element, accept #+header: keyword, e.g., > > #+header: whatever > #+begin_export latex > > ... > #+end_export > > Note that, however, such header arguments are ignored during export. Thanks. This means implementing this feature as a header argument should suffice for all useful cases (though it'll probably become more clearer when it's implemented and tested). -- İ. Göktuğ Kayaalp <https://www.gkayaalp.com/> 024C 30DD 597D 142B 49AC 40EB 465C D949 B101 2427 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Inheriting some local variables from source code block editing buffers 2018-05-01 8:43 ` Nicolas Goaziou 2018-05-01 8:45 ` Nicolas Goaziou 2018-05-01 11:41 ` Göktuğ Kayaalp @ 2018-05-01 11:45 ` Göktuğ Kayaalp 2 siblings, 0 replies; 28+ messages in thread From: Göktuğ Kayaalp @ 2018-05-01 11:45 UTC (permalink / raw) To: Nicolas Goaziou; +Cc: Bastien, emacs-orgmode BTW I can also implement this as a 3rd party extension instead of as a core feature, I can see how it's possibly not generic enough for that. -- İ. Göktuğ Kayaalp <https://www.gkayaalp.com/> 024C 30DD 597D 142B 49AC 40EB 465C D949 B101 2427 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Inheriting some local variables from source code block editing buffers 2018-04-29 17:19 Inheriting some local variables from source code block editing buffers Göktuğ Kayaalp 2018-04-29 22:09 ` Bastien @ 2018-05-14 5:44 ` Göktuğ Kayaalp 2018-05-14 12:13 ` Nicolas Goaziou 2018-05-14 13:33 ` Aaron Ecay 1 sibling, 2 replies; 28+ messages in thread From: Göktuğ Kayaalp @ 2018-05-14 5:44 UTC (permalink / raw) To: emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 2722 bytes --] Hello, Sorry for the silence, I've finally got around to implementing this, and implemented it as an advice, which supports both an ‘:edit-bindings’ Babel header argument for source code blocks, and an #+ATTR_EDIT: element property for export blocks, etc. Find the code below, and attached an Org mode file to help with testing. This advice can easily be made into a patch to the ‘org-src--edit-element’ function. One ‘gotcha’ is that :edit-bindings requires a quoted list whereas the explicit quote is not necessary with ATTR_EDIT: #+BEGIN_SRC elisp :edit-bindings '((lexical-binding t)) #+ATTR_EDIT: ((lexical-binding t)) Another problem is that I was not able to define a new element property named EDIT_BINDINGS, and had to take the shortcut with naming it as an ATTR_* variable. Preferably, it'd be EDIT_BINDINGS instead: #+BEGIN_SRC elisp :edit-bindings '((lexical-binding t)) #+EDIT_BINDINGS: ((lexical-binding t)) But personally I don't think it's that big of a problem. The advice: (define-advice org-src--edit-element (:around (fn datum &rest args) attr-edit&edit-bindings) "Apply edit-special bindings." (let ((attr-edit (org-element-property :attr_edit datum)) (edit-bindings (assoc :edit-bindings (caddr (org-babel-get-src-block-info nil datum)))) (source-buffer (current-buffer)) (sub (lambda (varlist source-buffer) (let (var val) (dolist (form varlist) ;; If it's a symbol, inherit from the Org mode buffer. (if (symbolp form) (setq var form val (with-current-buffer source-buffer (eval var))) ;; Else, apply the specified value. (setq var (car form) val (cadr form))) (unless (symbolp var) ;; XXX: maybe tell where it is? (user-error "Bad varlist at ATTR_EDIT")) (set (make-local-variable var) val)))))) ;; Apply function (apply fn datum args) ;; Apply edit attributes (ATTR_EDIT). (dolist (attr attr-edit) (let ((varlist (car (read-from-string attr)))) (unless (listp varlist) (user-error "Value of ATTR_EDIT must be a varlist.")) (funcall sub varlist source-buffer))) ;; Apply edit bindings (:edit-bindings header argument). ;; These override attr-edit values. (when edit-bindings (funcall sub (cdr edit-bindings) source-buffer)))) -- İ. Göktuğ Kayaalp <https://www.gkayaalp.com/> 024C 30DD 597D 142B 49AC 40EB 465C D949 B101 2427 [-- Attachment #2: Testing instructions. --] [-- Type: text/x-org, Size: 2374 bytes --] # $Id: edit-bindings.org,v 1.2 2018/05/14 05:23:48 g Exp $ #+title: :edit-bindings and #+ATTR_EDIT test file #+author: Göktuğ Kayaalp We use the variables ~lexical-binding~, ~truncate-lines~ and ~word-wrap~ for this test/demo. We assume the first is ~nil~, the second is set from a file local variable, and for the third we have no assumptions. #+BEGIN_SRC elisp :results verbatim (list lexical-binding truncate-lines word-wrap) #+END_SRC #+RESULTS: : (nil 62 t) Now we set up the default ~:edit-bindings~ property. #+PROPERTY: header-args :edit-bindings '((lexical-binding t)) From now on, we'll do our observations evaluating the forms in source code blocks via =C-c ' C-M-x=, and evaluating variables via =M-:= in the edit special buffer. In edit-special buffer, when editing, we observe that ~lexical-binding~ is set to ~t~: #+BEGIN_SRC elisp (eq lexical-binding t) #+END_SRC But ~truncate-lines~ is not set: #+BEGIN_SRC elisp (eq truncate-lines 62) #+END_SRC Copy ~truncate-lines~ over from this file's buffer local variables: #+BEGIN_SRC elisp :edit-bindings '(truncate-lines) (list lexical-binding truncate-lines) #+END_SRC We observe that now the buffer-wide value is shadoved, and while truncate lines is passed through, ~lexical-binding~ is not, it's nil. So we pass both through: #+BEGIN_SRC elisp :edit-bindings '((lexical-binding t) truncate-lines) (list (eq truncate-lines 62) (eq lexical-binding t)) #+END_SRC Export blocks can not use header arguments. Thus we use an element property called ~ATTR_EDIT~: #+ATTR_EDIT: ((fill-column 30) truncate-lines) #+BEGIN_EXPORT latex Nullam eu ante vel est convallis dignissim. Fusce suscipit, wisi nec facilisis facilisis, est dui fermentum leo, quis tempor ligula erat quis odio. Nunc porta vulputate tellus. Nunc rutrum turpis sed pede. Sed bibendum. #+END_EXPORT When we observe the value of ~truncate-lines~, we see that it is 62, and when we use ~fill-paragraph~, that it wraps according to the new value of ~fill-column~. We get an error if we supply a bad varlist, but editing continues anyways: #+BEGIN_SRC python :edit-bindings '(("monty" "python")) print(None) #+END_SRC #+ATTR_EDIT: Tea? #+BEGIN_EXPORT latex \textit{nope} #+END_EXPORT # Local Variables: # truncate-lines: 62 # End: ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Inheriting some local variables from source code block editing buffers 2018-05-14 5:44 ` Göktuğ Kayaalp @ 2018-05-14 12:13 ` Nicolas Goaziou 2018-05-14 16:34 ` Göktuğ Kayaalp 2018-05-14 13:33 ` Aaron Ecay 1 sibling, 1 reply; 28+ messages in thread From: Nicolas Goaziou @ 2018-05-14 12:13 UTC (permalink / raw) To: Göktuğ Kayaalp; +Cc: emacs-orgmode Hello, Göktuğ Kayaalp <self@gkayaalp.com> writes: > Sorry for the silence, I've finally got around to implementing this, and > implemented it as an advice, which supports both an ‘:edit-bindings’ > Babel header argument for source code blocks, and an #+ATTR_EDIT: > element property for export blocks, etc. Find the code below, and > attached an Org mode file to help with testing. Thank you. Some comments follow. You shouldn't add another "attr" keyword, which is reserved for export back-ends. Actually, every Babel header can be located either on the block opening line, e.g., #+begin_src emacs-lisp :some-property some-value or as an affiliated #+header: keyword, e.g., #+header: :some-property some-value #+begin_src emacs-lisp Note that "#+header:" keywords are supported everywhere, without modifying the parser, e.g., #+header: :some-property some-value A paragraph. Also, for integration in Org mode proper, some testing would be more than welcome Regards, -- Nicolas Goaziou ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Inheriting some local variables from source code block editing buffers 2018-05-14 12:13 ` Nicolas Goaziou @ 2018-05-14 16:34 ` Göktuğ Kayaalp 2018-05-14 16:47 ` Nicolas Goaziou 0 siblings, 1 reply; 28+ messages in thread From: Göktuğ Kayaalp @ 2018-05-14 16:34 UTC (permalink / raw) To: Nicolas Goaziou; +Cc: emacs-orgmode On 2018-05-14 14:13 +02, Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote: > You shouldn't add another "attr" keyword, which is reserved for export > back-ends. Actually, every Babel header can be located either on the > block opening line, e.g., > > #+begin_src emacs-lisp :some-property some-value > > or as an affiliated #+header: keyword, e.g., > > #+header: :some-property some-value > #+begin_src emacs-lisp > > > Note that "#+header:" keywords are supported everywhere, without > modifying the parser, e.g., > > #+header: :some-property some-value > A paragraph. The attr was meant for BEGIN_EXPORT blocks because it seems to me that an equivalent of ‘org-babel-get-src-block-info’ does not exist for those blocks, and that function _only_ works with BEGIN_SRC blocks. Is there a function available or would I have to write one to do this? Looking all over the Org manual searching for BEGIN_(LATEX|HTML), I haven't seen once a header argument used with a block that is not a BEGIN_SRC block, in neither of the forms. And none of the ‘org-edit-*’ functions apart from ‘org-edit-src-code’ in org-src.el seem to process header arguments, and nor does ‘org-src--edit-element’. I can't find any documentation on Org-mode's internal APIs and how different parts fit together, so I'm having to figure things out reading source code. The following form returns nil for the following examples: (plist-get :header (cadr (org-element-at-point))) ;=> nil (cl-remove-if-not #'symbolp (cadr (org-element-at-point))) ;=> (:type :begin :end :value :post-blank :post-affiliated :header :parent nil) #+header: :edit-bindings '((lexical-binding t)) #+BEGIN_EXPORT latex #+END_EXPORT #+BEGIN_EXPORT latex :edit-bindings '((lexical-binding t)) #+END_EXPORT #+BEGIN_SRC elisp :edit-bindings '((lexical-binding t)) #+END_SRC #+header: :edit-bindings '((lexical-binding t)) #+BEGIN_SRC elisp #+END_SRC > Also, for integration in Org mode proper, some testing would be more > than welcome If this feature will be included upstream, I can make this into a patch instead of an advice and add the related docs and tests. This was meant as a concrete example of the concept. -- İ. Göktuğ Kayaalp <https://www.gkayaalp.com/> 024C 30DD 597D 142B 49AC 40EB 465C D949 B101 2427 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Inheriting some local variables from source code block editing buffers 2018-05-14 16:34 ` Göktuğ Kayaalp @ 2018-05-14 16:47 ` Nicolas Goaziou 2018-05-15 18:36 ` Göktuğ Kayaalp 0 siblings, 1 reply; 28+ messages in thread From: Nicolas Goaziou @ 2018-05-14 16:47 UTC (permalink / raw) To: Göktuğ Kayaalp; +Cc: emacs-orgmode Göktuğ Kayaalp <self@gkayaalp.com> writes: > The attr was meant for BEGIN_EXPORT blocks because it seems to me that > an equivalent of ‘org-babel-get-src-block-info’ does not exist for those > blocks, and that function _only_ works with BEGIN_SRC blocks. Is there > a function available or would I have to write one to do this? With the following example: #+header: :foo bar #+begin_export latex Foo #+end_export (org-element-property :header (org-element-at-point)) => (":foo bar") and (cl-mapcan #'org-babel-parse-header-arguments (org-element-property :header (org-element-at-point))) => ((:foo . "bar")) > Looking all over the Org manual searching for BEGIN_(LATEX|HTML), I > haven't seen once a header argument used with a block that is not a > BEGIN_SRC block, in neither of the forms. And none of the ‘org-edit-*’ > functions apart from ‘org-edit-src-code’ in org-src.el seem to process > header arguments, and nor does ‘org-src--edit-element’. True, but this is also true for "attr_...". > I can't find any documentation on Org-mode's internal APIs and how > different parts fit together, so I'm having to figure things out reading > source code. See <https://orgmode.org/worg/dev/org-element-api.html>. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Inheriting some local variables from source code block editing buffers 2018-05-14 16:47 ` Nicolas Goaziou @ 2018-05-15 18:36 ` Göktuğ Kayaalp 2018-05-18 21:48 ` Göktuğ Kayaalp 0 siblings, 1 reply; 28+ messages in thread From: Göktuğ Kayaalp @ 2018-05-15 18:36 UTC (permalink / raw) To: Nicolas Goaziou; +Cc: Aaron Ecay, emacs-orgmode On 2018-05-14 18:47 +02, Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote: > (org-element-property :header (org-element-at-point)) => (":foo bar") [...] >> I can't find any documentation on Org-mode's internal APIs and how >> different parts fit together, so I'm having to figure things out reading >> source code. > > See <https://orgmode.org/worg/dev/org-element-api.html>. Thanks, and sorry. I've been mostly ignoring Worg up until now, because I haven't seen the worg/dev/ pages mentioned in the info manual (I'd expect to see those pages mentioned in the Hacking section BTW, I can send a patch mentioning them in a "Further resources" subsection there if you'd like that added). So, today I've started implementing a version of this that works like this: #+begin_example #+property: edit-bindings /varlist/ * heading :properties: :edit_bindings: /varlist/ :end: #+header: edit-bindings /varlist/ #+end_example where scoping is like (x > y meaning x overrides y): header line bindings > subtree bindings > #+property bindings before the element I'll send a complete patch soon. Best, -- İ. Göktuğ Kayaalp <https://www.gkayaalp.com/> 024C 30DD 597D 142B 49AC 40EB 465C D949 B101 2427 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Inheriting some local variables from source code block editing buffers 2018-05-15 18:36 ` Göktuğ Kayaalp @ 2018-05-18 21:48 ` Göktuğ Kayaalp 2018-05-19 12:26 ` Nicolas Goaziou 2018-05-21 14:20 ` Aaron Ecay 0 siblings, 2 replies; 28+ messages in thread From: Göktuğ Kayaalp @ 2018-05-18 21:48 UTC (permalink / raw) To: Nicolas Goaziou; +Cc: Aaron Ecay, emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 1277 bytes --] On 2018-05-15 21:36 +03, Göktuğ Kayaalp <self@gkayaalp.com> wrote: > So, today I've started implementing a version of this that works like > this: > > #+begin_example > #+property: edit-bindings /varlist/ > * heading > :properties: > :edit_bindings: /varlist/ > :end: > #+header: edit-bindings /varlist/ > #+end_example > > > where scoping is like (x > y meaning x overrides y): > > header line bindings > subtree bindings > #+property bindings before > the element > > I'll send a complete patch soon. And here it is. Please find the patch attached. I have ran the entire test suite against this, which completed w/ 6 failures, seemingly unrelated (they fail on revision 58c9bedfd too); find the list below. I have tried to follow apparent conventions in each file w.r.t. the copyright/authors lines, sorry if I modified them unnecessarily. 6 unexpected results: FAILED test-ob/org-babel-remove-result--results-list FAILED test-org-clock/clocktable/step FAILED test-org/add-planning-info FAILED test-org/clone-with-time-shift FAILED test-org/deadline FAILED test-org/schedule -- İ. Göktuğ Kayaalp <https://www.gkayaalp.com/> 024C 30DD 597D 142B 49AC 40EB 465C D949 B101 2427 [-- Attachment #2: The patch. --] [-- Type: text/x-diff, Size: 22351 bytes --] From b6d2b60730ceed68f46ef839c486e03764defdc7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=B0=2E=20G=C3=B6ktu=C4=9F=20Kayaalp?= <self@gkayaalp.com> Date: Tue, 15 May 2018 20:34:28 +0300 Subject: [PATCH] Implement edit bindings feature Enable defining local variable bindings to be applied when editing source code. * lisp/org-src.el (org-src--apply-edit-bindings) (org-src--simplify-edit-bindings) (org-src--parse-edit-bindings) (org-src--edit-bindings-string) (org-src--get-edit-bindings-for-subtree) (org-src--get-edit-bindings-from-header) (org-src--collect-global-edit-bindings) (org-src--collect-edit-bindings-for-element): New functions. (org-src-apply-risky-edit-bindings): New defcustom. (org-src--edit-element): * doc/org.texi (Editing source code): Add edit bindings. * testing/lisp/test-org-src.el (test-org-src/edit-bindings-parser) (test-org-src/collect-edit-bindings-for-element) (test-org-src/edit-bindings-precedence-and-application) (test-org-src/edit-bindings-use-cases): Add relevant tests. --- doc/org.texi | 43 +++++++++ etc/ORG-NEWS | 15 +++ lisp/org-src.el | 223 +++++++++++++++++++++++++++++++++++++++---- testing/lisp/test-org-src.el | 172 ++++++++++++++++++++++++++++++++- 4 files changed, 436 insertions(+), 17 deletions(-) diff --git a/doc/org.texi b/doc/org.texi index 6aab1ba4e..c588152fd 100644 --- a/doc/org.texi +++ b/doc/org.texi @@ -15364,6 +15364,7 @@ Source code in the dialect of the specified language identifier. @vindex org-edit-src-auto-save-idle-delay @vindex org-edit-src-turn-on-auto-save +@vindex org-src-apply-risky-edit-bindings @kindex C-c ' @kbd{C-c '} for editing the current code block. It opens a new major-mode edit buffer containing the body of the @samp{src} code block, ready for any @@ -15421,6 +15422,48 @@ Emacs-Lisp languages. ("python" (:background "#E5FFB8")))) @end lisp +It is possible to define local variable bindings for these buffers using the +@samp{edit-bindings} element header, the @samp{edit-bindings} buffer +property, or the @samp{EDIT_BINDINGS} entry property. All three can be used +together, the bindings from the header override those of the subtree, and +they both override the bindings from buffer properties. The syntax is +similar to that of @code{let} varlists, but a sole symbol means the +variable's value is copied from the Org mode buffer. Multiple uses of +@samp{edit-bindings} headers and buffer properties are supported, and works +like @code{let*}. Entry property @samp{EDIT_BINDINGS} can not be repeated. +Below is an example: + +@example +# -*- fill-column: 65 -*- +#+PROPERTY: edit-bindings '(fill-column (lexical-binding t)) + +* Example section +:PROPERTIES: +:EDIT_BINDINGS: '((emacs-lisp-docstring-fill-column 60)) +:END: + +#+HEADER: edit-bindings '((lexical-binding nil)) +#+BEGIN_SRC elisp +(defun hello () + (message "Hello world!")) +#+END_SRC + +* Another section +#+BEGIN_SRC elisp +(defun hello () + (message "Byes world!")) +#+END_SRC +@end example + +In this example, when editing the first block, @code{lexical-binding} will be +@code{nil}, and @code{emacs-lisp-docstring-fill-column} 60. With the second +one, they will be @code{t} and the variable's default value, respectively. +@code{fill-column} will be 65 for both. + +Set @code{org-src-apply-risky-edit-bindings} for how risky local variables in +these bindings are handled. The default behaviour is to ask to the user +whether or not to apply them. + @node Exporting code blocks @section Exporting code blocks @cindex code block, exporting diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS index 21eaaece6..879240f31 100644 --- a/etc/ORG-NEWS +++ b/etc/ORG-NEWS @@ -95,6 +95,21 @@ document, use =shrink= value instead, or in addition to align: #+END_EXAMPLE ** New features +*** Set local variables for editing blocks +Bindings from =edit-bindings= header and buffer property, and +=EDIT_BINDINGS= entry property are applied in Org Src buffers. For +example, when editing the following code block with +~org-edit-special~: + +#+BEGIN_EXAMPLE +#+header: edit-bindings '((lexical-binding t)) +#+BEGIN_SRC elisp +;; some code +#+END_SRC +#+END_EXAMPLE + +in the source code editing buffer, ~lexical-binding~ is set to ~t~. + *** Org Tempo may used for snippet expansion of structure template. See manual and the commentary section in ~org-tempo.el~ for details. *** Exclude unnumbered headlines from table of contents diff --git a/lisp/org-src.el b/lisp/org-src.el index b27e96cbc..a1b766813 100644 --- a/lisp/org-src.el +++ b/lisp/org-src.el @@ -1,10 +1,11 @@ ;;; org-src.el --- Source code examples in Org -*- lexical-binding: t; -*- ;; -;; Copyright (C) 2004-2017 Free Software Foundation, Inc. +;; Copyright (C) 2004-2018 Free Software Foundation, Inc. ;; ;; Author: Carsten Dominik <carsten at orgmode dot org> ;; Bastien Guerry <bzg@gnu.org> ;; Dan Davison <davison at stats dot ox dot ac dot uk> +;; Göktuğ Kayaalp <self at gkayaalp.com> ;; Keywords: outlines, hypermedia, calendar, wp ;; Homepage: http://orgmode.org ;; @@ -36,6 +37,7 @@ (require 'org-compat) (require 'ob-keys) (require 'ob-comint) +(require 'subr-x) (declare-function org-base-buffer "org" (buffer)) (declare-function org-do-remove-indentation "org" (&optional n)) @@ -226,6 +228,26 @@ issued in the language major mode buffer." :version "24.1" :group 'org-babel) +(defcustom org-src-apply-risky-edit-bindings 'ask + "What to do if an edit binding is a risky local variable. +If this is nil, bindings that satisfy ‘risky-local-variable-p’ +are skipped, with a warning message. Otherwise, its value should +be a symbol telling how to thread them. Possible values of this +setting are: + +nil Skip, warning the user via a message. +skip-silent Skip risky local varibles silently. +ask Prompt user for each variable. +t Apply the variable but show a warning. +apply-silent Apply risky local variables silently." + :group 'org-edit-structure + :risky t + :type '(radio + (const :tag "Skip, warning the user via a message" nil) + (const :tag "Skip risky local varibles silently" 'skip-silent) + (const :tag "Prompt user for each variable" 'ask) + (const :tag "Apply the variable but show a warning" t) + (const :tag "Apply risky local variables silently" 'apply-silent))) \f ;;; Internal functions and variables @@ -424,6 +446,168 @@ Assume point is in the corresponding edit buffer." (forward-line))) (buffer-string)))) +(defun org-src--apply-edit-bindings (simplified-bindings) + (pcase-dolist (`(,name . ,value) simplified-bindings) + (let ((prompt-apply + (concat "Setting risky local variable ‘%S’" + " in edit-special buffer, its value is: %S; Continue?")) + (risky-message "%s risky local variable ‘%S’ in edit-special buffer.") + (apply-binding (lambda () (set (make-local-variable name) + (eval value))))) + (unless + (and + (risky-local-variable-p name) + (cond ((or (and (eq org-src-apply-risky-edit-bindings 'ask) + (y-or-n-p (format prompt-apply name value))) + (eq org-src-apply-risky-edit-bindings 'apply-silent)) + (funcall apply-binding)) + (org-src-apply-risky-edit-bindings + (prog1 + (funcall apply-binding) + (message risky-message "Applied" name))) + ((not org-src-apply-risky-edit-bindings) + (message risky-message "Skipped" name)) + ((eq org-src-apply-risky-edit-bindings 'skip-silent)) + ('else + (user-error + "Unexpected value for ‘%S’, will not apply this or any more bindings." + 'org-src-apply-risky-edit-bindings)))) + (funcall apply-binding))))) + +(defun org-src--simplify-edit-bindings (raw-bindings) + ;; The many uses of ‘nreverse’ is aimed at keeping the order the + ;; bindings are written, so that the effect of the previous binding + ;; can reliably be used by the following one. + ;; + ;; The deep copy of raw bindings should not be necessary for general + ;; use cases, but it's useful for the tests, and might come in handy + ;; if values are cached in the future. + (let* ((b (copy-tree raw-bindings)) + (elem (plist-get (plist-get b :element-bindings) :varlist)) + (subtree (plist-get (plist-get b :subtree-bindings) :varlist)) + (global (plist-get b :global-bindings)) + (resulting-bindings + (nreverse + (append + (nreverse (apply #'append elem)) + (apply #'append + (nreverse subtree) + (mapcar (lambda (s) + (nreverse (plist-get s :varlist))) + (nreverse global)))))) + simplified-bindings) + (pcase-dolist (`(,name . ,value) resulting-bindings) + (setq simplified-bindings + (append (cl-remove-if + (lambda (x) (equal (car x) name)) + simplified-bindings) + (list (cons name value))))) + simplified-bindings)) + +(defun org-src--collect-edit-bindings-for-element () + (let* ((element-bindings (org-src--get-edit-bindings-from-header)) + (subtree-bindings (org-src--get-edit-bindings-for-subtree)) + (global-bindings + (save-excursion + (save-restriction + (narrow-to-region (point-min) (plist-get element-bindings :end)) + (org-src--collect-global-edit-bindings))))) + (list :element-bindings element-bindings + :subtree-bindings subtree-bindings + :global-bindings global-bindings))) + +(defun org-src--collect-global-edit-bindings () + ;; XXX: is setting GRANULARITY to 'element a performance + ;; improvement, and does it cause any problems over just using the + ;; default 'object? + ;; + ;; Also, is it possible to not have to parse the entire buffer every + ;; time? + (org-element-map + (org-element-parse-buffer 'element) + 'keyword + (lambda (keyword) + (cl-destructuring-bind + (_1 (_2 type _3 value _4 pos-beg _5 pos-end &rest _6)) + keyword + (ignore _1 _2 _3 _4 _5 _6) + (when-let* + ((sexp-str + (and (string= type "PROPERTY") + (org-src--edit-bindings-string value)))) + (list + :varlist + (org-src--parse-edit-bindings sexp-str pos-beg pos-end) + :begin pos-beg :end pos-end)))))) + +(defun org-src--get-edit-bindings-from-header () + (let* ((element (org-element-at-point)) + (props (cadr element)) + (beg (plist-get props :begin)) + (end (plist-get props :end)) + (headers (org-element-property :header element)) + (bindings + (nreverse + (cl-mapcar + (lambda (header) + (when-let* ((sexp-str (org-src--edit-bindings-string header))) + (org-src--parse-edit-bindings sexp-str beg end))) + headers)))) + (list :varlist bindings :begin beg :end end))) + +(defun org-src--get-edit-bindings-for-subtree () + (save-excursion + (when-let* + ((entry-bindings + (and (ignore-errors (org-back-to-heading t)) + (org-entry-get (point) "EDIT_BINDINGS" 'selective)))) + (let ((beg (org-entry-beginning-position)) + (end (org-entry-end-position))) + (list + :varlist + ;; Use the string removing the initial quote character which + ;; is required for consistency with #+headers, as without + ;; them Babel causes errors. + (org-src--parse-edit-bindings (substring entry-bindings 1) beg end) + :begin beg :end end))))) + +(defun org-src--edit-bindings-string (property-value) + (let ((str + (save-match-data + ;; We include a quote in order to fool Babel, which parses + ;; headers too. + (and (string-match "^edit-bindings '(" property-value) + (string-trim (substring property-value (1- (match-end 0)))))))) + (unless (and str (string-empty-p str)) + str))) + +(defun org-src--parse-edit-bindings (sexp-str pos-beg pos-end) + ;; XXX: require cadr of the varlist items to be atoms, for security? + ;; Or prompt users? Because otherwise there can be complete + ;; programs embedded in there. + (cl-destructuring-bind + (sexp . final-string-index) + (read-from-string sexp-str) + (when sexp + ;; Do not allow trailing stuff. + '(unless (= (1+ (length sexp-str)) final-string-index) + (user-error "Junk after edit-bindings varlist at line %d" + (line-number-at-pos pos-beg t)))) + ;; XXX: Only allow static data, no function calls? + (cl-loop for varexp in sexp + collect + (pcase varexp + ((pred null) + (ignore varexp)) + (`(,name ,value) + `(,name . ,value)) + ((pred symbolp) + `(,varexp + . ',(buffer-local-value + varexp (current-buffer)))) + (_ (user-error "Erroneous expression in varlist: %S" + varexp)))))) + (defun org-src--edit-element (datum name &optional initialize write-back contents remote) "Edit DATUM contents in a dedicated buffer NAME. @@ -513,21 +697,28 @@ Leave point in edit buffer." (org-src-mode) ;; Move mark and point in edit buffer to the corresponding ;; location. - (if remote - (progn - ;; Put point at first non read-only character after - ;; leading blank. - (goto-char - (or (text-property-any (point-min) (point-max) 'read-only nil) - (point-max))) - (skip-chars-forward " \r\t\n")) - ;; Set mark and point. - (when mark-coordinates - (org-src--goto-coordinates mark-coordinates (point-min) (point-max)) - (push-mark (point) 'no-message t) - (setq deactivate-mark nil)) - (org-src--goto-coordinates - point-coordinates (point-min) (point-max))))))) + (prog1 + (if remote + (progn + ;; Put point at first non read-only character after + ;; leading blank. + (goto-char + (or (text-property-any (point-min) (point-max) 'read-only nil) + (point-max))) + (skip-chars-forward " \r\t\n")) + ;; Set mark and point. + (when mark-coordinates + (org-src--goto-coordinates mark-coordinates (point-min) (point-max)) + (push-mark (point) 'no-message t) + (setq deactivate-mark nil)) + (org-src--goto-coordinates + point-coordinates (point-min) (point-max))) + ;; Apply edit-bindings. + (when-let* ((edit-bindings + (with-current-buffer (org-src--source-buffer) + (org-src--collect-edit-bindings-for-element)))) + (org-src--apply-edit-bindings + (org-src--simplify-edit-bindings edit-bindings)))))))) \f diff --git a/testing/lisp/test-org-src.el b/testing/lisp/test-org-src.el index 86f08eccb..4406e5f02 100644 --- a/testing/lisp/test-org-src.el +++ b/testing/lisp/test-org-src.el @@ -1,8 +1,10 @@ ;;; test-org-src.el --- tests for org-src.el +;; Copyright (C) 2018 Göktuğ Kayaalp ;; Copyright (C) 2012-2015 Le Wang -;; Author: Le Wang <l26wang at gmail dot com> +;; Authors: Le Wang <l26wang at gmail dot com> +;; Göktuğ Kayaalp <self at gkayaalp.com> ;; This file is not part of GNU Emacs. @@ -480,6 +482,174 @@ This is a tab:\t. (should (equal "#" (org-unescape-code-in-string "#"))) (should (equal "," (org-unescape-code-in-string ",")))) +(ert-deftest test-org-src/edit-bindings-parser () + "Test edit-bindings parser." + ;; ‘org-src--edit-bindings-string’ + (should (null (org-src--edit-bindings-string "not-edit-bindings '(whatever)"))) + (should (string= (org-src--edit-bindings-string "edit-bindings '()") + "()")) + (should (string= (org-src--edit-bindings-string "edit-bindings '(fill-column)") + "(fill-column)")) + (should (string= + (org-src--edit-bindings-string "edit-bindings '((fill-column 80))") + "((fill-column 80))")) + ;; ‘org-src--parse-edit-bindings’ + (should (equal (org-src--parse-edit-bindings "((lexical-binding t))" 0 0) + '((lexical-binding . t)))) + (should (equal (org-src--parse-edit-bindings "(lexical-binding)" 0 0) + '((lexical-binding . 'nil)))) + ;; ‘org-src--collect-global-edit-bindings’ + (org-test-with-temp-text + "<point>#+property: edit-bindings '()" + (should (null (plist-get (org-src--collect-global-edit-bindings) :varlist)))) + (org-test-with-temp-text + "<point>#+property: edit-bindings '(())" + (should (null (plist-get (org-src--collect-global-edit-bindings) :varlist)))) + (org-test-with-temp-text + "<point>#+property: edit-bindings '(major-mode)" + (let ((ret (car (org-src--collect-global-edit-bindings)))) + (should (equal '((major-mode . 'org-mode)) (plist-get ret :varlist))))) + ;; ‘org-src--get-edit-bindings-from-header’ + (org-test-with-temp-text + "<point>#+header: edit-bindings '(major-mode) +#+BEGIN_EXPORT latex +#+END_EXPORT" + (let ((ret (org-src--get-edit-bindings-from-header))) + (should (equal (plist-get ret :varlist) + '(((major-mode . 'org-mode))))))) + (org-test-with-temp-text + "<point>#+header: edit-bindings '(major-mode) +#+header: edit-bindings '((xxx 12)) +#+BEGIN_EXPORT latex +#+END_EXPORT" + (let ((ret (org-src--get-edit-bindings-from-header))) + (should (equal (plist-get ret :varlist) + '(((major-mode . 'org-mode)) + ((xxx . 12)))))))) + +(ert-deftest test-org-src/collect-edit-bindings-for-element () + "Collecting edit-bindings settings scoped to an element." + (org-test-with-temp-text + "<point>#+header: edit-bindings '(major-mode) +#+BEGIN_SRC elisp +#+END_SRC" + (cl-destructuring-bind + (_1 elem _2 subtree _3 global) + (org-src--collect-edit-bindings-for-element) + (should (equal (car (plist-get elem :varlist)) '((major-mode . 'org-mode)))) + (should (null (plist-get subtree :varlist))) + (should (null (plist-get global :varlist))))) + (org-test-with-temp-text + "#+property: edit-bindings '((fill-column 38)) +<point>#+header: edit-bindings '(major-mode) +#+BEGIN_SRC elisp +#+END_SRC" + (cl-destructuring-bind + (_1 elem _2 subtree _3 global) + (org-src--collect-edit-bindings-for-element) + (should (equal (plist-get elem :varlist) '(((major-mode . 'org-mode))))) + (should (equal (plist-get (car global) :varlist) '((fill-column . 38)))) + (should (null (plist-get subtree :varlist))))) + (org-test-with-temp-text + "#+property: edit-bindings '((fill-column 38)) +<point>#+header: edit-bindings '(major-mode) +#+BEGIN_SRC elisp +#+END_SRC +#+property: edit-bindings '((lexical-let 'orly))" + (cl-destructuring-bind + (_1 elem _2 subtree _3 global) + (org-src--collect-edit-bindings-for-element) + (should (equal (plist-get elem :varlist) '(((major-mode . 'org-mode))))) + (should (equal (plist-get (car global) :varlist) '((fill-column . 38)))) + (should (null subtree)))) + (org-test-with-temp-text + "#+property: edit-bindings '((fill-column 38)) +* A subtree! +:PROPERTIES: +:EDIT_BINDINGS: '((fill-column 47)) +:END: +<point>#+header: edit-bindings '(major-mode) +#+BEGIN_SRC elisp +#+END_SRC +#+property: edit-bindings '((lexical-binding 'orly))" + (cl-destructuring-bind + (_1 elem _2 subtree _3 global) + (org-src--collect-edit-bindings-for-element) + (should (equal (plist-get elem :varlist) '(((major-mode . 'org-mode))))) + (should (equal (plist-get subtree :varlist) '((fill-column . 47)))) + (should (equal (plist-get (car global) :varlist) '((fill-column . 38)))) + (should + (not (equal (plist-get (car global) :varlist) + '((lexical-binding . 'orly)))))))) + +(ert-deftest test-org-src/edit-bindings-precedence-and-application () + "Test handling of scope precedence, and application of bindings." + (let ((case1 (list :element-bindings '(:varlist (((a . 'b) (c . 'd)))) + :subtree-bindings '(:varlist ((c . 'e) (p . 'q))) + :global-bindings '((:varlist ((z . 'o) (k . 'f))) + (:varlist ((q . 'u) (x . 'y)))))) + (case1-expected + '((z . 'o) (k . 'f) (q . 'u) (x . 'y) ;global + (p . 'q) ;subtree + (a . 'b) (c . 'd) ;element + )) + (case2 (list :element-bindings '(:varlist (((a . 'b)))) + :global-bindings '((:varlist ((a . 'z))))))) + ;; Precedence: + (should (equal (org-src--simplify-edit-bindings nil) nil)) + (should + (equal (org-src--simplify-edit-bindings case1) + case1-expected)) + (should + (equal (org-src--simplify-edit-bindings case2) + '((a . 'b)))) + ;; Application: + (with-temp-buffer + (org-src--apply-edit-bindings + (org-src--simplify-edit-bindings case2)) + (should (equal (buffer-local-value 'a (current-buffer)) 'b))) + (with-temp-buffer + (org-src--apply-edit-bindings + (org-src--simplify-edit-bindings case1)) + (pcase-dolist (`(,var . ,val) case1-expected) + (should (equal (buffer-local-value var (current-buffer)) + (eval val))))))) + +(ert-deftest test-org-src/edit-bindings-use-cases () + "Test possible uses of edit bindings feature." + (org-test-with-temp-text + "<point>#+header: edit-bindings '((xxx t)) +#+BEGIN_SRC elisp +#+END_SRC" + (org-edit-special) + (should + (equal (buffer-local-value 'xxx (current-buffer)) t)) + (org-edit-src-exit)) + (org-test-with-temp-text + "#+property: edit-bindings '((xxx 12) major-mode) +<point>#+header: edit-bindings '((xxx t)) +#+BEGIN_SRC elisp +#+END_SRC +#+property: edit-bindings '((xxx 13))" + (org-edit-special) + (should (equal (buffer-local-value 'xxx (current-buffer)) t)) + (should (equal (buffer-local-value 'major-mode (current-buffer)) 'org-mode)) + (org-edit-src-exit)) + (org-test-with-temp-text + "#+property: edit-bindings '((xxx 12)) +* header +:properties: +:edit_bindings: '((zzz 34)) +:end: +#+property: edit-bindings '((zzz 413)) +<point>#+header: edit-bindings '((xxx t)) +#+BEGIN_SRC elisp +#+END_SRC +#+property: edit-bindings '((xxx 13))" + (org-edit-special) + (should (equal (buffer-local-value 'xxx (current-buffer)) t)) + (should (equal (buffer-local-value 'zzz (current-buffer)) 34)) + (org-edit-src-exit))) (provide 'test-org-src) ;;; test-org-src.el ends here -- 2.11.0 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: Inheriting some local variables from source code block editing buffers 2018-05-18 21:48 ` Göktuğ Kayaalp @ 2018-05-19 12:26 ` Nicolas Goaziou 2018-05-21 14:20 ` Aaron Ecay 1 sibling, 0 replies; 28+ messages in thread From: Nicolas Goaziou @ 2018-05-19 12:26 UTC (permalink / raw) To: Göktuğ Kayaalp; +Cc: Aaron Ecay, emacs-orgmode Hello, Göktuğ Kayaalp <self@gkayaalp.com> writes: > And here it is. Please find the patch attached. Thank you. Comments follow. > I have ran the entire test suite against this, which completed w/ > 6 failures, seemingly unrelated (they fail on revision 58c9bedfd too); You may be using an old revision. I see no error from tests in HEAD. > find the list below. I have tried to follow apparent conventions in > each file w.r.t. the copyright/authors lines, sorry if I modified them > unnecessarily. Usually, we do not modify authors lines, which represents the people who initially wrote the file. No worries, though. > Subject: [PATCH] Implement edit bindings feature > > Enable defining local variable bindings to be applied when editing > source code. > > * lisp/org-src.el (org-src--apply-edit-bindings) > (org-src--simplify-edit-bindings) > (org-src--parse-edit-bindings) > (org-src--edit-bindings-string) > (org-src--get-edit-bindings-for-subtree) > (org-src--get-edit-bindings-from-header) > (org-src--collect-global-edit-bindings) > (org-src--collect-edit-bindings-for-element): New functions. > (org-src-apply-risky-edit-bindings): New defcustom. > (org-src--edit-element): > * doc/org.texi (Editing source code): Add edit bindings. "org.texi" no longer exists in master. We write the manual as an Org file: "org-manual.org". > +It is possible to define local variable bindings for these buffers using the > +@samp{edit-bindings} element header, the @samp{edit-bindings} buffer > +property, or the @samp{EDIT_BINDINGS} entry property. All three can be used > +together, the bindings from the header override those of the subtree, and > +they both override the bindings from buffer properties. The syntax is > +similar to that of @code{let} varlists, but a sole symbol means the > +variable's value is copied from the Org mode buffer. Multiple uses of > +@samp{edit-bindings} headers and buffer properties are supported, and works > +like @code{let*}. Entry property @samp{EDIT_BINDINGS} can not be repeated. > +Below is an example: It seems you are partly re-inventing the wheel here. Org already has such mechanism, called Babel headers: #+PROPERTY: header-args :edit-bindings '(...) :PROPERTIES: :HEADER-ARGS: :edit-bindings '(...) :END: #+header: :edit-bindings '(...) ... Of course, this is currently limited to source blocks and inline source blocks. However, I think it is better to extend them to your use-case than rewriting something different, albeit similar. You already looked at `org-babel-get-src-block-info'. You may want to start hacking on it and extend it instead of providing your own tools. > ** New features > +*** Set local variables for editing blocks > +Bindings from =edit-bindings= header and buffer property, and > +=EDIT_BINDINGS= entry property are applied in Org Src buffers. For > +example, when editing the following code block with > +~org-edit-special~: AFAIU, these bindings are not used when evaluating the buffer, which may be surprising. What about calling them :local-bindings instead of :edit-bindings and handle them in Babel too? In this case, what would be the difference with, e.g., :var lexical-binding=t > +(defun org-src--apply-edit-bindings (simplified-bindings) Could you provide a docstring for the functions? > + (pcase-dolist (`(,name . ,value) simplified-bindings) > + (let ((prompt-apply > + (concat "Setting risky local variable ‘%S’" > + " in edit-special buffer, its value is: %S; Continue?")) You can use \ + \n, i.e., continuation string instead of calling `concat' each time. > + (risky-message "%s risky local variable ‘%S’ in edit-special buffer.") I suggest to use two different messages instead of doing the above, if one day we move to proper i18n. > + (apply-binding (lambda () (set (make-local-variable name) > + (eval value))))) > + (unless > + (and > + (risky-local-variable-p name) > + (cond ((or (and (eq org-src-apply-risky-edit-bindings 'ask) > + (y-or-n-p (format prompt-apply name value))) > + (eq org-src-apply-risky-edit-bindings 'apply-silent)) > + (funcall apply-binding)) > + (org-src-apply-risky-edit-bindings > + (prog1 > + (funcall apply-binding) > + (message risky-message "Applied" name))) You probably mean (funcall apply-binding) (message ...) t which is clearer, IMO. > + ((not org-src-apply-risky-edit-bindings) > + (message risky-message "Skipped" name)) > + ((eq org-src-apply-risky-edit-bindings 'skip-silent)) > + ('else > + (user-error > + "Unexpected value for ‘%S’, will not apply this or any more bindings." > + 'org-src-apply-risky-edit-bindings)))) Error messages must not end with a period. > + (funcall apply-binding))))) If the second `cond' branch is used, `apply-binding' is called twice, which is sub-optimal. > +(defun org-src--simplify-edit-bindings (raw-bindings) This function really needs a docstring. > +(defun org-src--collect-edit-bindings-for-element () This is where the re-inventing the wheel part starts. > +(defun org-src--collect-global-edit-bindings () Ditto. > + ;; XXX: is setting GRANULARITY to 'element a performance > + ;; improvement, and does it cause any problems over just using the > + ;; default 'object? Yes, setting GRANULARITY to `element' is faster than setting it to `object', but you shouldn't parse the whole buffer in the first place. > + ;; Also, is it possible to not have to parse the entire buffer every > + ;; time? Of course. You usually look for a regexp, e.g., "#+\\+KEYWORD:", then check if you're really at a keyword with (eq (org-element-type element) 'keyword), then process the keyword. > + (cl-loop for varexp in sexp > + collect > + (pcase varexp > + ((pred null) -> ('nil ...) > +;; Copyright (C) 2018 Göktuğ Kayaalp > ;; Copyright (C) 2012-2015 Le Wang Actually, copyright is wrong. It belongs to FSF, since tests are probably going to be included in Emacs, too. Regards, -- Nicolas Goaziou 0x80A93738 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Inheriting some local variables from source code block editing buffers 2018-05-18 21:48 ` Göktuğ Kayaalp 2018-05-19 12:26 ` Nicolas Goaziou @ 2018-05-21 14:20 ` Aaron Ecay 1 sibling, 0 replies; 28+ messages in thread From: Aaron Ecay @ 2018-05-21 14:20 UTC (permalink / raw) To: Göktuğ Kayaalp, Nicolas Goaziou; +Cc: emacs-orgmode Hi Göktuğ, A couple of comments: 2018ko maiatzak 18an, Göktuğ Kayaalp-ek idatzi zuen: [...] > +(defcustom org-src-apply-risky-edit-bindings 'ask > + "What to do if an edit binding is a risky local variable. > +If this is nil, bindings that satisfy ‘risky-local-variable-p’ > +are skipped, with a warning message. Otherwise, its value should > +be a symbol telling how to thread them. Possible values of this > +setting are: > + > +nil Skip, warning the user via a message. > +skip-silent Skip risky local varibles silently. > +ask Prompt user for each variable. > +t Apply the variable but show a warning. > +apply-silent Apply risky local variables silently." It would be more consistent/less confusing to use skip-warn and apply-warn instead of nil and t. It would also lead to fewer bugs in the sense that: [...] > + (cond ((or (and (eq org-src-apply-risky-edit-bindings 'ask) > + (y-or-n-p (format prompt-apply name value))) > + (eq org-src-apply-risky-edit-bindings 'apply-silent)) > + (funcall apply-binding)) > + (org-src-apply-risky-edit-bindings > + (prog1 > + (funcall apply-binding) > + (message risky-message "Applied" name))) > + ((not org-src-apply-risky-edit-bindings) > + (message risky-message "Skipped" name)) > + ((eq org-src-apply-risky-edit-bindings 'skip-silent)) > + ('else > + (user-error > + "Unexpected value for ‘%S’, will not apply this or any more bindings." > + 'org-src-apply-risky-edit-bindings)))) If I am reading the above cond correctly, then the (eq org-src-apply-risky-edit-bindings 'skip-silent) branch will never be reached, because it will be eaten by the second branch. (And so will the “else” branch, for that matter.) IMO you should use cl-case instead of cond, which will be less conducive to subtle problems like this and make it clear that the possible values are exhaustively handled. (That approach will mean shuffling the y-or-n-p stuff around slightly). [...] > +(defun org-src--parse-edit-bindings (sexp-str pos-beg pos-end) > + ;; XXX: require cadr of the varlist items to be atoms, for security? > + ;; Or prompt users? Because otherwise there can be complete > + ;; programs embedded in there. Maybe instead of using risky-local-variable-p above, this feature should use safe-local-variable-p instead. Then we wouldnʼt have to worry about the security implications of allowing non-atom values. It seems like a version of this feature that only worked for atoms would be quite limited in functionality. In that case, we should probably call the defcustom above ...apply-unsafe-edit-bindings for the sake of accuracy. -- Aaron Ecay ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Inheriting some local variables from source code block editing buffers 2018-05-14 5:44 ` Göktuğ Kayaalp 2018-05-14 12:13 ` Nicolas Goaziou @ 2018-05-14 13:33 ` Aaron Ecay 2018-05-14 16:46 ` Göktuğ Kayaalp 1 sibling, 1 reply; 28+ messages in thread From: Aaron Ecay @ 2018-05-14 13:33 UTC (permalink / raw) To: Göktuğ Kayaalp, emacs-orgmode Hi Göktuğ, This patch looks good, thanks. Of course, for merging to org core it will need to be an actual patch and not advice. There is also copyright assignment to think of. Do you already have a FSF copyright assignment on file? 2018ko maiatzak 14an, Göktuğ Kayaalp-ek idatzi zuen: [...] > One ‘gotcha’ is that :edit-bindings requires a quoted list whereas the > explicit quote is not necessary with ATTR_EDIT: > > #+BEGIN_SRC elisp :edit-bindings '((lexical-binding t)) > #+ATTR_EDIT: ((lexical-binding t)) That quote is required for the src block version is inherent in the design of babel. For consistency, you could require (or at least permit without requiring) a quote in the other case as well. > > Another problem is that I was not able to define a new element property > named EDIT_BINDINGS, and had to take the shortcut with naming it as an > ATTR_* variable. Preferably, it'd be EDIT_BINDINGS instead: > > #+BEGIN_SRC elisp :edit-bindings '((lexical-binding t)) > #+EDIT_BINDINGS: ((lexical-binding t)) > > But personally I don't think it's that big of a problem. > > > The advice: > > (define-advice org-src--edit-element > (:around > (fn datum &rest args) > attr-edit&edit-bindings) > "Apply edit-special bindings." > (let ((attr-edit (org-element-property :attr_edit datum)) > (edit-bindings > (assoc :edit-bindings (caddr (org-babel-get-src-block-info nil datum)))) > (source-buffer (current-buffer)) > (sub (lambda (varlist source-buffer) > (let (var val) > (dolist (form varlist) > ;; If it's a symbol, inherit from the Org mode buffer. > (if (symbolp form) > (setq var form > val (with-current-buffer source-buffer (eval var))) > ;; Else, apply the specified value. > (setq var (car form) val (cadr form))) > (unless (symbolp var) > ;; XXX: maybe tell where it is? > (user-error "Bad varlist at ATTR_EDIT")) > (set (make-local-variable var) val)))))) I think you could replace the (let (var val)...) form with: #+begin_src emacs-lisp (pcase-dolist ((or (and (pred symbolp) var (let val (buffer-local-value var source-buffer))) `(,var ,val)) varlist) (set (make-local-variable var) val)) #+end_src This silently skips varlist entries that are of the wrong shape, but it would be possible to make it raise an error as in your version. I like the pcase version better because itʼs shorter and has fewer nested conditionals, but itʼs ultimately a matter of taste. -- Aaron Ecay ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Inheriting some local variables from source code block editing buffers 2018-05-14 13:33 ` Aaron Ecay @ 2018-05-14 16:46 ` Göktuğ Kayaalp 0 siblings, 0 replies; 28+ messages in thread From: Göktuğ Kayaalp @ 2018-05-14 16:46 UTC (permalink / raw) To: emacs-orgmode On 2018-05-14 14:33 +01, Aaron Ecay <aaronecay@gmail.com> wrote: > Hi Göktuğ, > > This patch looks good, thanks. Of course, for merging to org core it > will need to be an actual patch and not advice. Certainly; this is meant as a ‘tangible’ example, and can easily be turned into a patch if you want to merge this. > There is also copyright > assignment to think of. Do you already have a FSF copyright assignment > on file? I have signed the assignment for GNU Emacs, and contributed a couple patches. IDK if that can be reused here though, but if not I can do the paperwork again. > 2018ko maiatzak 14an, Göktuğ Kayaalp-ek idatzi zuen: > > [...] > >> One ‘gotcha’ is that :edit-bindings requires a quoted list whereas the >> explicit quote is not necessary with ATTR_EDIT: >> >> #+BEGIN_SRC elisp :edit-bindings '((lexical-binding t)) >> #+ATTR_EDIT: ((lexical-binding t)) > > That quote is required for the src block version is inherent in the > design of babel. For consistency, you could require (or at least permit > without requiring) a quote in the other case as well. I guess requiring that would be better given it can cause confusion if it works one way but not the other. > I think you could replace the (let (var val)...) form with: > > #+begin_src emacs-lisp > (pcase-dolist ((or (and (pred symbolp) var > (let val (buffer-local-value var source-buffer))) > `(,var ,val)) > varlist) > (set (make-local-variable var) val)) > #+end_src > > This silently skips varlist entries that are of the wrong shape, but it > would be possible to make it raise an error as in your version. I like > the pcase version better because itʼs shorter and has fewer nested > conditionals, but itʼs ultimately a matter of taste. Yeah I can integrate this. I was initially going to use pcase, but I can't understand how it works for the life of me. It's one of the two lisp macros together with loop that I can't get my head around. So, I will turn this into a patch, make the change regarding the initial quote, use pcase, and see how Nicolas responds to me about the #+ATTR_EDIT. -- İ. Göktuğ Kayaalp <https://www.gkayaalp.com/> 024C 30DD 597D 142B 49AC 40EB 465C D949 B101 2427 ^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2018-05-21 14:21 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-29 17:19 Inheriting some local variables from source code block editing buffers Göktuğ Kayaalp
2018-04-29 22:09 ` Bastien
2018-05-01 3:30 ` Göktuğ Kayaalp
2018-05-01 8:43 ` Nicolas Goaziou
2018-05-01 8:45 ` Nicolas Goaziou
2018-05-01 11:41 ` Göktuğ Kayaalp
2018-05-01 11:55 ` Nicolas Goaziou
[not found] ` <ygmvac7lcl1.fsf@gkayaalp.com>
2018-05-01 14:00 ` Nicolas Goaziou
2018-05-01 16:37 ` Aaron Ecay
[not found] ` <ygmin87kwua.fsf@gkayaalp.com>
2018-05-01 19:35 ` Aaron Ecay
2018-05-01 20:04 ` Göktuğ Kayaalp
2018-05-01 20:53 ` Aaron Ecay
2018-05-01 22:12 ` Göktuğ Kayaalp
2018-05-01 22:19 ` Aaron Ecay
2018-05-01 22:26 ` Göktuğ Kayaalp
2018-05-02 10:16 ` Nicolas Goaziou
2018-05-02 19:52 ` Göktuğ Kayaalp
2018-05-01 11:45 ` Göktuğ Kayaalp
2018-05-14 5:44 ` Göktuğ Kayaalp
2018-05-14 12:13 ` Nicolas Goaziou
2018-05-14 16:34 ` Göktuğ Kayaalp
2018-05-14 16:47 ` Nicolas Goaziou
2018-05-15 18:36 ` Göktuğ Kayaalp
2018-05-18 21:48 ` Göktuğ Kayaalp
2018-05-19 12:26 ` Nicolas Goaziou
2018-05-21 14:20 ` Aaron Ecay
2018-05-14 13:33 ` Aaron Ecay
2018-05-14 16:46 ` Göktuğ Kayaalp
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).