* [ANN] lisp/ob-tangle-sync.el @ 2023-04-26 14:48 Mehmet Tekman 2023-04-26 16:43 ` John Wiegley ` (2 more replies) 0 siblings, 3 replies; 77+ messages in thread From: Mehmet Tekman @ 2023-04-26 14:48 UTC (permalink / raw) To: emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 2423 bytes --] Dear fellow org-users, I would like to contribute some a new library into org-mode, which performs automatic synchronization between tangled files and their org-mode source blocks via a global minor mode `org-babel-tangle-sync-mode' which uses the after-save-hook. Full disclosure: I created a MELPA package with similar functionality 3 years ago, but it did not use the existing org libraries much and needlessly reinvented many wheels: https://gitlab.com/mtekman/org-tanglesync.el This is a complete (and very concise) rewrite that uses the org framework and that I wish to incorporate into org-mode itself. My changes to the org-mode main branch can be found here: https://gitlab.com/mtekman/org-mode (ob-tangle-sync branch) A toy example of what this does would be as follows: - Source =emacs_conf= org-mode file #+begin_src bash :comments yes :tangle ~/.bashrc export PATH=$PATH:/opt/bin #+end_src - Tangled =~/.bashrc= file #+begin_src bash # [[file:repos/_mtekman/emacs_conf.org::*bashrc][bashrc:1]] export PATH=$PATH:/opt/bin #+end_src By activating =org-babel-tangle-sync-mode=, I can edit either of those buffers and every time that I save it would automatically update the other. If I add the header argument =:tangle-sync <action>= then I can specify an action of: - skip :: do nothing, just save the buffer, even if the sync mode is active - pull :: only pull changes from =~/.bashrc= into the Emacs_conf org-mode file - export :: only export changes from Emacs_conf to =~/.bashrc=, even if called from =~/.bashrc= - both :: (default) synchronize freely between tangled file and source block (the is also the nil value) By default, the mode acts on any org-mode source block with =:tangle= headers and any tangled file with file comments. This can be changed by setting a list of "sync files" via =org-babel-tangle-sync-files= which only acts if the source block exists in a file given in that custom variable. I'm currently trying to write tests for the library, but I would also greatly welcome any feedback and comments on the currently written code (attached as a diff, and also available in the above org-mode repo) As this is also my first time submitting directly to GNU Emacs, I, um, might also need some hand-holding. I think I've followed the main org-contribute guidelines, but I've likely missed or glossed over a step or two. Cheerful regards, Mehmet [-- Attachment #2: lisp_ob-tangle-sync.el --] [-- Type: text/x-emacs-lisp, Size: 7952 bytes --] diff --git a/lisp/ob-tangle-sync.el b/lisp/ob-tangle-sync.el new file mode 100644 index 000000000..61c23f647 --- /dev/null +++ b/lisp/ob-tangle-sync.el @@ -0,0 +1,172 @@ +;;; ob-tangle-sync.el --- Synchronize Source Code and Org Files -*- lexical-binding: t; -*- + +;; Copyright (C) 2009-2023 Free Software Foundation, Inc. + +;; Author: Mehmet Tekman +;; Keywords: literate programming, reproducible research +;; URL: https://orgmode.org + +;; This file is part of GNU Emacs. + +;; GNU Emacs is free software: you can redistribute it and/or modify +;; it under the terms of the GNU General Public License as published by +;; the Free Software Foundation, either version 3 of the License, or +;; (at your option) any later version. + +;; GNU Emacs is distributed in the hope that it will be useful, +;; but WITHOUT ANY WARRANTY; without even the implied warranty of +;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +;; GNU General Public License for more details. + +;; You should have received a copy of the GNU General Public License +;; along with GNU Emacs. If not, see <https://www.gnu.org/licenses/>. + +;;; Commentary: + +;; Synchronize the code between source blocks and raw source-code files. + +;;; Code: + +(require 'org-macs) +(org-assert-version) + +(require 'ol) +(require 'org) +(require 'org-element) +(require 'ob-core) + +(defgroup org-babel-tangle-sync nil + "Options for synchronizing source code and code blocks." + :tag "Org Babel Tangle sync" + :group 'org-babel-tangle) + +;;;###autoload +(define-minor-mode org-babel-tangle-sync-mode + "Global minor mode that synchronizes tangled files after every save." + :global t + :interactive t + :lighter " o-ts" + (if org-babel-tangle-sync-mode + (add-hook 'after-save-hook 'org-babel-tangle-sync-synchronize nil t) + (remove-hook 'after-save-hook 'org-babel-tangle-sync-synchronize t))) + +(defcustom org-babel-tangle-sync-files nil + "A list of `org-mode' files. +When `org-babel-tangle-sync-mode' is enabled only files listed +here are subject to the org-babel-tangle-sync treatment. If nil, +then all org files with tangle headers are considered." + :group 'org-babel-tangle-sync + :type 'list + :package-version '(Org . "9.6.5") + :set (lambda (_var val) (mapcar #'(lambda (x) (expand-file-name x)) val))) + + +(defun org-babel-tangle-sync--babel-tangle-jump (link block-name) + "Jump from a tangled file to the Org file without returning anything. +The location of the code block in the Org file is given by a +combination of the LINK filename and header, followed by the +BLOCK-NAME Org mode source block number. The code is borrowed +heavily from `org-babel-tangle-jump-to-org'" + ;; Go to the beginning of the relative block in Org file. + ;; Explicitly allow fuzzy search even if user customized + ;; otherwise. + (let (org-link-search-must-match-exact-headline) + (org-link-open-from-string link)) + ;;(setq target-buffer (current-buffer)) + (if (string-match "[^ \t\n\r]:\\([[:digit:]]+\\)" block-name) + (let ((n (string-to-number (match-string 1 block-name)))) + (if (org-before-first-heading-p) (goto-char (point-min)) + (org-back-to-heading t)) + ;; Do not skip the first block if it begins at point min. + (cond ((or (org-at-heading-p) + (not (eq (org-element-type (org-element-at-point)) + 'src-block))) + (org-babel-next-src-block n)) + ((= n 1)) + (t (org-babel-next-src-block (1- n))))) + (org-babel-goto-named-src-block block-name)) + (goto-char (org-babel-where-is-src-block-head)) + (forward-line 1)) + +;;;###autoload +(defun org-babel-tangle-sync-synchronize () + "Synchronize a tangled code block to its source-specific file, or vice versa. +If the cursor is either within the source file or in destination +tangled file, perform a desired tangling action. The tangling +action by default is to detangle the tangled files' changes back +to its source block, or to tangle the source block to its tangled +file. Actions are one of `skip' (no action), `pull' (detangle +only), `export' (tangle only), and `both' (default, synchronize +in both directions). All `org-mode' source blocks and all tangled +files with comments are considered valid targets, unless +specified otherwise by `org-babel-tangle-sync-files'." + (interactive) + (let* ((link (save-excursion + (progn (re-search-backward org-link-bracket-re nil t) + (match-string-no-properties 0)))) + (block-name (match-string 2)) + (orgfile-p (string= major-mode "org-mode")) + (tangled-file-p (and link (not orgfile-p)))) + + ;; Tangled File → Source Block + (if tangled-file-p + ;; Examine the block: Get the source file and the desired tangle-sync action + (let* ((parsed-link (with-temp-buffer + (let ((org-inhibit-startup nil)) + (insert link) + (org-mode) + (goto-char (point-min)) + (org-element-link-parser)))) + (source-file (expand-file-name + (org-element-property :path parsed-link))) + (sync-action (save-window-excursion + (progn + (org-babel-tangle-sync--babel-tangle-jump link block-name) + (alist-get :tangle-sync + (nth 2 (org-babel-get-src-block-info + 'no-eval))))))) + ;; De-tangle file back to source block if: + ;; - member of sync file list (or list is empty) + ;; - source file tangle-sync action isn't "skip" or "export", + (if (or (null org-babel-tangle-sync-files) + (member source-file org-babel-tangle-sync-files)) + (cond ((string= sync-action "skip") nil) + ((string= sync-action "export") + (save-window-excursion + (progn (org-babel-tangle-sync--babel-tangle-jump link block-name) + (let ((current-prefix-arg '(16))) + (call-interactively 'org-babel-tangle)) + (message "Exported from %s" source-file)))) + (t + (save-window-excursion + (org-babel-detangle) + (message "Synced to %s" source-file)))))) + + ;; Source Block → Tangled File (or Source Block ← Tangled File (via "pull")) + (when orgfile-p + ;; Tangle action of Source file on Block if: + ;; - member of sync file list (or list is empty) + ;; Actions + ;; - pull (Source Block ← File) + ;; - skip (nothing) + ;; - export, both, nil (Source Block → File) + (if (or (null org-babel-tangle-sync-files) + (member buffer-file-name org-babel-tangle-sync-files)) + + (let* ((src-headers (nth 2 (org-babel-get-src-block-info 'no-eval))) + (tangle-file (cdr (assq :tangle src-headers))) + (tangle-action (alist-get :tangle-sync src-headers))) + (when tangle-file + (cond ((string= tangle-action "pull") (save-excursion + (org-babel-detangle tangle-file))) + ((string= tangle-action "skip") nil) + (t (let ((current-prefix-arg '(16))) + (call-interactively 'org-babel-tangle) + ;; Revert to see changes, then re-enable the mode + (with-current-buffer (get-file-buffer tangle-file) + (revert-buffer) + (org-babel-tangle-sync-mode t)))))))))))) + +(provide 'ob-tangle-sync) + +;;; ob-tangle-sync.el ends here ^ permalink raw reply related [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-04-26 14:48 [ANN] lisp/ob-tangle-sync.el Mehmet Tekman @ 2023-04-26 16:43 ` John Wiegley 2023-04-26 18:43 ` Mehmet Tekman 2023-04-27 2:55 ` Ruijie Yu via General discussions about Org-mode. 2023-04-27 12:02 ` Ihor Radchenko 2 siblings, 1 reply; 77+ messages in thread From: John Wiegley @ 2023-04-26 16:43 UTC (permalink / raw) To: Mehmet Tekman; +Cc: emacs-orgmode >>>>> "MT" == Mehmet Tekman <mtekman89@gmail.com> writes: MT> - skip :: do nothing, just save the buffer, even if the sync mode is MT> active MT> - pull :: only pull changes from =~/.bashrc= into the Emacs_conf org-mode file MT> - export :: only export changes from Emacs_conf to =~/.bashrc=, even if called MT> from =~/.bashrc= For the sake of consistency, perhaps import and export, rather than pull and export? And, great idea! -- John Wiegley GPG fingerprint = 4710 CF98 AF9B 327B B80F http://newartisans.com 60E1 46C4 BD1A 7AC1 4BA2 ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-04-26 16:43 ` John Wiegley @ 2023-04-26 18:43 ` Mehmet Tekman 0 siblings, 0 replies; 77+ messages in thread From: Mehmet Tekman @ 2023-04-26 18:43 UTC (permalink / raw) To: emacs-orgmode > For the sake of consistency, perhaps import and export, rather than pull and > export? And, great idea! Absolutely - unless the "pull" / "push" crowd wants to chime in! I'm also not 100% sold on the "both" action keyword. There, writing "sync" would make more sense, but I'm hesitant to write a =:tangle-sync sync= in the header. ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-04-26 14:48 [ANN] lisp/ob-tangle-sync.el Mehmet Tekman 2023-04-26 16:43 ` John Wiegley @ 2023-04-27 2:55 ` Ruijie Yu via General discussions about Org-mode. 2023-04-27 6:27 ` Mehmet Tekman 2023-04-27 12:02 ` Ihor Radchenko 2 siblings, 1 reply; 77+ messages in thread From: Ruijie Yu via General discussions about Org-mode. @ 2023-04-27 2:55 UTC (permalink / raw) To: Mehmet Tekman; +Cc: emacs-orgmode Mehmet Tekman <mtekman89@gmail.com> writes: > Dear fellow org-users, > > I would like to contribute some a new library into org-mode, which > performs automatic synchronization between tangled files and their > org-mode source blocks via a global minor mode > `org-babel-tangle-sync-mode' which uses the after-save-hook. Great idea! Some inline comments below. > diff --git a/lisp/ob-tangle-sync.el b/lisp/ob-tangle-sync.el > new file mode 100644 > index 000000000..61c23f647 > --- /dev/null > +++ b/lisp/ob-tangle-sync.el > @@ -0,0 +1,172 @@ > +;;; ob-tangle-sync.el --- Synchronize Source Code and Org Files -*- lexical-binding: t; -*- > + > +;; Copyright (C) 2009-2023 Free Software Foundation, Inc. > + > +;; Author: Mehmet Tekman > +;; Keywords: literate programming, reproducible research > +;; URL: https://orgmode.org > + > +;; This file is part of GNU Emacs. > + > +;; GNU Emacs is free software: you can redistribute it and/or modify > +;; it under the terms of the GNU General Public License as published by > +;; the Free Software Foundation, either version 3 of the License, or > +;; (at your option) any later version. > + > +;; GNU Emacs is distributed in the hope that it will be useful, > +;; but WITHOUT ANY WARRANTY; without even the implied warranty of > +;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +;; GNU General Public License for more details. > + > +;; You should have received a copy of the GNU General Public License > +;; along with GNU Emacs. If not, see <https://www.gnu.org/licenses/>. > + > +;;; Commentary: > + > +;; Synchronize the code between source blocks and raw source-code files. > + > +;;; Code: > + > +(require 'org-macs) > +(org-assert-version) > + > +(require 'ol) > +(require 'org) > +(require 'org-element) > +(require 'ob-core) > + > +(defgroup org-babel-tangle-sync nil > + "Options for synchronizing source code and code blocks." > + :tag "Org Babel Tangle sync" > + :group 'org-babel-tangle) > + > +;;;###autoload > +(define-minor-mode org-babel-tangle-sync-mode > + "Global minor mode that synchronizes tangled files after every save." > + :global t Is there possibility to have a local minor mode (without introducing too much code changes)? > + :interactive t I believe interactive is the default? > + :lighter " o-ts" > + (if org-babel-tangle-sync-mode > + (add-hook 'after-save-hook 'org-babel-tangle-sync-synchronize nil t) > + (remove-hook 'after-save-hook 'org-babel-tangle-sync-synchronize t))) > + > +(defcustom org-babel-tangle-sync-files nil > + "A list of `org-mode' files. > +When `org-babel-tangle-sync-mode' is enabled only files listed > +here are subject to the org-babel-tangle-sync treatment. If nil, > +then all org files with tangle headers are considered." > + :group 'org-babel-tangle-sync > + :type 'list > + :package-version '(Org . "9.6.5") > + :set (lambda (_var val) (mapcar #'(lambda (x) (expand-file-name x)) val))) You only need to say #'expand-file-name instead of the quoted lambda. Also, you need to set the variable, otherwise the variable `org-babel-tangle-sync-files' is undefined. What I have in mind is this: :set (lambda (var val) (set var (mapcar #'expand-file-name val))) > + > + > +(defun org-babel-tangle-sync--babel-tangle-jump (link block-name) > + "Jump from a tangled file to the Org file without returning anything. > +The location of the code block in the Org file is given by a > +combination of the LINK filename and header, followed by the > +BLOCK-NAME Org mode source block number. The code is borrowed > +heavily from `org-babel-tangle-jump-to-org'" > + ;; Go to the beginning of the relative block in Org file. > + ;; Explicitly allow fuzzy search even if user customized > + ;; otherwise. > + (let (org-link-search-must-match-exact-headline) > + (org-link-open-from-string link)) > + ;;(setq target-buffer (current-buffer)) > + (if (string-match "[^ \t\n\r]:\\([[:digit:]]+\\)" block-name) > + (let ((n (string-to-number (match-string 1 block-name)))) > + (if (org-before-first-heading-p) (goto-char (point-min)) > + (org-back-to-heading t)) > + ;; Do not skip the first block if it begins at point min. > + (cond ((or (org-at-heading-p) > + (not (eq (org-element-type (org-element-at-point)) > + 'src-block))) > + (org-babel-next-src-block n)) > + ((= n 1)) > + (t (org-babel-next-src-block (1- n))))) > + (org-babel-goto-named-src-block block-name)) > + (goto-char (org-babel-where-is-src-block-head)) > + (forward-line 1)) > + > +;;;###autoload > +(defun org-babel-tangle-sync-synchronize () > + "Synchronize a tangled code block to its source-specific file, or vice versa. > +If the cursor is either within the source file or in destination > +tangled file, perform a desired tangling action. The tangling > +action by default is to detangle the tangled files' changes back > +to its source block, or to tangle the source block to its tangled > +file. Actions are one of `skip' (no action), `pull' (detangle > +only), `export' (tangle only), and `both' (default, synchronize > +in both directions). All `org-mode' source blocks and all tangled > +files with comments are considered valid targets, unless > +specified otherwise by `org-babel-tangle-sync-files'." > + (interactive) > + (let* ((link (save-excursion > + (progn (re-search-backward org-link-bracket-re nil t) > + (match-string-no-properties 0)))) Here you don't have to use `progn' because it is implied from `save-excursion'. > + (block-name (match-string 2)) > + (orgfile-p (string= major-mode "org-mode")) > + (tangled-file-p (and link (not orgfile-p)))) > + > + ;; Tangled File → Source Block > + (if tangled-file-p > + ;; Examine the block: Get the source file and the desired tangle-sync action > + (let* ((parsed-link (with-temp-buffer > + (let ((org-inhibit-startup nil)) > + (insert link) > + (org-mode) > + (goto-char (point-min)) > + (org-element-link-parser)))) > + (source-file (expand-file-name > + (org-element-property :path parsed-link))) > + (sync-action (save-window-excursion > + (progn > + (org-babel-tangle-sync--babel-tangle-jump link block-name) > + (alist-get :tangle-sync > + (nth 2 (org-babel-get-src-block-info > + 'no-eval))))))) > + ;; De-tangle file back to source block if: > + ;; - member of sync file list (or list is empty) > + ;; - source file tangle-sync action isn't "skip" or "export", > + (if (or (null org-babel-tangle-sync-files) > + (member source-file org-babel-tangle-sync-files)) > + (cond ((string= sync-action "skip") nil) > + ((string= sync-action "export") > + (save-window-excursion > + (progn (org-babel-tangle-sync--babel-tangle-jump link block-name) > + (let ((current-prefix-arg '(16))) > + (call-interactively 'org-babel-tangle)) > + (message "Exported from %s" source-file)))) > + (t > + (save-window-excursion > + (org-babel-detangle) > + (message "Synced to %s" source-file)))))) > + > + ;; Source Block → Tangled File (or Source Block ← Tangled File (via "pull")) > + (when orgfile-p > + ;; Tangle action of Source file on Block if: > + ;; - member of sync file list (or list is empty) > + ;; Actions > + ;; - pull (Source Block ← File) > + ;; - skip (nothing) > + ;; - export, both, nil (Source Block → File) > + (if (or (null org-babel-tangle-sync-files) > + (member buffer-file-name org-babel-tangle-sync-files)) > + > + (let* ((src-headers (nth 2 (org-babel-get-src-block-info 'no-eval))) > + (tangle-file (cdr (assq :tangle src-headers))) > + (tangle-action (alist-get :tangle-sync src-headers))) > + (when tangle-file > + (cond ((string= tangle-action "pull") (save-excursion > + (org-babel-detangle tangle-file))) > + ((string= tangle-action "skip") nil) > + (t (let ((current-prefix-arg '(16))) > + (call-interactively 'org-babel-tangle) > + ;; Revert to see changes, then re-enable the mode > + (with-current-buffer (get-file-buffer tangle-file) > + (revert-buffer) > + (org-babel-tangle-sync-mode t)))))))))))) > + > +(provide 'ob-tangle-sync) > + > +;;; ob-tangle-sync.el ends here -- Best, RY [Please note that this mail might go to spam due to some misconfiguration in my mail server -- still investigating.] ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-04-27 2:55 ` Ruijie Yu via General discussions about Org-mode. @ 2023-04-27 6:27 ` Mehmet Tekman 2023-04-28 10:57 ` Ruijie Yu via General discussions about Org-mode. 0 siblings, 1 reply; 77+ messages in thread From: Mehmet Tekman @ 2023-04-27 6:27 UTC (permalink / raw) To: emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 2255 bytes --] Ruijie Yu <ruijie@netyu.xyz> writes: > Great idea! Some inline comments below. > > You only need to say #'expand-file-name instead of the quoted lambda. > Also, you need to set the variable, otherwise the variable > `org-babel-tangle-sync-files' is undefined. > > What I have in mind is this: > :set (lambda (var val) (set var (mapcar #'expand-file-name val))) > > Here you don't have to use `progn' because it is implied from `save-excursion'. > Thanks! I've made your changes, and I've also incorporated John Wiegley's comments about using "import" instead of "pull" as a tangle-sync action word (small diff attached). I've also written up my changes in the ~etc/ORG-NEWS~ and targeted my custom variable for the 9.7 release (diff attached). > Is there possibility to have a local minor mode (without introducing too > much code changes)? I initially tried it this way, but the problem is that an org source block buffer might be in sync-mode, but it's corresponding tangle file might not be, making any changes asymmetric. Another issue is in order to see the changes in the tangled file, the tangle buffer needs to be reverted (with user prompt) which then switches off the sync-mode for that buffer on reload. One way around this (and it's something I implemented 3 years ago in my messy org-tanglesync[0] MELPA code) is to set an explicit list of "sync files", and then for Emacs to parse every =:tangle= header in a given file when loaded (via =org-src-mode-hook=) to create an alist of config files and their associated tangled files[1], such as =((file1.conf . (tanglefile1.txt tanglefile2.txt etc)))=. Then, for example, when ~tanglefile1.txt~ is loaded, Emacs knows that it should load the sync-mode too. This approach works reasonably well when the "sync files" list is mandatory, but it's also prone to errors if a sync file is edited and the alist of config files isn't updated, and the user would also lose the flexibility of having ~ob-tangle-sync~ function everywhere. I think a global minor mode is really elegant in this regard and I wish I knew about it 3 years ago! Best, Mehmet 0: https://gitlab.com/mtekman/org-tanglesync.el 1: https://gitlab.com/mtekman/org-tanglesync.el/-/blob/master/org-tanglesync.el#L400-L410 [-- Attachment #2: etc_ORG-NEWS.diff --] [-- Type: text/x-patch, Size: 1543 bytes --] diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS index 03894f128..29f60c755 100644 --- a/etc/ORG-NEWS +++ b/etc/ORG-NEWS @@ -207,12 +207,35 @@ selection. TODO state, priority, tags, statistics cookies, and COMMENT keywords are allowed in the tree structure. -*** Asynchronous code evaluatation in ~ob-shell~ +*** Asynchronous code evaluation in ~ob-shell~ Running shell blocks with the ~:session~ header freezes Emacs until execution completes. The new ~:async~ header allows users to continue editing with Emacs while a ~:session~ block executes. +*** Automatic sync of source blocks and tangled blocks in ~ob-tangle-sync~ + +Invoking minor mode =org-babel-tangle-sync-mode= synchronizes contents +between a currently visited tangled file its org-mode source block +(and vice versa) via the =after-save-hook=. + +Desired tangling actions can be assymetric depending on whether the +org-mode source block header argument =:tangle-sync <action>= has an +action of: + +- =skip= :: do nothing, just save the buffer, even if the sync mode is + active + +- =import= :: only pull changes from the tangled block into the + org-mode source block (even when visited from either) + +- =export= :: only pull changes from the org-mode source block into + the tangled block (even when visited from either) + +- =both= (or nil) :: freely sync changes of current buffer to + associated source or target + + ** Miscellaneous *** Blank lines after removed objects are not retained during export [-- Attachment #3: lisp_ob-tangle-sync.el.diff --] [-- Type: text/x-patch, Size: 3369 bytes --] diff --git a/lisp/ob-tangle-sync.el b/lisp/ob-tangle-sync.el index 61c23f647..cfa6abdd2 100644 --- a/lisp/ob-tangle-sync.el +++ b/lisp/ob-tangle-sync.el @@ -44,7 +44,6 @@ (define-minor-mode org-babel-tangle-sync-mode "Global minor mode that synchronizes tangled files after every save." :global t - :interactive t :lighter " o-ts" (if org-babel-tangle-sync-mode (add-hook 'after-save-hook 'org-babel-tangle-sync-synchronize nil t) @@ -57,8 +56,8 @@ here are subject to the org-babel-tangle-sync treatment. If nil, then all org files with tangle headers are considered." :group 'org-babel-tangle-sync :type 'list - :package-version '(Org . "9.6.5") - :set (lambda (_var val) (mapcar #'(lambda (x) (expand-file-name x)) val))) + :package-version '(Org . "9.7") + :set (lambda (var val) (set var (mapcar #'expand-file-name val)))) (defun org-babel-tangle-sync--babel-tangle-jump (link block-name) @@ -95,15 +94,15 @@ If the cursor is either within the source file or in destination tangled file, perform a desired tangling action. The tangling action by default is to detangle the tangled files' changes back to its source block, or to tangle the source block to its tangled -file. Actions are one of `skip' (no action), `pull' (detangle +file. Actions are one of `skip' (no action), `import' (detangle only), `export' (tangle only), and `both' (default, synchronize in both directions). All `org-mode' source blocks and all tangled files with comments are considered valid targets, unless specified otherwise by `org-babel-tangle-sync-files'." (interactive) (let* ((link (save-excursion - (progn (re-search-backward org-link-bracket-re nil t) - (match-string-no-properties 0)))) + (re-search-backward org-link-bracket-re nil t) + (match-string-no-properties 0))) (block-name (match-string 2)) (orgfile-p (string= major-mode "org-mode")) (tangled-file-p (and link (not orgfile-p)))) @@ -142,12 +141,12 @@ specified otherwise by `org-babel-tangle-sync-files'." (org-babel-detangle) (message "Synced to %s" source-file)))))) - ;; Source Block → Tangled File (or Source Block ← Tangled File (via "pull")) + ;; Source Block → Tangled File (or Source Block ← Tangled File (via "import")) (when orgfile-p ;; Tangle action of Source file on Block if: ;; - member of sync file list (or list is empty) ;; Actions - ;; - pull (Source Block ← File) + ;; - import (Source Block ← File) ;; - skip (nothing) ;; - export, both, nil (Source Block → File) (if (or (null org-babel-tangle-sync-files) @@ -157,7 +156,7 @@ specified otherwise by `org-babel-tangle-sync-files'." (tangle-file (cdr (assq :tangle src-headers))) (tangle-action (alist-get :tangle-sync src-headers))) (when tangle-file - (cond ((string= tangle-action "pull") (save-excursion + (cond ((string= tangle-action "import") (save-excursion (org-babel-detangle tangle-file))) ((string= tangle-action "skip") nil) (t (let ((current-prefix-arg '(16))) ^ permalink raw reply related [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-04-27 6:27 ` Mehmet Tekman @ 2023-04-28 10:57 ` Ruijie Yu via General discussions about Org-mode. 2023-04-28 11:28 ` Mehmet Tekman 0 siblings, 1 reply; 77+ messages in thread From: Ruijie Yu via General discussions about Org-mode. @ 2023-04-28 10:57 UTC (permalink / raw) To: Mehmet Tekman; +Cc: emacs-orgmode Mehmet Tekman <mtekman89@gmail.com> writes: > Ruijie Yu <ruijie@netyu.xyz> writes: > >> Great idea! Some inline comments below. >> >> You only need to say #'expand-file-name instead of the quoted lambda. >> Also, you need to set the variable, otherwise the variable >> `org-babel-tangle-sync-files' is undefined. >> >> What I have in mind is this: >> :set (lambda (var val) (set var (mapcar #'expand-file-name val))) >> >> Here you don't have to use `progn' because it is implied from `save-excursion'. >> > > Thanks! I've made your changes, and I've also incorporated John > Wiegley's comments about using "import" instead of "pull" as a > tangle-sync action word (small diff attached). > > I've also written up my changes in the ~etc/ORG-NEWS~ and > targeted my custom variable for the 9.7 release (diff attached). Thanks. Can you make a full patch from the current main branch to your changes, with a commit message and so on? This would help reviewers to look at the full picture of what are modified. Take a look at the manpage git-format-patch(1) if you aren't sure how to do it. For the commit message, take a look at https://orgmode.org/worg/org-contribute.html#commit-messages for inspirations. >> Is there possibility to have a local minor mode (without introducing too >> much code changes)? > > I initially tried it this way, but the problem is that an org > source block buffer might be in sync-mode, but it's > corresponding tangle file might not be, making any changes > asymmetric. > > Another issue is in order to see the changes in the tangled file, > the tangle buffer needs to be reverted (with user prompt) which > then switches off the sync-mode for that buffer on reload. > > One way around this (and it's something I implemented 3 years ago in > my messy org-tanglesync[0] MELPA code) is to set an explicit list of > "sync files", and then for Emacs to parse every =:tangle= header in a > given file when loaded (via =org-src-mode-hook=) to create an alist of > config files and their associated tangled files[1], such as > =((file1.conf . (tanglefile1.txt tanglefile2.txt etc)))=. Then, for > example, when ~tanglefile1.txt~ is loaded, Emacs knows that it should > load the sync-mode too. > > This approach works reasonably well when the "sync files" list is > mandatory, but it's also prone to errors if a sync file is edited and > the alist of config files isn't updated, and the user would also lose > the flexibility of having ~ob-tangle-sync~ function everywhere. > > I think a global minor mode is really elegant in this regard and I > wish I knew about it 3 years ago! Thanks for explaining this. Yes, this sounds like a lot of work with probably insufficient audience, so I'd wait for more use cases to come up before thinking about local minor modes. -- Best, RY [Please note that this mail might go to spam due to some misconfiguration in my mail server -- still investigating.] ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-04-28 10:57 ` Ruijie Yu via General discussions about Org-mode. @ 2023-04-28 11:28 ` Mehmet Tekman 2023-05-02 20:43 ` Mehmet Tekman 0 siblings, 1 reply; 77+ messages in thread From: Mehmet Tekman @ 2023-04-28 11:28 UTC (permalink / raw) To: emacs-orgmode Ruijie Yu <ruijie@netyu.xyz> writes: > Thanks. Can you make a full patch from the current main branch to your > changes, with a commit message and so on? This would help reviewers to > look at the full picture of what are modified. Take a look at the > manpage git-format-patch(1) if you aren't sure how to do it. I'm at the moment patching the =org-babel-tangle= function, to act appropriately to the =:tangle-sync <action>= requests. It's harder than I thought, because the way the function currently works is to populate a temporary buffer with source block contents and then to overwrite the destination tangle file, i.e. it is unaware of the tangled files contents during processing. This is efficient, but works anathema to syncing changes in the opposite direction, i.e. when a block requests =:tangle-sync import=. I'm currently doing a small rewrite of the function that populates a list of comments in the tangled file (restricted to stemming from the source org file), and then either populates this temporary buffer with the org mode source block contents or with the already tangled contents (when "import" is given for that block). I will submit a full patch with these changes (with properly detailed commit messages) from the main branch in the next few days. ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-04-28 11:28 ` Mehmet Tekman @ 2023-05-02 20:43 ` Mehmet Tekman 2023-05-03 2:31 ` Ruijie Yu via General discussions about Org-mode. 2023-05-03 11:43 ` Ihor Radchenko 0 siblings, 2 replies; 77+ messages in thread From: Mehmet Tekman @ 2023-05-02 20:43 UTC (permalink / raw) To: emacs-orgmode [-- Attachment #1.1: Type: text/plain, Size: 1319 bytes --] Hello again, sorry for the delay - I had some holiday time off that couldn't wait ;-) I've modified the ob-tangle.el file for the main tangling and detangling functions. Most importantly, both functions can now exchange information from the source Org mode file to the target remote tangle file in either direction, depending on whether the source Org file has `:tangle-sync <action>' in the header. The action is one of: - "export" = always transmit information from the source Org mode block to the target remote file. - "import" = always transmit information from the target remote file to the source Org mode block. - "skip" = skip the block. - "both" = transmit information from source block to target block or target block to source, depending on whether the tangle or detangle is called from the source buffer or the target buffer respectively. These functions work at the whole buffer and at the per-block level. The `org-babel-tangle-sync' functions automate this process by hooking into the `after-save-hook' and tangling / detangling the current block. I feel that I should write what the main motivation for this is: Dotfiles that are always in sync with the org-mode files they stem from. Hope this turns into something big! Best, Mehmet [-- Attachment #1.2: Type: text/html, Size: 1544 bytes --] [-- Attachment #2: 0005-lisp-ob-tangle-sync.el-Automatic-synchronization-of-.patch --] [-- Type: application/x-patch, Size: 6766 bytes --] [-- Attachment #3: 0004-lisp-ob-tangle.el-Sync-aware-tangle-function-with-be.patch --] [-- Type: application/x-patch, Size: 5843 bytes --] [-- Attachment #4: 0002-lisp-ob-tangle.el-Sync-action-aware-detangle-functio.patch --] [-- Type: application/x-patch, Size: 4628 bytes --] [-- Attachment #5: 0003-lisp-ob-tangle.el-Tangle-function-made-aware-of-remo.patch --] [-- Type: application/x-patch, Size: 4121 bytes --] [-- Attachment #6: 0001-lisp-ob-tangle.el-Detangle-a-single-block.patch --] [-- Type: application/x-patch, Size: 5474 bytes --] [-- Attachment #7: 0006-etc-ORG-NEWS-lisp-ob-tangle.el-Added-news-and-name.patch --] [-- Type: application/x-patch, Size: 2779 bytes --] ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-05-02 20:43 ` Mehmet Tekman @ 2023-05-03 2:31 ` Ruijie Yu via General discussions about Org-mode. 2023-05-03 7:53 ` Mehmet Tekman 2023-05-03 11:43 ` Ihor Radchenko 1 sibling, 1 reply; 77+ messages in thread From: Ruijie Yu via General discussions about Org-mode. @ 2023-05-03 2:31 UTC (permalink / raw) To: Mehmet Tekman; +Cc: emacs-orgmode Mehmet Tekman <mtekman89@gmail.com> writes: > Hello again, sorry for the delay - I had some holiday time off > that couldn't wait ;-) > > I've modified the ob-tangle.el file for the main tangling and > detangling functions. Most importantly, both functions can now > exchange information from the source Org mode file to the target > remote tangle file in either direction, depending on whether the > source Org file has `:tangle-sync <action>' in the header. > > The action is one of: > > - "export" = always transmit information from the source Org mode > block to the target remote file. > - "import" = always transmit information from the target remote > file to the source Org mode block. > - "skip" = skip the block. > - "both" = transmit information from source block to target block > or target block to source, depending on whether the > tangle or detangle is called from the source buffer or > the target buffer respectively. > > These functions work at the whole buffer and at the per-block > level. The `org-babel-tangle-sync' functions automate this > process by hooking into the `after-save-hook' and tangling / > detangling the current block. > > I feel that I should write what the main motivation for this is: > Dotfiles that are always in sync with the org-mode files they > stem from. > > Hope this turns into something big! > Best, > > Mehmet > > [4. application/x-patch; 0005-lisp-ob-tangle-sync.el-Automatic-synchronization-of-.patch]... > > [5. application/x-patch; 0004-lisp-ob-tangle.el-Sync-aware-tangle-function-with-be.patch]... > > [6. application/x-patch; 0002-lisp-ob-tangle.el-Sync-action-aware-detangle-functio.patch]... > > [7. application/x-patch; 0003-lisp-ob-tangle.el-Tangle-function-made-aware-of-remo.patch]... > > [8. application/x-patch; 0001-lisp-ob-tangle.el-Detangle-a-single-block.patch]... > > [9. application/x-patch; 0006-etc-ORG-NEWS-lisp-ob-tangle.el-Added-news-and-name.patch]... I noticed that you modified argument order of a public function `org-babel-detangle' -- is it possible that you retain the order of arguments? The accumulated change you proposed is this: -(defun org-babel-detangle (&optional source-code-file) +(defun org-babel-detangle (&optional arg source-code-file ignore-header) What I think it should be: (defun org-babel-detangle (&optional source-code-file arg ignore-header) ...) This way, existing (external) users of this function can rest assured that their code is not broken by your patchset. Thoughts? -- Best, RY [Please note that this mail might go to spam due to some misconfiguration in my mail server -- still investigating.] ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-05-03 2:31 ` Ruijie Yu via General discussions about Org-mode. @ 2023-05-03 7:53 ` Mehmet Tekman 2023-05-03 8:34 ` Mehmet Tekman 0 siblings, 1 reply; 77+ messages in thread From: Mehmet Tekman @ 2023-05-03 7:53 UTC (permalink / raw) To: emacs-orgmode [-- Attachment #1.1: Type: text/plain, Size: 3422 bytes --] > I noticed that you modified argument order of a public function > `org-babel-detangle' -- is it possible that you retain the order of > arguments? Good catch - I think I changed the order so that it would match the interactivity of its sister function `org-babel-tangle', and I think in a previous rendition of ob-tangle-sync.el, I called `org-babel-detangle' interactively. But I see now that I no longer directly call it anywhere in my edited code, so I can easily change it back. Attached is a patch Best On Wed, 3 May 2023 at 04:35, Ruijie Yu <ruijie@netyu.xyz> wrote: > > Mehmet Tekman <mtekman89@gmail.com> writes: > > > Hello again, sorry for the delay - I had some holiday time off > > that couldn't wait ;-) > > > > I've modified the ob-tangle.el file for the main tangling and > > detangling functions. Most importantly, both functions can now > > exchange information from the source Org mode file to the target > > remote tangle file in either direction, depending on whether the > > source Org file has `:tangle-sync <action>' in the header. > > > > The action is one of: > > > > - "export" = always transmit information from the source Org mode > > block to the target remote file. > > - "import" = always transmit information from the target remote > > file to the source Org mode block. > > - "skip" = skip the block. > > - "both" = transmit information from source block to target block > > or target block to source, depending on whether the > > tangle or detangle is called from the source buffer or > > the target buffer respectively. > > > > These functions work at the whole buffer and at the per-block > > level. The `org-babel-tangle-sync' functions automate this > > process by hooking into the `after-save-hook' and tangling / > > detangling the current block. > > > > I feel that I should write what the main motivation for this is: > > Dotfiles that are always in sync with the org-mode files they > > stem from. > > > > Hope this turns into something big! > > Best, > > > > Mehmet > > > > [4. application/x-patch; > 0005-lisp-ob-tangle-sync.el-Automatic-synchronization-of-.patch]... > > > > [5. application/x-patch; > 0004-lisp-ob-tangle.el-Sync-aware-tangle-function-with-be.patch]... > > > > [6. application/x-patch; > 0002-lisp-ob-tangle.el-Sync-action-aware-detangle-functio.patch]... > > > > [7. application/x-patch; > 0003-lisp-ob-tangle.el-Tangle-function-made-aware-of-remo.patch]... > > > > [8. application/x-patch; > 0001-lisp-ob-tangle.el-Detangle-a-single-block.patch]... > > > > [9. application/x-patch; > 0006-etc-ORG-NEWS-lisp-ob-tangle.el-Added-news-and-name.patch]... > > I noticed that you modified argument order of a public function > `org-babel-detangle' -- is it possible that you retain the order of > arguments? > > The accumulated change you proposed is this: > > -(defun org-babel-detangle (&optional source-code-file) > +(defun org-babel-detangle (&optional arg source-code-file ignore-header) > > What I think it should be: > > (defun org-babel-detangle (&optional source-code-file arg > ignore-header) ...) > > This way, existing (external) users of this function can rest assured > that their code is not broken by your patchset. Thoughts? > > -- > Best, > > > RY > > [Please note that this mail might go to spam due to some > misconfiguration in my mail server -- still investigating.] > [-- Attachment #1.2: Type: text/html, Size: 4328 bytes --] [-- Attachment #2: 0007-lisp-ob-tangle.el-Restore-argument-order-in-org-babe.patch --] [-- Type: text/x-patch, Size: 2193 bytes --] From 3399279e2c5c2a56076fa85adffffe7aa5c2ca74 Mon Sep 17 00:00:00 2001 From: MT <mtekman89@gmail.com> Date: Wed, 3 May 2023 09:47:34 +0200 Subject: [PATCH 7/7] lisp/ob-tangle.el: Restore argument order in org-babel-detangle * ob-tangle.el (org-babel-detangle): Change `source-code-file' to be the first argument again for external usage. This changes the prefix argument functionality, which is replaced by a much easier `single-p' argument to denote whether a single block is being detangled. --- lisp/ob-tangle.el | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lisp/ob-tangle.el b/lisp/ob-tangle.el index f199c77bc..fde6683a6 100644 --- a/lisp/ob-tangle.el +++ b/lisp/ob-tangle.el @@ -660,13 +660,13 @@ by `org-babel-get-src-block-info'." (org-fill-template org-babel-tangle-comment-format-end link-data)))) ;; de-tangling functions -(defun org-babel-detangle (&optional arg source-code-file ignore-header) +(defun org-babel-detangle (&optional source-code-file single-p ignore-header) "Propagate changes in source file back original to Org file. This requires that code blocks were tangled with link comments -which enable the original code blocks to be found. With one -universal prefix argument, only detangle the block at point. If -IGNORE-HEADER then detangle regardless of `:tangle-sync' status." - (interactive "P") +which enable the original code blocks to be found. If SINGLE-P +is t then only detangle a single block. If IGNORE-HEADER then +detangle regardless of `:tangle-sync' status." + (interactive) (save-excursion (when source-code-file (find-file source-code-file)) (let ((counter 0) (skip-counter 0) (tang-counter 0) end) @@ -677,7 +677,7 @@ IGNORE-HEADER then detangle regardless of `:tangle-sync' status." ((string= action "export") (setq tang-counter (1+ tang-counter)))) (setq counter (1+ counter)))))) - (if (equal arg '(4)) + (if single-p (funcall single-block-metrics) (goto-char (point-min)) (while (re-search-forward org-link-bracket-re nil t) -- 2.40.1 ^ permalink raw reply related [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-05-03 7:53 ` Mehmet Tekman @ 2023-05-03 8:34 ` Mehmet Tekman 2023-05-03 8:44 ` Ihor Radchenko 0 siblings, 1 reply; 77+ messages in thread From: Mehmet Tekman @ 2023-05-03 8:34 UTC (permalink / raw) To: emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 405 bytes --] (Sorry for the previous top posting.) One issue that I cannot seem to solve by myself is getting the `org-babel-tangle-sync-mode' to persist on the `after-save-hook' after it's been activated. My understanding of this hook is that it is global and persists across buffers, but I'm seeing some inconsistent behaviour that requires me to toggle the mode on and off again. Is there something I'm missing? [-- Attachment #2: Type: text/html, Size: 519 bytes --] ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-05-03 8:34 ` Mehmet Tekman @ 2023-05-03 8:44 ` Ihor Radchenko 0 siblings, 0 replies; 77+ messages in thread From: Ihor Radchenko @ 2023-05-03 8:44 UTC (permalink / raw) To: Mehmet Tekman; +Cc: emacs-orgmode Mehmet Tekman <mtekman89@gmail.com> writes: > (Sorry for the previous top posting.) > > One issue that I cannot seem to solve by myself is getting the > `org-babel-tangle-sync-mode' to persist on the `after-save-hook' > after it's been activated. My understanding of this hook is > that it is global and persists across buffers, but I'm seeing > some inconsistent behaviour that requires me to toggle the mode > on and off again. Any hook can be local. See LOCAL argument in help:add-hook -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-05-02 20:43 ` Mehmet Tekman 2023-05-03 2:31 ` Ruijie Yu via General discussions about Org-mode. @ 2023-05-03 11:43 ` Ihor Radchenko 2023-05-03 13:54 ` Mehmet Tekman 2023-05-03 15:05 ` Mehmet Tekman 1 sibling, 2 replies; 77+ messages in thread From: Ihor Radchenko @ 2023-05-03 11:43 UTC (permalink / raw) To: Mehmet Tekman; +Cc: emacs-orgmode Mehmet Tekman <mtekman89@gmail.com> writes: > I've modified the ob-tangle.el file for the main tangling and > detangling functions. Most importantly, both functions can now > exchange information from the source Org mode file to the target > remote tangle file in either direction, depending on whether the > source Org file has `:tangle-sync <action>' in the header. Thanks! > The action is one of: > > - "export" = always transmit information from the source Org mode > block to the target remote file. > - "import" = always transmit information from the target remote > file to the source Org mode block. > - "skip" = skip the block. > - "both" = transmit information from source block to target block > or target block to source, depending on whether the > tangle or detangle is called from the source buffer or > the target buffer respectively. May it be better to make :tangle header argument compound? Like ":tangle "file" export". Similar to :results header argument. See "16.6 Results of Evaluation" section of Org manual. Also, some general comments on the patches: 1. Please make sure that patches do not leave Org in broken state in the middle of the patchset. Your patch #1 adds two `defun's in the middle of `org-babel-detangle', which is not right. 2. When naming private functions, "--" should be after library prefix: org-babel--... 3. Please do not use private functions from third-party libraries. I am talking about `cl--set-buffer-substring' in particular. 4. Please make sure that docstrings clearly describe what the function does, each of its arguments, and the expected return value. In the patch, `org-babel-detangle--block-contents', NEAREST is ambiguous. The actual meaning is "block at point or previous block". 5. Pay attention to buffer boundaries. In particular, remember that buffer may be narrowed when you call a command and that expressions like (+ 2 (line-beginning-position)) may resolve beyond `point-min'/`point-max'. 6. Avoid using cryptic list functions like `cdadar' when you can use something more readable. 7. When naming functions or macros, make sure that the name is roughly describing the purpose of the function. In `org-babel-detangle', you added `single-block-metrics' local function that does not only do the metrics, but also (unexpectedly!) calls `org-babel-detangle-single-block'. This is especially important for local functions that lack docstring. 8. In `org-babel-tangle', (setq block-counter (+ 1 block-counter)) appears to be misplaced into outer sexp level after your patch. 9. In commit messages, please mark newly added functions clearly. Like "(org-babel-foo): New function. It does this and that." Same for commit summaries - "lisp/ob-tangle.el: Detangle a single block" is awkward. You should clearly indicate that you added something new to the library. -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-05-03 11:43 ` Ihor Radchenko @ 2023-05-03 13:54 ` Mehmet Tekman 2023-05-03 18:06 ` Ihor Radchenko 2023-05-03 15:05 ` Mehmet Tekman 1 sibling, 1 reply; 77+ messages in thread From: Mehmet Tekman @ 2023-05-03 13:54 UTC (permalink / raw) To: Ihor Radchenko; +Cc: emacs-orgmode Ihor Radchenko <yantar92@posteo.net> writes: > Mehmet Tekman <mtekman89@gmail.com> writes: >> >> One issue that I cannot seem to solve by myself is getting the >> `org-babel-tangle-sync-mode' to persist on the `after-save-hook' >> after it's been activated. My understanding of this hook is >> that it is global and persists across buffers, but I'm seeing >> some inconsistent behaviour that requires me to toggle the mode >> on and off again. > > Any hook can be local. > See LOCAL argument in help:add-hook Ah, that solves the problem immediately thank you! > Mehmet Tekman <mtekman89@gmail.com> writes: > >> I've modified the ob-tangle.el file for the main tangling and >> detangling functions. Most importantly, both functions can now >> exchange information from the source Org mode file to the target >> remote tangle file in either direction, depending on whether the >> source Org file has `:tangle-sync <action>' in the header. > > Thanks! > >> The action is one of: >> >> - "export" = always transmit information from the source Org mode >> block to the target remote file. >> - "import" = always transmit information from the target remote >> file to the source Org mode block. >> - "skip" = skip the block. >> - "both" = transmit information from source block to target block >> or target block to source, depending on whether the >> tangle or detangle is called from the source buffer or >> the target buffer respectively. > > May it be better to make :tangle header argument compound? > Like ":tangle "file" export". Similar to :results header argument. See > "16.6 Results of Evaluation" section of Org manual. > That's a great idea I had not considered, and would definitely reduce the header bloat, especially since `:tangle-sync' *requires* the `:tangle' header to be there. > Also, some general comments on the patches: > Great! > 2. When naming private functions, "--" should be after library prefix: > org-babel--... > Thanks, I will rename `org-babel-detangle-single-block--from-source'. > 3. Please do not use private functions from third-party libraries. I am > talking about `cl--set-buffer-substring' in particular. > So initially I used `(setf (buffer-substring X Y) new-content)` but I recieved a warning from Emacs that it was an obsolete generalized variable. After some searching I found this entry in an emacs fork used the cl library: https://github.com/emacs-citar/citar/commit/809953a2191d0e3217ffbed9270be9b3cd6abfd2 Since `(require 'cl-lib)' is already imported in ~ob-tangle.el~, I did not think it was too taboo to use. How does one then set the buffer substring? > 4. Please make sure that docstrings clearly describe what the function > does, each of its arguments, and the expected return value. > In the patch, `org-babel-detangle--block-contents', NEAREST is > ambiguous. The actual meaning is "block at point or previous block". > > 5. Pay attention to buffer boundaries. In particular, remember that > buffer may be narrowed when you call a command and that expressions > like (+ 2 (line-beginning-position)) may resolve beyond > `point-min'/`point-max'. > > 6. Avoid using cryptic list functions like `cdadar' when you can use > something more readable. > > 7. When naming functions or macros, make sure that the name is roughly > describing the purpose of the function. In `org-babel-detangle', you > added `single-block-metrics' local function that does not only do the > metrics, but also (unexpectedly!) calls > `org-babel-detangle-single-block'. This is especially important for > local functions that lack docstring. > > 8. In `org-babel-tangle', (setq block-counter (+ 1 block-counter)) > appears to be misplaced into outer sexp level after your patch. Thanks for these, I will clean up and better document my code. > 1. Please make sure that patches do not leave Org in broken state in the > middle of the patchset. Your patch #1 adds two `defun's in the middle > of `org-babel-detangle', which is not right. > > 9. In commit messages, please mark newly added functions clearly. > Like "(org-babel-foo): New function. It does this and that." > Same for commit summaries - "lisp/ob-tangle.el: Detangle a single > block" is awkward. You should clearly indicate that you added > something new to the library. Apologies. I rebased and squashed all my commits into one, and then selectively staged hunks into seperate commits for the git format-patc process. For some reason the diff function decided that the new functions should exist right in the middle of an existing function and I was not sure how to resolve it at the time (though I have a better idea now). I will take better care with the messages. I tried to look for previous "[ANN]" postings in the mailing list that I could emulate, but didn't pay enough attention it seems. I'm finally using `gnus' as my mail client so I'm slowly getting into a more streamlined mindset that should be better at submitting and formatting patches. (To reply to a mailing list, I do a wide reply to the author and hope that the `Mail-Followup-To' header is used?) Apropos patches: Given how broken my current patches are, my next set of changes will be not contingent on the previous ones. I will start a new set of patches. I hope that's okay. Best, Mehmet ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-05-03 13:54 ` Mehmet Tekman @ 2023-05-03 18:06 ` Ihor Radchenko 0 siblings, 0 replies; 77+ messages in thread From: Ihor Radchenko @ 2023-05-03 18:06 UTC (permalink / raw) To: Mehmet Tekman; +Cc: emacs-orgmode Mehmet Tekman <mtekman89@gmail.com> writes: >> 3. Please do not use private functions from third-party libraries. I am >> talking about `cl--set-buffer-substring' in particular. >> > > So initially I used `(setf (buffer-substring X Y) new-content)` but I > recieved a warning from Emacs that it was an obsolete generalized > variable. Yup. See https://yhetil.org/emacs-devel/87tu5fzu2r.fsf@localhost/ We need to do things manually. > After some searching I found this entry in an emacs fork used the cl > library: > https://github.com/emacs-citar/citar/commit/809953a2191d0e3217ffbed9270be9b3cd6abfd2 > > Since `(require 'cl-lib)' is already imported in ~ob-tangle.el~, I did > not think it was too taboo to use. Private functions are a subject of change without notice. That's why we do not use them, unless they are _our_ private functions we have control about. > How does one then set the buffer substring? `replace-region-contents'. > Apologies. I rebased and squashed all my commits into one, and then > selectively staged hunks into seperate commits for the git format-patc > process. For some reason the diff function decided that the new > functions should exist right in the middle of an existing function and I > was not sure how to resolve it at the time (though I have a better idea > now). Interactive rebase is helpful. You can also edit, reset files, or apply patches in the middle of rebasing to do extra adjustments. > I will take better care with the messages. I tried to look for previous > "[ANN]" postings in the mailing list that I could emulate, but didn't > pay enough attention it seems. Just try to follow what we usually do in commit messages. See https://git.savannah.gnu.org/cgit/emacs/org-mode.git/log/ Also, see https://www.gnu.org/prep/standards/html_node/Change-Logs.html and https://git.savannah.gnu.org/cgit/emacs.git/plain/CONTRIBUTE > I'm finally using `gnus' as my mail client so I'm slowly getting into a > more streamlined mindset that should be better at submitting and > formatting patches. (To reply to a mailing list, I do a wide reply to > the author and hope that the `Mail-Followup-To' header is used?) Reply all, or wide reply should be the right way. We do not want to exclude participants who are not subscribed to the mailing list. > Apropos patches: > Given how broken my current patches are, my next set of changes will be > not contingent on the previous ones. I will start a new set of patches. > I hope that's okay. A common approach is changing subject to [PATCH v2] ... You can also use so-called reroll count when generating patchset from git (or magit). -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-05-03 11:43 ` Ihor Radchenko 2023-05-03 13:54 ` Mehmet Tekman @ 2023-05-03 15:05 ` Mehmet Tekman 2023-05-03 15:21 ` Ihor Radchenko 1 sibling, 1 reply; 77+ messages in thread From: Mehmet Tekman @ 2023-05-03 15:05 UTC (permalink / raw) To: Ihor Radchenko; +Cc: emacs-orgmode >> Ihor Radchenko <yantar92@posteo.net> writes: >> >> May it be better to make :tangle header argument compound? >> Like ":tangle "file" export". Similar to :results header argument. See >> "16.6 Results of Evaluation" section of Org manual. >> > > Mehmet Tekman <mtekman89@gmail.com> writes: > > That's a great idea I had not considered, and would definitely reduce > the header bloat, especially since `:tangle-sync' *requires* the >`:tangle' header to be there. On closer inspection it might actually be unpractical to have the sync action within the `:tangle' syntax, the main reason being that people might define the tangle target in the document header for ease of use. e.g. #+begin_example #+TITLE: Sync Many Blocks #+PROPERTY: header-args :tangle /tmp/default_tangle.txt :comments yes ** File Contents *** Free Head #+begin_src conf This is some text that can be synced both ways by default #+end_src *** Import #+begin_src conf :tangle-sync import This is text that can only be imported from the remote, but it still refers to the default tangle argument #+end_src #+end_example If we instead encode the sync action in the `:tangle' syntax then users will have to explicitly type out the tangle file in each block. Also, tangle-sync wouldn't be the only extra tangle-related property in a block, since tangle-mode is also a thing. Is it okay to leave `:tangle-sync' as it is? ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-05-03 15:05 ` Mehmet Tekman @ 2023-05-03 15:21 ` Ihor Radchenko [not found] ` <87lei577g4.fsf@gmail.com> 0 siblings, 1 reply; 77+ messages in thread From: Ihor Radchenko @ 2023-05-03 15:21 UTC (permalink / raw) To: Mehmet Tekman; +Cc: emacs-orgmode Mehmet Tekman <mtekman89@gmail.com> writes: > On closer inspection it might actually be unpractical to have the sync > action within the `:tangle' syntax, the main reason being that people > might define the tangle target in the document header for ease of use. This is not a problem. Try #+PROPERTY: header-args :results drawer output #+begin_src emacs-lisp :results value (list 1 2) #+end_src #+begin_src bash echo 1 2 #+end_src -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 77+ messages in thread
[parent not found: <87lei577g4.fsf@gmail.com>]
[parent not found: <87lei5v1fg.fsf@localhost>]
[parent not found: <87fs8duyae.fsf@localhost>]
* Re: [ANN] lisp/ob-tangle-sync.el [not found] ` <87fs8duyae.fsf@localhost> @ 2023-05-09 14:03 ` Mehmet Tekman 2023-05-10 9:46 ` Ihor Radchenko 0 siblings, 1 reply; 77+ messages in thread From: Mehmet Tekman @ 2023-05-09 14:03 UTC (permalink / raw) To: Ihor Radchenko; +Cc: emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 1591 bytes --] Ihor Radchenko <yantar92@posteo.net> writes: > Yes. See `org-babel-common-header-args-w-values'. In particular, take a > look at (results ...). Thanks, it took me some time to get my head around how to use this. > I now ported a bit of documentation from my refactor branch. > See https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=f268819d1 I'm having some problems getting: `:tangle <tangle/yes/no/<filename>> [import/export/both/skip]' to work nicely with the existing framework. The main issue lies in the `:any` keyword in `org-babel-common-header-args-w-values' for the tangle entry making it difficult to determine where one exclusionary group begins and where the other ends. e.g. (defconst org-babel-common-header-args-w-values ... (tangle . ((tangle yes no :any) (import export skip sync))) ... ) If I remove the :any keyword, these two groups work with the existing framework in `org-babel-merge-params', but this would then mean that the first tangle argument can't just be a filename string. I can get around it by changing `:any' to `file' and then letting users describe their tangle headers via e.g. `:tangle file import file: /some/file' but this would be a breaking change. In the meantime I put together a hacky solution that parses the tangle header independently (`org-babel--handle-tangle-args') with the aim that the first argument should ideally define a tangle filename action and the second argument should ideally define a tangle sync action. Please see the attached minor patch (diff) and a toy org example file. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: Exclusive values for TANGLE header arguments added to the `org-babel-common-header-args-w-values' constant, along with a dedicated function `org-babel--handle-tangle-args' which parses it to conform to a `:tangle <filename action> <sync action>' format. --] [-- Type: text/x-patch, Size: 4013 bytes --] diff --git a/lisp/ob-core.el b/lisp/ob-core.el index 65fa47ab5..026788c00 100644 --- a/lisp/ob-core.el +++ b/lisp/ob-core.el @@ -431,7 +431,8 @@ then run `org-babel-switch-to-session'." (sep . :any) (session . :any) (shebang . :any) - (tangle . ((tangle yes no :any))) + (tangle . ((tangle yes no :any) + (import export skip sync))) (tangle-mode . ((#o755 #o555 #o444 :any))) (var . :any) (wrap . :any))) @@ -2802,6 +2803,9 @@ parameters when merging lists." (exports-exclusive-groups (mapcar (lambda (group) (mapcar #'symbol-name group)) (cdr (assq 'exports org-babel-common-header-args-w-values)))) + (tangle-exclusive-groups + (mapcar (lambda (group) (mapcar #'symbol-name group)) + (cdr (assq 'tangle org-babel-common-header-args-w-values)))) (merge (lambda (exclusive-groups &rest result-params) ;; Maintain exclusivity of mutually exclusive parameters, @@ -2821,7 +2825,7 @@ parameters when merging lists." params ;Final parameters list. ;; Some keywords accept multiple values. We need to treat ;; them specially. - vars results exports) + vars results exports tangle) (dolist (plist plists) (dolist (pair plist) (pcase pair @@ -2872,6 +2876,12 @@ parameters when merging lists." (cond ((and value (functionp value)) (funcall value)) (value value) (t "")))))) + (`(:tangle . ,value) + (setq tangle (funcall merge + tangle-exclusive-groups + tangle + (split-string + (or value ""))))) ((or '(:dir . attach) '(:dir . "'attach")) (unless (org-attach-dir nil t) (error "No attachment directory for element (add :ID: or :DIR: property)")) @@ -2897,11 +2907,38 @@ parameters when merging lists." params))))) ;; Handle other special keywords, which accept multiple values. (setq params (nconc (list (cons :results (mapconcat #'identity results " ")) - (cons :exports (mapconcat #'identity exports " "))) + (cons :exports (mapconcat #'identity exports " ")) + (cons :tangle (org-babel--handle-tangle-args tangle))) params)) ;; Return merged params. (org-babel-eval-headers params))) +(defun org-babel--handle-tangle-args (tangle) + "Sanitize tangle arguments. +From the list of TANGLE parameters, assert that that there are at +maximum only two elements in the following preferential order: +the first element relates to a filename descriptor (such as a +path, `tangle', `yes', or `no'); the second element relates to a +valid sync direction." + (let* ((num-args (length tangle)) + ;; Extract valid mutex groups + (valid-tangle-headers (cdr (assoc 'tangle + org-babel-common-header-args-w-values))) + (valid-fname-args (seq-remove (lambda (x) (equal :any x)) (car valid-tangle-headers))) + (valid-sync-args (cadr valid-tangle-headers)) + ;; Attempt to split TANGLE by these mutex groups + (sync-arg (seq-filter (lambda (x) (member (intern x) valid-sync-args)) tangle)) + (fname-arg (seq-remove (lambda (x) (member x sync-arg)) tangle)) + ;; Search for a potential filename + (filename (seq-remove (lambda (x) (member (intern x) valid-fname-args)) fname-arg))) + (setq sync-arg (car sync-arg) + ;; Assumption: the last added tangle argument is more + ;; important than the one preceding it. + fname-arg (or (car filename) + (car fname-arg))) + (concat fname-arg (if sync-arg " " "" ) sync-arg))) + + (defun org-babel-noweb-p (params context) "Check if PARAMS require expansion in CONTEXT. CONTEXT may be one of :tangle, :export or :eval." [-- Attachment #3: Toy example file --] [-- Type: text/plain, Size: 721 bytes --] #+TITLE: Sync test #+PROPERTY: header-args :tangle /tmp/default_tangle.txt Running =(assoc :tangle (nth 2 (org-babel-get-src-block-info)))= on each of these should yield: #+begin_src conf (:tangle . /tmp/default_tangle.txt) #+end_src #+begin_src conf :tangle skip (:tangle . /tmp/default_tangle.txt skip) #+end_src #+begin_src conf :tangle randomfile sync (:tangle . randomfile sync) #+end_src #+begin_src conf :tangle randomfile (:tangle . randomfile) #+end_src #+begin_src conf :tangle import export ## Ignores import (:tangle . /tmp/default_tangle.txt export) #+end_src #+begin_src conf :tangle fname1 fname2 sync export ## Ignores fname1 and sync (:tangle . fname2 export) #+end_src ^ permalink raw reply related [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-05-09 14:03 ` Mehmet Tekman @ 2023-05-10 9:46 ` Ihor Radchenko 2023-05-10 11:06 ` mtekman89 0 siblings, 1 reply; 77+ messages in thread From: Ihor Radchenko @ 2023-05-10 9:46 UTC (permalink / raw) To: Mehmet Tekman; +Cc: emacs-orgmode Mehmet Tekman <mtekman89@gmail.com> writes: > I'm having some problems getting: > > `:tangle <tangle/yes/no/<filename>> [import/export/both/skip]' > > to work nicely with the existing framework. The main issue lies in the > `:any` keyword in `org-babel-common-header-args-w-values' for the tangle > entry making it difficult to determine where one exclusionary group > begins and where the other ends. > > e.g. > > (defconst org-babel-common-header-args-w-values > ... > (tangle . ((tangle yes no :any) > (import export skip sync))) > ... > ) And the reason is that tangle function inside `org-babel-merge-params' does not know how to handle :any in exclusive groups. We should modify it. For example like the following: 1. We will assume that :any can only occur one time in the exclusive groups. (Otherwise, there is no single definite way to parse header arguments) 2. Merge function will treat :any specially - when parameter does not match any of the argument values from all the groups combined, it is considered as :any and replace the previous corresponding values in its exclusive group, if any; In other words, we will need a special match for :any - "anything not equal to other values in all the groups combined". -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-05-10 9:46 ` Ihor Radchenko @ 2023-05-10 11:06 ` mtekman89 2023-05-10 11:32 ` Ihor Radchenko 0 siblings, 1 reply; 77+ messages in thread From: mtekman89 @ 2023-05-10 11:06 UTC (permalink / raw) To: Ihor Radchenko; +Cc: emacs-orgmode Ihor Radchenko <yantar92@posteo.net> writes: > 1. We will assume that :any can only occur one time in the exclusive > groups. (Otherwise, there is no single definite way to parse header > arguments) Makes sense (or we could revamp all header parsing into some kind of finite state machine… (I joke… for now…)) > 2. Merge function will treat :any specially - when parameter does not > match any of the argument values from all the groups combined, it is > considered as :any and replace the previous corresponding values in > its exclusive group, if any; This is something that I do in my `org-babel--handle-tangle-args', where the "filename" argument is the result of eliminating all other entries in the group. > In other words, we will need a special match for :any - "anything not > equal to other values in all the groups combined". Agreed, and I can try putting together something working in a day or two. Or, if this is something you can easily implement yourself, please do let me know. Best, Mehmet ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-05-10 11:06 ` mtekman89 @ 2023-05-10 11:32 ` Ihor Radchenko 2023-05-10 16:20 ` Mehmet Tekman 0 siblings, 1 reply; 77+ messages in thread From: Ihor Radchenko @ 2023-05-10 11:32 UTC (permalink / raw) To: mtekman89; +Cc: emacs-orgmode mtekman89@gmail.com writes: >> In other words, we will need a special match for :any - "anything not >> equal to other values in all the groups combined". > > Agreed, and I can try putting together something working in a day or > two. Or, if this is something you can easily implement yourself, please > do let me know. It will be great if you could do it. I have other things to work on. -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-05-10 11:32 ` Ihor Radchenko @ 2023-05-10 16:20 ` Mehmet Tekman 2023-05-12 12:33 ` Ihor Radchenko 0 siblings, 1 reply; 77+ messages in thread From: Mehmet Tekman @ 2023-05-10 16:20 UTC (permalink / raw) To: Ihor Radchenko; +Cc: emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 1401 bytes --] Ihor Radchenko <yantar92@posteo.net> writes: > It will be great if you could do it. > I have other things to work on. Of course! I'm just a little unfamiliar on how one coordinates active collaboration via mailing list :-) Anyways - I did it, and it took less time than I thought > We should modify it. For example like the following: > > 1. We will assume that :any can only occur one time in the exclusive > groups. (Otherwise, there is no single definite way to parse header > arguments) > 2. Merge function will treat :any specially - when parameter does not > match any of the argument values from all the groups combined, it is > considered as :any and replace the previous corresponding values in > its exclusive group, if any; > In other words, we will need a special match for :any - "anything not > equal to other values in all the groups combined". I've modified the `merge' function within `org-babel-merge-params' so that the main logic now accumulates a list of potential candidates that could be the :any keyword, and selects the last added candidate as the match. The first two patches are very minor, simply adding tangle-exclusive-groups using the existing code templates. The last patch is the merge function rewrite. It all seems to be passing tests, though I would like to add my toy.org file to the org testing framework at some point. Best, Mehmet [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: Added new tangle exclusion group for tangle sync action --] [-- Type: text/x-patch, Size: 916 bytes --] From eeb3f165498fcc420b862f67fb616b474a14b684 Mon Sep 17 00:00:00 2001 From: MT <mtekman89@gmail.com> Date: Wed, 10 May 2023 17:38:22 +0200 Subject: [PATCH 1/3] * lisp/ob-core.el (org-babel-common-header-args-w-values): Added mutually exclusive tangle groups relating to desired tangle sync actions (e.g. :tangle (yes|no|<filename>) [(import|export|sync)]) --- lisp/ob-core.el | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lisp/ob-core.el b/lisp/ob-core.el index 65fa47ab5..013a37ce5 100644 --- a/lisp/ob-core.el +++ b/lisp/ob-core.el @@ -431,7 +431,8 @@ then run `org-babel-switch-to-session'." (sep . :any) (session . :any) (shebang . :any) - (tangle . ((tangle yes no :any))) + (tangle . ((tangle yes no :any) + (import export skip sync))) (tangle-mode . ((#o755 #o555 #o444 :any))) (var . :any) (wrap . :any))) -- 2.40.1 [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #3: Implemented initial tangle header merge framework from existing code template --] [-- Type: text/x-patch, Size: 2565 bytes --] From 366a120a394d7783bf1640037ade31f826ef0277 Mon Sep 17 00:00:00 2001 From: MT <mtekman89@gmail.com> Date: Wed, 10 May 2023 17:41:37 +0200 Subject: [PATCH 2/3] * lisp/ob-core.el (org-babel-merge-params): Tangle header with exclusive parameters can now be parsed, following the template of :exports and :results --- lisp/ob-core.el | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/lisp/ob-core.el b/lisp/ob-core.el index 013a37ce5..ed31a9de1 100644 --- a/lisp/ob-core.el +++ b/lisp/ob-core.el @@ -2803,6 +2803,9 @@ parameters when merging lists." (exports-exclusive-groups (mapcar (lambda (group) (mapcar #'symbol-name group)) (cdr (assq 'exports org-babel-common-header-args-w-values)))) + (tangle-exclusive-groups + (mapcar (lambda (group) (mapcar #'symbol-name group)) + (cdr (assq 'tangle org-babel-common-header-args-w-values)))) (merge (lambda (exclusive-groups &rest result-params) ;; Maintain exclusivity of mutually exclusive parameters, @@ -2822,7 +2825,7 @@ parameters when merging lists." params ;Final parameters list. ;; Some keywords accept multiple values. We need to treat ;; them specially. - vars results exports) + vars results exports tangle) (dolist (plist plists) (dolist (pair plist) (pcase pair @@ -2873,6 +2876,12 @@ parameters when merging lists." (cond ((and value (functionp value)) (funcall value)) (value value) (t "")))))) + (`(:tangle . ,value) + (setq tangle (funcall merge + tangle-exclusive-groups + tangle + (split-string + (or value ""))))) ((or '(:dir . attach) '(:dir . "'attach")) (unless (org-attach-dir nil t) (error "No attachment directory for element (add :ID: or :DIR: property)")) @@ -2898,7 +2907,8 @@ parameters when merging lists." params))))) ;; Handle other special keywords, which accept multiple values. (setq params (nconc (list (cons :results (mapconcat #'identity results " ")) - (cons :exports (mapconcat #'identity exports " "))) + (cons :exports (mapconcat #'identity exports " ")) + (cons :tangle (mapconcat #'identity tangle " "))) params)) ;; Return merged params. (org-babel-eval-headers params))) -- 2.40.1 [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #4: Major rewrite of the merge function to handle :any keywords --] [-- Type: text/x-patch, Size: 4186 bytes --] From 6ce5313b7d3f0ab718072942f082bc259dccbae6 Mon Sep 17 00:00:00 2001 From: MT <mtekman89@gmail.com> Date: Wed, 10 May 2023 17:44:42 +0200 Subject: [PATCH 3/3] * lisp/ob-core.el (org-babel-merge-params): Major rewrite of the `merge' function, which adds the capability to process the :any keyword when merging parameters with exclusive groups. --- lisp/ob-core.el | 60 +++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 53 insertions(+), 7 deletions(-) diff --git a/lisp/ob-core.el b/lisp/ob-core.el index ed31a9de1..3d9000efc 100644 --- a/lisp/ob-core.el +++ b/lisp/ob-core.el @@ -2811,15 +2811,61 @@ parameters when merging lists." ;; Maintain exclusivity of mutually exclusive parameters, ;; as defined in EXCLUSIVE-GROUPS while merging lists in ;; RESULT-PARAMS. - (let (output) + (let (output group-any) (dolist (new-params result-params (delete-dups output)) (dolist (new-param new-params) - (dolist (exclusive-group exclusive-groups) - (when (member new-param exclusive-group) - (setq output (cl-remove-if - (lambda (o) (member o exclusive-group)) - output)))) - (push new-param output)))))) + (let (matched-param potential-any-param) + ;; new-param may be an :any value, so we check + ;; across all exclusive-groups. + ;; - If new-param matches one of groups, we + (dolist (exclusive-group exclusive-groups) + (if (member new-param exclusive-group) + (setq output (cl-remove-if + (lambda (o) (member o exclusive-group)) + output) + ;; Cancel any potential matches if it's caught + matched-param t + potential-any-param nil) + ;; If not a direct match, flag it as a potential + ;; :any match This can happen multiple times for + ;; each new-param, but only once for each + ;; exclusive-group. + (if (and (not matched-param) + (member ":any" exclusive-group)) + ;; At this point, the new-param has not yet matched + ;; anything in the N exclusive groups + ;; - We also assume that only 1 of these N groups + ;; has the :any keyword. + ;; - This point in the code can therefore be only + ;; reached once under this assumption. + ;; - We therefore setq instead of push + (setq potential-any-param (cons new-param + exclusive-group))))) + + ;; At this point we know whether new-param and 1 + ;; of the exclusive groups have an :any keyword - + ;; - Due to multiple new-params potentially being + ;; matches in the same group, we push these to a + ;; super group of "any" keywords, and process them + ;; later. + (if potential-any-param + (setq group-any potential-any-param) + ;; If the param isn't :any, add it to the output + ;; as a regular keyword + (push new-param output))))) + + (when group-any + ;; Whatever is leftover at this point are :any candidates. + ;; - We assume that last added is the most relevant and + ;; that everything else should be ignored + ;; - We add the first, and reject everything else in that + ;; exclusion group. + (push (car group-any) output) + (setq output (cl-remove-if + (lambda (o) (member o (cdr group-any))) + output))) + output))) + (variable-index 0) ;Handle positional arguments. clearnames params ;Final parameters list. -- 2.40.1 ^ permalink raw reply related [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-05-10 16:20 ` Mehmet Tekman @ 2023-05-12 12:33 ` Ihor Radchenko 2023-05-16 12:49 ` Mehmet Tekman 0 siblings, 1 reply; 77+ messages in thread From: Ihor Radchenko @ 2023-05-12 12:33 UTC (permalink / raw) To: Mehmet Tekman; +Cc: emacs-orgmode Mehmet Tekman <mtekman89@gmail.com> writes: >> It will be great if you could do it. >> I have other things to work on. > > Of course! I'm just a little unfamiliar on how one coordinates active > collaboration via mailing list :-) Usually, we leave working on the patches to one person - whoever is the most interested in the patch. Maintainers and other interested users provide comments and suggestions. > I've modified the `merge' function within `org-babel-merge-params' so > that the main logic now accumulates a list of potential candidates that > could be the :any keyword, and selects the last added candidate as the > match. I feel confused when reading the modified code. > It all seems to be passing tests, though I would like to add my toy.org > file to the org testing framework at some point. I recommend trying various edge cases with your patch. In particular: 1. Testing exclusive group inheritance when we inherit a header arg value that matches :any: #+PROPERTIES: header-args :tangle "foo.txt" #+begin_src ... :tangle no #+PROPERTIES: header-args :tangle no #+begin_src ... :tangle "foo.txt" #+PROPERTIES: header-args :tangle no * Heading :PROPERTIES: :header-args: :tangle "foo.txt" :END: #+begin_src ... :tangle yes 2. :tangle "file with spaces.txt" I feel that the following code is not reliable when we inherit exact and :any exclusive group members in alternations. > + (if (member new-param exclusive-group) > ... > + ;; Cancel any potential matches if it's caught > + matched-param t > + potential-any-param nil) > + ;; If not a direct match, flag it as a potential > + ;; :any match This can happen multiple times for > + ;; each new-param, but only once for each > + ;; exclusive-group. > + (if (and (not matched-param) > + (member ":any" exclusive-group)) -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-05-12 12:33 ` Ihor Radchenko @ 2023-05-16 12:49 ` Mehmet Tekman 2023-05-16 18:57 ` Ihor Radchenko 0 siblings, 1 reply; 77+ messages in thread From: Mehmet Tekman @ 2023-05-16 12:49 UTC (permalink / raw) To: Ihor Radchenko; +Cc: emacs-orgmode Hello, thanks for the last review and sorry for the lapse in communication. May is proving to be a very strange month. Ihor Radchenko <yantar92@posteo.net> writes: > Mehmet Tekman <mtekman89@gmail.com> writes: >> >> I've modified the `merge' function within `org-babel-merge-params' so >> that the main logic now accumulates a list of potential candidates that >> could be the :any keyword, and selects the last added candidate as the >> match. > > I feel confused when reading the modified code. > Okay, I guess comments are not my strong suit. I will try to remove any unnecessary comments and use better variable names. >> It all seems to be passing tests, though I would like to add my toy.org >> file to the org testing framework at some point. > > I recommend trying various edge cases with your patch. > In particular: > > 1. Testing exclusive group inheritance when we inherit a header arg > value that matches :any: > > #+PROPERTIES: header-args :tangle "foo.txt" > #+begin_src ... :tangle no > > #+PROPERTIES: header-args :tangle no > #+begin_src ... :tangle "foo.txt" > > #+PROPERTIES: header-args :tangle no > * Heading > > :PROPERTIES: > :header-args: :tangle "foo.txt" > :END: > #+begin_src ... :tangle yes > > 2. :tangle "file with spaces.txt" > I think before I do any more disastrous changes, I will try to expand the existing ert test suite to incorporate my toy org file and your above examples. That way I'd at least have a consistent framework to validate some of my work. > I feel that the following code is not reliable when we inherit exact and > :any exclusive group members in alternations. Ah, hmm - can you give me an example here? I thought the idea was that an :any string would only be invokable once for a given header parameter. Ihor Radchenko <yantar92@posteo.net> writes: >>> We should modify it. For example like the following: >>> >>> 1. We will assume that :any can only occur one time in the exclusive >>> groups. (Otherwise, there is no single definite way to parse header >>> arguments) Best, Mehmet ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-05-16 12:49 ` Mehmet Tekman @ 2023-05-16 18:57 ` Ihor Radchenko 2023-05-17 13:45 ` Mehmet Tekman 0 siblings, 1 reply; 77+ messages in thread From: Ihor Radchenko @ 2023-05-16 18:57 UTC (permalink / raw) To: Mehmet Tekman; +Cc: emacs-orgmode Mehmet Tekman <mtekman89@gmail.com> writes: >> I feel confused when reading the modified code. >> > > Okay, I guess comments are not my strong suit. I will try to remove any > unnecessary comments and use better variable names. No, comments were useful. They revealed that our understanding of the expected behaviour might differ. >> I recommend trying various edge cases with your patch. >> In particular: >> >> 1. Testing exclusive group inheritance when we inherit a header arg >> value that matches :any: >> >> #+PROPERTIES: header-args :tangle "foo.txt" >> #+begin_src ... :tangle no >> >> #+PROPERTIES: header-args :tangle no >> #+begin_src ... :tangle "foo.txt" >> >> #+PROPERTIES: header-args :tangle no >> * Heading >> >> :PROPERTIES: >> :header-args: :tangle "foo.txt" >> :END: >> #+begin_src ... :tangle yes > ... >> I feel that the following code is not reliable when we inherit exact and >> :any exclusive group members in alternations. > > Ah, hmm - can you give me an example here? I thought the idea was that > an :any string would only be invokable once for a given header > parameter. See the above 3 examples: (1) "foo.txt" shadowed by "no"; (2) "no" shadowed by "foo.txt"; (3) "no" shadowed by "foo.txt" then shadowed by "yes". You can also consider "foo.txt" shadowed by "bar.txt" and other variations. -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-05-16 18:57 ` Ihor Radchenko @ 2023-05-17 13:45 ` Mehmet Tekman 2023-05-18 10:30 ` Ihor Radchenko 0 siblings, 1 reply; 77+ messages in thread From: Mehmet Tekman @ 2023-05-17 13:45 UTC (permalink / raw) To: Ihor Radchenko; +Cc: emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 2441 bytes --] Hello again, Ihor Radchenko <yantar92@posteo.net> writes: >>> I recommend trying various edge cases with your patch. >>> In particular: >>> >>> 1. Testing exclusive group inheritance when we inherit a header arg >>> value that matches :any: >>> >>> #+PROPERTIES: header-args :tangle "foo.txt" >>> #+begin_src ... :tangle no >>> >>> #+PROPERTIES: header-args :tangle no >>> #+begin_src ... :tangle "foo.txt" >>> >>> #+PROPERTIES: header-args :tangle no >>> * Heading >>> >>> :PROPERTIES: >>> :header-args: :tangle "foo.txt" >>> :END: >>> #+begin_src ... :tangle yes >> ... >>> I feel that the following code is not reliable when we inherit exact and >>> :any exclusive group members in alternations. >> >> Ah, hmm - can you give me an example here? I thought the idea was that >> an :any string would only be invokable once for a given header >> parameter. > > See the above 3 examples: (1) "foo.txt" shadowed by "no"; (2) "no" > shadowed by "foo.txt"; (3) "no" shadowed by "foo.txt" then shadowed by > "yes". > > You can also consider "foo.txt" shadowed by "bar.txt" and other > variations. I've created a small patch that contains a single ert test function that checks an example org file I made (based on the above as well as my own toy file) to try to validate the `org-babel-merge-params' rewrite. I've attached it below in case you want to test it... ...but there were a few cases where I wasn't entirely sure what the result of the merge was supposed to be: For example, a document with: > #+TITLE: Header tests > #+PROPERTY: header-args :tangle /tmp/default_tangle.txt > > * Inherit tangle header from document > > #+begin_src conf > (:tangle . /tmp/default_tangle.txt) > #+end_src I would expect the output of: (assoc :tangle (nth 2 (org-babel-get-src-block-info))) within that block to evaluate to the contents written in that block. Instead it evaluates to `(:tangle . no)' when run in a vanilla Emacs. Is this expected? Another example: > * Header args overwritten by local header > :PROPERTIES: > :header-args: :tangle "foo.txt" > :END: > #+begin_src :tangle yes > (:tangle . foo.txt) > #+end_src > > ** Inherited header > > #+begin_src :tangle "file with spaces.txt" > (:tangle . "file with spaces.txt") > #+end_src The first block correctly gives "foo.txt" under vanilla Emacs, but the second block also gives "foo.txt". Is this expected behaviour? Best, Mehmet [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: test org file and associated ert test function --] [-- Type: text/x-patch, Size: 7406 bytes --] From 2f040db1197f835262d32e7ced857f2a47dd8ca8 Mon Sep 17 00:00:00 2001 From: MT <mtekman89@gmail.com> Date: Wed, 17 May 2023 13:16:08 +0200 Subject: [PATCH 4/4] * testing/examples/header_test.org: New example test file Contains several source blocks with document, heading, block header arguments * testing/lisp/test-ob.el (test-ob/merge-params): New test function for `org-babel-merge-params' validation. This function takes a list of all the ID properties for each of the blocks in `header_test.org', along with a symbol or list of header properties to test against. The expected value is written within the block contents. --- testing/examples/header_test.org | 127 +++++++++++++++++++++++++++++++ testing/lisp/test-ob.el | 44 +++++++++++ 2 files changed, 171 insertions(+) create mode 100644 testing/examples/header_test.org diff --git a/testing/examples/header_test.org b/testing/examples/header_test.org new file mode 100644 index 000000000..9a33661be --- /dev/null +++ b/testing/examples/header_test.org @@ -0,0 +1,127 @@ +#+TITLE: Header tests +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt + +The text contents in each block are tested against the output of +=(assoc <prop> (nth 2 (org-babel-get-src-block-info)))= + +Multiple header properties can be tested by separating each property +output with a **newline followed by exactly two spaces**. + +* Inherit tangle header from document +:PROPERTIES: +:ID: a41c3238-f457-4769-b10b-8d50e9d386a1 +:END: + +#+begin_src conf + (:tangle . /tmp/default_tangle.txt) +#+end_src + +* Inherit tangle header but use local sync action +:PROPERTIES: +:ID: debf7bf8-e5eb-412d-9127-57950a27c390 +:END: + +#+begin_src conf :tangle skip + (:tangle . /tmp/default_tangle.txt skip) +#+end_src + +* Use local tangle file and sync action +:PROPERTIES: +:ID: 1ca658c1-0dfd-42a5-bbe3-305582deeb00 +:END: ++ Ignore document header completely. +#+begin_src conf :tangle randomfile sync + (:tangle . randomfile sync) +#+end_src + +* Use local tangle file and sync action 2 +:PROPERTIES: +:header-args: :tangle "newfile.txt" import +:END: +** Subheading +:PROPERTIES: +:ID: 602503b8-6657-49c6-9cac-7edac396f725 +:END: ++ Ignore document header and parent header completely. +#+begin_src conf :tangle randomfile sync + (:tangle . randomfile sync) +#+end_src + + +* Test tangle and results param together +:PROPERTIES: +:ID: 4fb9938c-aec0-479f-bbc6-6b7a4228d9c2 +:END: +#+begin_src conf :tangle randomfile + (:tangle . randomfile) + (:results . replace) +#+end_src + +* Inherit the tangle file name, take the last sync action +:PROPERTIES: +:ID: 7a98b56d-e59f-426d-bd58-f93bb22cf57b +:END: ++ Ignores import +#+begin_src conf :tangle import export + (:tangle . /tmp/default_tangle.txt export) +#+end_src + +* Take the last local tangle file name and the last sync action +:PROPERTIES: +:ID: cd85e03a-1a4c-45d5-ac33-90d96999b665 +:END: ++ Ignores fname1 and sync +#+begin_src conf :tangle fname1 fname2 sync export + (:tangle . fname2 export) +#+end_src + +* Merge document results param and local results param +:PROPERTIES: +:ID: f4e4e422-029b-4ef7-b594-cd70cff2d943 +:END: + +#+begin_src sh :results file wrap + (:results . wrap file replace) + (:exports . code) +#+end_src + +* All tangle headers should be ignored (ITS FAILING HERE) +:PROPERTIES: +:ID: 9715d355-009c-4188-8b97-bcbebaeee86f +:END: + +#+begin_src conf :tangle no + (:tangle . no) +#+end_src + +* Tangle filename ignores document and heading args, inherits heading exports +:PROPERTIES: +:ID: 1a3b5565-27b5-450e-a2c5-7f95a8142f3b +:header-args: :tangle no :exports verbatim +:END: + +#+begin_src conf :tangle "foo.txt" :comments link + (:tangle . foo.txt) + (:exports . verbatim code) + (:comments . link) +#+end_src + +* Heading tangle parameter is not overwritten by local "yes" +:PROPERTIES: +:ID: fe54b2be-d9f1-40b4-83df-49501e69d083 +:header-args: :tangle "foo.txt" +:END: +#+begin_src :tangle yes + (:tangle . foo.txt) +#+end_src + +** Local tangle filename with spaces overwrites parent foo.txt +:PROPERTIES: +:ID: ab8af294-c586-4ec8-9f45-3c3baaeb184d +:END: ++ The expected hierarchy is =/tmp/default_tangle.txt= is supplanted by + =foo.txt= which is supplanted by =file with spaces.txt= + +#+begin_src :tangle "file with spaces.txt" + (:tangle . "file with spaces.txt") +#+end_src diff --git a/testing/lisp/test-ob.el b/testing/lisp/test-ob.el index c8dbd44f4..e05dd083a 100644 --- a/testing/lisp/test-ob.el +++ b/testing/lisp/test-ob.el @@ -314,6 +314,50 @@ this is simple" (org-babel-next-src-block) (should (= 14 (org-babel-execute-src-block))))) +(ert-deftest test-ob/merge-params () + "Test the output of merging multiple header parameters. The +expected output is given in the contents of the source code block +at each header. The desired test header parameters are given +either as a symbol or a list in the `idtest-alist' variable. +Multiple header parameters must be separated by a newline and +exactly two spaces in the block contents." + (let ((idtest-alist '(("a41c3238-f457-4769-b10b-8d50e9d386a1" . :tangle) + ("debf7bf8-e5eb-412d-9127-57950a27c390" . :tangle) + ("1ca658c1-0dfd-42a5-bbe3-305582deeb00" . :tangle) + ("602503b8-6657-49c6-9cac-7edac396f725" . :tangle) + ("4fb9938c-aec0-479f-bbc6-6b7a4228d9c2" . (:tangle :results)) + ("7a98b56d-e59f-426d-bd58-f93bb22cf57b" . :tangle) + ("cd85e03a-1a4c-45d5-ac33-90d96999b665" . :tangle) + ("f4e4e422-029b-4ef7-b594-cd70cff2d943" . (:results :exports)) + ("9715d355-009c-4188-8b97-bcbebaeee86f" . :tangle) + ("1a3b5565-27b5-450e-a2c5-7f95a8142f3b" . (:tangle :exports :comments)) + ("fe54b2be-d9f1-40b4-83df-49501e69d083" . :tangle) + ("ab8af294-c586-4ec8-9f45-3c3baaeb184d" . :tangle))) + buffer + failed-ids) + (unwind-protect + (org-test-in-example-file (expand-file-name "header_test.org" org-test-example-dir) + (setq buffer (current-buffer)) + (dolist (testpair idtest-alist) + (let ((id (car testpair)) + (prop (cdr testpair))) + (org-test-at-id id) + (org-babel-next-src-block) + (unless (string= + (if (string= "symbol" (type-of prop)) + (format "%s" (assoc prop + (nth 2 (org-babel-get-src-block-info)))) + (mapconcat (lambda (x) (format "%s" + (assoc x + (nth 2 (org-babel-get-src-block-info))))) + prop "\n ")) ;; newline with exactly two spaces. + (string-trim (org-element-property :value (org-element-at-point)))) + (push id failed-ids)))) + (kill-buffer buffer) + (if failed-ids + (user-error "%d Failed Blocks: %s" (length failed-ids) failed-ids)) + (should (= 0 (length failed-ids) )))))) + (ert-deftest test-ob/inline-src-blocks () (should (= 1 -- 2.40.1 ^ permalink raw reply related [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-05-17 13:45 ` Mehmet Tekman @ 2023-05-18 10:30 ` Ihor Radchenko 2023-05-19 7:10 ` Mehmet Tekman 0 siblings, 1 reply; 77+ messages in thread From: Ihor Radchenko @ 2023-05-18 10:30 UTC (permalink / raw) To: Mehmet Tekman; +Cc: emacs-orgmode Mehmet Tekman <mtekman89@gmail.com> writes: >> See the above 3 examples: (1) "foo.txt" shadowed by "no"; (2) "no" >> shadowed by "foo.txt"; (3) "no" shadowed by "foo.txt" then shadowed by >> "yes". >> >> You can also consider "foo.txt" shadowed by "bar.txt" and other >> variations. > > I've created a small patch that contains a single ert test function that > checks an example org file I made (based on the above as well as my own > toy file) to try to validate the `org-babel-merge-params' rewrite. > > I've attached it below in case you want to test it... Is the patch testing your code? (Note on the ert tests: we generally prefer `org-test-with-temp-text' - the old approach with test IDs is not very readable because one needs to search those IDs manually when debugging test failures.) > ...but there were a few cases where I wasn't entirely sure what the > result of the merge was supposed to be: > > For example, a document with: > >> #+TITLE: Header tests >> #+PROPERTY: header-args :tangle /tmp/default_tangle.txt >> >> * Inherit tangle header from document >> >> #+begin_src conf >> (:tangle . /tmp/default_tangle.txt) >> #+end_src > > I would expect the output of: > > (assoc :tangle (nth 2 (org-babel-get-src-block-info))) > > within that block to evaluate to the contents written in that > block. Instead it evaluates to `(:tangle . no)' when run in a vanilla > Emacs. Is this expected? You forgot C-c C-c on the PROPERTY line, or re-opening the file. #+PROPERTY lines are not automatically parsed when you add them. They are only parsed upon explicit request (C-c C-c) or when opening the file. > Another example: > >> * Header args overwritten by local header >> :PROPERTIES: >> :header-args: :tangle "foo.txt" >> :END: >> #+begin_src :tangle yes >> (:tangle . foo.txt) >> #+end_src >> >> ** Inherited header >> >> #+begin_src :tangle "file with spaces.txt" >> (:tangle . "file with spaces.txt") >> #+end_src > > The first block correctly gives "foo.txt" under vanilla Emacs, but the > second block also gives "foo.txt". Is this expected behaviour? You forgot to specify the source block languages. Org sees your example as two src blocks of language ":tangle" with no proper header arguments. (See M-x org-lint) If you add a language, like #+begin_src conf :tangle yes (:tangle . foo.txt) #+end_src The expected result is (:tangle . "yes") for the first block and (:tangle . "file with spaces.txt") for the second block. -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-05-18 10:30 ` Ihor Radchenko @ 2023-05-19 7:10 ` Mehmet Tekman 2023-07-15 12:38 ` Ihor Radchenko 0 siblings, 1 reply; 77+ messages in thread From: Mehmet Tekman @ 2023-05-19 7:10 UTC (permalink / raw) To: Ihor Radchenko; +Cc: emacs-orgmode Ah, running `org-lint' and correcting the header line as well as adding languages to the other blocks fixed the mistakes - thanks for this, I thought I was losing my mind. Ihor Radchenko <yantar92@posteo.net> writes: > The expected result is (:tangle . "yes") for the first block and > (:tangle . "file with spaces.txt") for the second block. I also think I did not realise what "yes" did until now. I assumed it defaulted to the last given filename in the hierarchy, but after reading the manual I see that it just sets it to the name of the org file. > Is the patch testing your code? > > (Note on the ert tests: we generally prefer `org-test-with-temp-text' - > the old approach with test IDs is not very readable because one needs to > search those IDs manually when debugging test failures.) > It is, but please ignore it for now. I will rewrite to to conform to the `org-test-with-temp-text' examples. Best, Mehmet ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-05-19 7:10 ` Mehmet Tekman @ 2023-07-15 12:38 ` Ihor Radchenko 2023-07-16 9:42 ` Mehmet Tekman 0 siblings, 1 reply; 77+ messages in thread From: Ihor Radchenko @ 2023-07-15 12:38 UTC (permalink / raw) To: Mehmet Tekman; +Cc: emacs-orgmode Mehmet Tekman <mtekman89@gmail.com> writes: > ... I will rewrite to to conform to the > `org-test-with-temp-text' examples. Two months have passed since the last message in this thread. Mehmet, have you had a chance to work on this? May you need any help? -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-07-15 12:38 ` Ihor Radchenko @ 2023-07-16 9:42 ` Mehmet Tekman 2023-07-17 11:29 ` Mehmet Tekman 0 siblings, 1 reply; 77+ messages in thread From: Mehmet Tekman @ 2023-07-16 9:42 UTC (permalink / raw) To: Ihor Radchenko; +Cc: emacs-orgmode Apologies, this patch has been burning in the back of mind for a while now, but I keep getting distracted with other random side projects. I appreciate the wake up call, and I will refocus my efforts next week in my free time. Best, Mehmet ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-07-16 9:42 ` Mehmet Tekman @ 2023-07-17 11:29 ` Mehmet Tekman 2023-07-18 8:47 ` Ihor Radchenko 0 siblings, 1 reply; 77+ messages in thread From: Mehmet Tekman @ 2023-07-17 11:29 UTC (permalink / raw) To: Ihor Radchenko; +Cc: emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 1457 bytes --] Hello again! > Ihor Radchenko <yantar92@posteo.net> writes: > > Two months have passed since the last message in this thread. > Mehmet, have you had a chance to work on this? May you need any help? General recap of where we are so far with this patch: The tangle-sync mode I wished to implement had some ambiguities when resolving `:tangle' headers. This turned out to be a general problem for any header that had multiple parameters, so the merge-params function was re-written. This introduced some ambiguities in itself, so we decided to write a better testing framework for it. > Mehmet Tekman <mtekman89@gmail.com> writes: > > ... I will rewrite to to conform to the > `org-test-with-temp-text' examples. So I've just rewritten the `merge-params' testing function I wrote before, this time using the `org-test-with-temp-text' method/macro. I seem to be getting 5 failing tests for reasons that I'm not quite sure why. I've tested it against upstream/main, but still getting the same errors. Maybe you can try it and see where I'm going wrong? The patch does not rely on any previous patches Best, Mehmet On Sun, 16 Jul 2023 at 11:42, Mehmet Tekman <mtekman89@gmail.com> wrote: > > Apologies, this patch has been burning in the back of mind for a > while now, but I keep getting distracted with other random side > projects. > > I appreciate the wake up call, and I will refocus my efforts next > week in my free time. > > Best, > Mehmet [-- Attachment #2: 0001-testing-lisp-test-ob.el-test-ob-merge-params-new-test.patch --] [-- Type: text/x-patch, Size: 4915 bytes --] diff --git a/testing/lisp/test-ob.el b/testing/lisp/test-ob.el index c8dbd44f4..9ba2d09d3 100644 --- a/testing/lisp/test-ob.el +++ b/testing/lisp/test-ob.el @@ -314,6 +314,135 @@ this is simple" (org-babel-next-src-block) (should (= 14 (org-babel-execute-src-block))))) + +(ert-deftest test-ob/merge-params () + "Test the output of merging multiple header parameters. The +expected output is given in the contents of the source code block +in each test. The desired test header parameters are given +either as a symbol or a list in the `idtest-alist' variable. +Multiple header parameters must be separated by a newline and +exactly two spaces in the block contents." + (let ((idtest-alist '((inherit-document-header-args + :tangle "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* One +#+begin_src conf + (:tangle . /tmp/default_tangle.txt) +#+end_src") + (inherit-document-header-with-local-sync-action + :tangle "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* One +#+begin_src conf :tangle skip + (:tangle . /tmp/default_tangle.txt skip) +#+end_src") + (override-document-header-with-local-tfile + :tangle "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* One +#+begin_src conf :tangle randomfile sync + (:tangle . randomfile sync) +#+end_src") + (override-document-and-parent-header-with-local-tfile-and-action + :tangle "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* One +:PROPERTIES: +:header-args: :tangle \"newfile.txt\" import +:END: +** Two +#+begin_src conf :tangle randomfile sync + (:tangle . randomfile sync) +#+end_src") + (test-tangle-and-default-results-param-together + (:tangle :results) "\ +* One +#+begin_src conf :tangle randomfile + (:tangle . randomfile) + (:results . replace) +#+end_src") + (inherit-document-tfile-take-only-last-local-sync-action + :tangle "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* One +#+begin_src conf :tangle import export + (:tangle . /tmp/default_tangle.txt export) +#+end_src") + (ignore-document-header-take-last-tfile-and-sync-action + :tangle "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* One +#+begin_src conf :tangle fname1 fname2 sync export + (:tangle . fname2 export) +#+end_src") + (test-results-and-exports + (:results :exports) "\ +* One +#+begin_src sh :results file wrap + (:results . wrap file replace) + (:exports . code) +#+end_src") + (do-not-tangle-this-block + :tangle "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* One +#+begin_src conf :tangle no + (:tangle . no) +#+end_src") + (test-tangle-exports-and-comments + (:tangle :exports :comments) "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* One +:PROPERTIES: +:header-args: :tangle no :exports verbatim +:END: +#+begin_src conf :tangle \"foo.txt\" :comments link + (:tangle . foo.txt) + (:exports . verbatim code) + (:comments . link) +#+end_src") + (override-document-and-heading-tfile-with-yes + :tangle "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* One +:PROPERTIES: +:header-args: :tangle \"foo.txt\" +:END: +#+begin_src :tangle yes + (:tangle . yes) +#+end_src") + (tangle-file-with-spaces + :tangle "\ +* One +:PROPERTIES: +:header-args: :tangle \"foo.txt\" +:END: +** Two +#+begin_src :tangle \"file with spaces.txt\" + (:tangle . \"file with spaces.txt\") +#+end_src"))) + failed-ids) + (dolist (testpair idtest-alist) + (let ((test-name (nth 0 testpair)) + (test-prop (nth 1 testpair)) + (test-cont (nth 2 testpair))) + (org-test-with-temp-text + test-cont + (org-babel-next-src-block) + (unless + (string= + (if (string= "symbol" (type-of test-prop)) + (format "%s" (assoc test-prop (nth 2 (org-babel-get-src-block-info)))) + (mapconcat + (lambda (x) (format "%s" (assoc x (nth 2 (org-babel-get-src-block-info))))) + test-prop "\n ")) ;; newline with exactly two spaces. + (string-trim (org-element-property :value (org-element-at-point)))) + (push test-name failed-ids))))) + (if failed-ids + (user-error "%d Failed Blocks: %s" (length failed-ids) failed-ids)) + (should (= 0 (length failed-ids))))) + + (ert-deftest test-ob/inline-src-blocks () (should (= 1 ^ permalink raw reply related [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-07-17 11:29 ` Mehmet Tekman @ 2023-07-18 8:47 ` Ihor Radchenko 2023-07-21 8:48 ` Mehmet Tekman 0 siblings, 1 reply; 77+ messages in thread From: Ihor Radchenko @ 2023-07-18 8:47 UTC (permalink / raw) To: Mehmet Tekman; +Cc: emacs-orgmode Mehmet Tekman <mtekman89@gmail.com> writes: >> ... I will rewrite to to conform to the >> `org-test-with-temp-text' examples. > > So I've just rewritten the `merge-params' testing function I > wrote before, this time using the `org-test-with-temp-text' > method/macro. Side note: You are re-implementing the already available ERT features for failed test reporting. Instead of controlling which tests failed manually, you should better follow the example of all other tests and simply use a sequence of `should'/`should-not' forms. If you did that, ERT would by itself report which tests fail and why. > I seem to be getting 5 failing tests for reasons that I'm not > quite sure why. I've tested it against upstream/main, but still > getting the same errors. Maybe you can try it and see where I'm > going wrong? I am getting the following failures: 6 Failed Blocks: (tangle-file-with-spaces override-document-and-heading-tfile-with-yes ignore-document-header-take-last-tfile-and-sync-action inherit-document-tfile-take-only-last-local-sync-action inherit-document-header-with-local-sync-action inherit-document-header-args) > + (let ((idtest-alist '((inherit-document-header-args > + :tangle "\ > +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt > +* One > +#+begin_src conf > + (:tangle . /tmp/default_tangle.txt) > +#+end_src") This is failing because of subtlety in `org-test-with-temp-text' that inserts text after activating org-mode. PROPERTY lines are only computed when org-mode is activated, unless you explicitly call `org-set-regexps-and-options' to refresh them. Your code does not refresh those and PROPERTY line is not being handled by Org. Same story for other tests with PROPERTY. > + (tangle-file-with-spaces > + :tangle "\ > +* One > +:PROPERTIES: > +:header-args: :tangle \"foo.txt\" > +:END: > +** Two > +#+begin_src :tangle \"file with spaces.txt\" > + (:tangle . \"file with spaces.txt\") > +#+end_src"))) This fails because you did not provide LANG for source block. override-document-and-heading-tfile-with-yes test is also missing LANG. P.S. There going to be Emacs meetup this Saturday, July 22 (https://emacs-apac.gitlab.io/announcements/july-2023/). I plan to be there and, if you need it, we can try to resolve difficulties in more interactive way. -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-07-18 8:47 ` Ihor Radchenko @ 2023-07-21 8:48 ` Mehmet Tekman 2023-07-22 8:02 ` Ihor Radchenko 0 siblings, 1 reply; 77+ messages in thread From: Mehmet Tekman @ 2023-07-21 8:48 UTC (permalink / raw) To: Ihor Radchenko; +Cc: emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 1944 bytes --] Hello > Ihor Radchenko <yantar92@posteo.net> writes: > > P.S. There going to be Emacs meetup this Saturday, July 22 > (https://emacs-apac.gitlab.io/announcements/july-2023/). I plan to be > there and, if you need it, we can try to resolve difficulties in more > interactive way. I am currently travelling this weekend, otherwise I happily would attend so that this patch could speed forward. Next month maybe? > This is failing because of subtlety in > `org-test-with-temp-text' that inserts text after activating > org-mode. PROPERTY lines are only computed when org-mode is > activated, unless you explicitly call > `org-set-regexps-and-options' to refresh them. Okay, it's working as intended now. > This fails because you did not provide LANG for source block. > override-document-and-heading-tfile-with-yes test is also > missing LANG. Thanks, I thought I fixed this last time you mentioned it but apparently only in my mind... > Side note: You are re-implementing the already available ERT > features for failed test reporting. Instead of controlling > which tests failed manually, you should better follow the > example of all other tests and simply use a sequence of > `should'/`should-not' forms. Okay, I've re-written this to conform to the `should' macros. I *really* wish these macros could be named so that I could debug failing statements better, but for now numbering them in the comments and using the `l' binding works well enough for debugging. I have two tests that I've set to "should-not" pass for now, but once the correct merging behaviour has been implemented, I believe that I will change them to "should" pass statements: - 9. do-not-tangle-this-block - 12. tangle-file-with-spaces Attached is the new diff (travelling makes it hard to write meaningful commit messages so I did not use git-format-patch), which should be used against the upstream/main. Looking forward to further feedback! Best, Mehmet [-- Attachment #2: 0001-testing-lisp-test-ob.el-test-ob-merge-params-new-test-v2.patch --] [-- Type: text/x-patch, Size: 5228 bytes --] diff --git a/testing/lisp/test-ob.el b/testing/lisp/test-ob.el index c8dbd44f4..2b9326865 100644 --- a/testing/lisp/test-ob.el +++ b/testing/lisp/test-ob.el @@ -314,6 +314,151 @@ this is simple" (org-babel-next-src-block) (should (= 14 (org-babel-execute-src-block))))) + +(ert-deftest test-ob/merge-params () + "Test the output of merging multiple header parameters. The +expected output is given in the contents of the source code block +in each test. The desired test header parameters are given +either as a symbol or a list in the `idtest-alist' variable. +Multiple header parameters must be separated by a newline and +exactly two spaces in the block contents." + (let ((test-the-result + (lambda (test-prop) + (org-babel-next-src-block) + (org-set-regexps-and-options) + (string= + (if (string= "symbol" (type-of test-prop)) + (format "%s" (assoc test-prop (nth 2 (org-babel-get-src-block-info)))) + (mapconcat + (lambda (x) (format "%s" (assoc x (nth 2 (org-babel-get-src-block-info))))) + test-prop "\n ")) ;; newline with exactly two spaces. + (string-trim (org-element-property :value (org-element-at-point))))))) + (should ;; 1. inherit-document-header-args + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* One +#+begin_src conf + (:tangle . /tmp/default_tangle.txt) +#+end_src" + (funcall test-the-result :tangle))) + (should ;; 2. inherit-document-header-with-local-sync-action + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* One +#+begin_src conf :tangle skip + (:tangle . /tmp/default_tangle.txt skip) +#+end_src" + (funcall test-the-result :tangle))) + (should ;; 3. override-document-header-with-local-tfile + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* One +#+begin_src conf :tangle randomfile sync + (:tangle . randomfile sync) +#+end_src" + (funcall test-the-result :tangle))) + (should ;; 4. override-document-and-parent-header-with-local-tfile-and-action + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* One +:PROPERTIES: +:header-args: :tangle \"newfile.txt\" import +:END: +** Two +#+begin_src conf :tangle randomfile sync + (:tangle . randomfile sync) +#+end_src" + (funcall test-the-result :tangle))) + (should ;; 5. test-tangle-and-default-results-param-together + (org-test-with-temp-text + "\ +* One +#+begin_src conf :tangle randomfile + (:tangle . randomfile) + (:results . replace) +#+end_src" + (funcall test-the-result '(:tangle :results)))) + (should ;; 6. inherit-document-tfile-take-only-last-local-sync-action + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* One +#+begin_src conf :tangle import export + (:tangle . /tmp/default_tangle.txt export) +#+end_src" + (funcall test-the-result :tangle))) + (should ;; 7. ignore-document-header-take-last-tfile-and-sync-action + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* One +#+begin_src conf :tangle fname1 fname2 sync export + (:tangle . fname2 export) +#+end_src" + (funcall test-the-result :tangle))) + (should ;; 8. test-results-and-exports + (org-test-with-temp-text + "\ +* One +#+begin_src sh :results file wrap + (:results . wrap file replace) + (:exports . code) +#+end_src" + (funcall test-the-result '(:results :exports)))) + (should-not ;; 9. do-not-tangle-this-block -- + ;; THIS SHOULD NOT FAIL WITH NEW MERGE FUNCTION. + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* One +#+begin_src conf :tangle no + (:tangle . no) +#+end_src" + (funcall test-the-result :tangle))) + (should ;; 10. test-tangle-exports-and-comments + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* One +:PROPERTIES: +:header-args: :tangle no :exports verbatim +:END: +#+begin_src conf :tangle \"foo.txt\" :comments link + (:tangle . foo.txt) + (:exports . verbatim code) + (:comments . link) +#+end_src" + (funcall test-the-result '(:tangle :exports :comments)))) + (should ;; 11. override-document-and-heading-tfile-with-yes + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* One +:PROPERTIES: +:header-args: :tangle \"foo.txt\" +:END: +#+begin_src conf :tangle yes + (:tangle . foo.txt) +#+end_src" + (funcall test-the-result :tangle))) + (should-not ;; 12. tangle-file-with-spaces + ;; THIS SHOULD NOT FAIL WITH NEW MERGE FUNCTION. + (org-test-with-temp-text + "\ +* One +:PROPERTIES: +:header-args: :tangle \"foo.txt\" +:END: +** Two +#+begin_src conf :tangle \"file with spaces.txt\" + (:tangle . \"file with spaces.txt\") +#+end_src" + (funcall test-the-result :tangle))))) + (ert-deftest test-ob/inline-src-blocks () (should (= 1 ^ permalink raw reply related [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-07-21 8:48 ` Mehmet Tekman @ 2023-07-22 8:02 ` Ihor Radchenko 2023-07-25 11:19 ` Mehmet Tekman 0 siblings, 1 reply; 77+ messages in thread From: Ihor Radchenko @ 2023-07-22 8:02 UTC (permalink / raw) To: Mehmet Tekman; +Cc: emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 1689 bytes --] Mehmet Tekman <mtekman89@gmail.com> writes: >> P.S. There going to be Emacs meetup this Saturday, July 22 >> (https://emacs-apac.gitlab.io/announcements/july-2023/). I plan to be >> there and, if you need it, we can try to resolve difficulties in more >> interactive way. > > I am currently travelling this weekend, otherwise I happily would > attend so that this patch could speed forward. Next month maybe? Another meetup is on July 26. https://emacs-berlin.org/ Or we can arrange a jitsi call. >> Side note: You are re-implementing the already available ERT >> features for failed test reporting. Instead of controlling >> which tests failed manually, you should better follow the >> example of all other tests and simply use a sequence of >> `should'/`should-not' forms. > > Okay, I've re-written this to conform to the `should' macros. I > *really* wish these macros could be named so that I could debug > failing statements better, but for now numbering them in the > comments and using the `l' binding works well enough for > debugging. To name them, you can use separate ert-deftest statements. Also, see the attached modified version of your diff - I made it so that `should' failures immediately provide some clue about expected result value, that can help to identify which of the should clauses fails. > I have two tests that I've set to "should-not" pass for now, but > once the correct merging behaviour has been implemented, I > believe that I will change them to "should" pass statements: > > - 9. do-not-tangle-this-block > - 12. tangle-file-with-spaces The tests are indeed not passing on vanilla Org. I assume that you tested them with your other patch applied. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-testing-lisp-test-ob.el-test-ob-merge-params-new-test-v3-Ihor.patch --] [-- Type: text/x-patch, Size: 5575 bytes --] diff --git a/testing/lisp/test-ob.el b/testing/lisp/test-ob.el index c8dbd44f4..de92cb90b 100644 --- a/testing/lisp/test-ob.el +++ b/testing/lisp/test-ob.el @@ -314,6 +314,147 @@ (ert-deftest test-ob/parse-header-args2 () (org-babel-next-src-block) (should (= 14 (org-babel-execute-src-block))))) +(defun test-ob/get-src-block-property (properties) + "Get plist of PROPERTIES and values for the first src block in buffer. +PROPERTIES is a list of property keywords or a single keyword." + (org-with-wide-buffer + (goto-char (point-min)) + (org-babel-next-src-block) + (org-set-regexps-and-options) + (let ((all-props (nth 2 (org-babel-get-src-block-info)))) + (if (listp properties) + (apply #'nconc (mapcar (lambda (p) (list p (cdr (assoc p all-props)))) properties)) + (list properties (cdr (assoc properties all-props))))))) + +(ert-deftest test-ob/merge-params () + "Test the output of merging multiple header parameters. The +expected output is given in the contents of the source code block +in each test. The desired test header parameters are given +either as a symbol or a list in the `idtest-alist' variable. +Multiple header parameters must be separated by a newline and +exactly two spaces in the block contents." + (should ;; 1. inherit-document-header-args + (equal '(:tangle "/tmp/default_tangle.txt") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* One +#+begin_src conf +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 2. inherit-document-header-with-local-sync-action + (equal '(:tangle "/tmp/default_tangle.txt skip") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* One +#+begin_src conf :tangle skip +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 3. override-document-header-with-local-tfile + (equal '(:tangle "randomfile sync") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* One +#+begin_src conf :tangle randomfile sync +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 4. override-document-and-parent-header-with-local-tfile-and-action + (equal '(:tangle "randomfile sync") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* One +:PROPERTIES: +:header-args: :tangle \"newfile.txt\" import +:END: +** Two +#+begin_src conf :tangle randomfile sync +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 5. test-tangle-and-default-results-param-together + (equal '(:tangle "randomfile" :results "replace") + (org-test-with-temp-text + "\ +* One +#+begin_src conf :tangle randomfile +#+end_src" + (test-ob/get-src-block-property '(:tangle :results))))) + (should ;; 6. inherit-document-tfile-take-only-last-local-sync-action + (equal '(:tangle "/tmp/default_tangle.txt export") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* One +#+begin_src conf :tangle import export +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 7. ignore-document-header-take-last-tfile-and-sync-action + (equal '(:tangle "fname2 export") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* One +#+begin_src conf :tangle fname1 fname2 sync export +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 8. test-results-and-exports + (equal '(:results "wrap file replace" :exports "code") + (org-test-with-temp-text + "\ +* One +#+begin_src sh :results file wrap +#+end_src" + (test-ob/get-src-block-property '(:results :exports))))) + (should-not ;; 9. do-not-tangle-this-block -- + (equal '(:tangle "no") + ;; THIS SHOULD NOT FAIL WITH NEW MERGE FUNCTION. + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* One +#+begin_src conf :tangle no +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 10. test-tangle-exports-and-comments + (equal '(:tangle "foo.txt" :exports "verbatim code" :comments "link") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* One +:PROPERTIES: +:header-args: :tangle no :exports verbatim +:END: +#+begin_src conf :tangle \"foo.txt\" :comments link +#+end_src" + (test-ob/get-src-block-property '(:tangle :exports :comments))))) + (should ;; 11. override-document-and-heading-tfile-with-yes + (equal '(:tangle "foo.txt") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* One +:PROPERTIES: +:header-args: :tangle \"foo.txt\" +:END: +#+begin_src conf :tangle yes +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should-not ;; 12. tangle-file-with-spaces + ;; THIS SHOULD NOT FAIL WITH NEW MERGE FUNCTION. + (equal '(:tangle "file with spaces.txt") + (org-test-with-temp-text + "\ +* One +:PROPERTIES: +:header-args: :tangle \"foo.txt\" +:END: +** Two +#+begin_src conf :tangle \"file with spaces.txt\" +#+end_src" + (test-ob/get-src-block-property :tangle))))) + (ert-deftest test-ob/inline-src-blocks () (should (= 1 [-- Attachment #3: Type: text/plain, Size: 224 bytes --] -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply related [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-07-22 8:02 ` Ihor Radchenko @ 2023-07-25 11:19 ` Mehmet Tekman 2023-07-25 16:19 ` Ihor Radchenko 0 siblings, 1 reply; 77+ messages in thread From: Mehmet Tekman @ 2023-07-25 11:19 UTC (permalink / raw) To: Ihor Radchenko; +Cc: emacs-orgmode Ihor Radchenko <yantar92@posteo.net> writes: > Another meetup is on July 26. https://emacs-berlin.org/ > Or we can arrange a jitsi call. Sure, I can be there tomorrow! > To name them, you can use separate ert-deftest statements. Oh sure, but I don't want to clutter the library. > Also, see the attached modified version of your diff - I made it so > that `should' failures immediately provide some clue about expected > result value, that can help to identify which of the should clauses > fails. Ah nice, much appreciated thank you. I was hesitant to split out the main testing function (previously called by `funcall') into its own function out of the same fears as above, but I see now it's not the only one. > The tests are indeed not passing on vanilla Org. I assume that you > tested them with your other patch applied. I tested them on upstream/main... or so I thought, though I now realise that I needed to delete the =~/.emacs.d/straight/build/org= directory to trigger a rebuild of org-mode for the current branch I was on. (Live and learn...) Problem: My testing patch is indeed not working on vanilla Org, but it contains tests that will (presumably) work with the new merge-params patch. Question: What is the order of submission of patches? Do I submit a testing patch first which works strictly with vanilla Org first, and then a new merge-params function, and then yet another testing patch that is compliant with the new merge-params function? Or do I submit the new merge-params function, and then my testing patch on top? Best, Mehmet ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-07-25 11:19 ` Mehmet Tekman @ 2023-07-25 16:19 ` Ihor Radchenko 2023-07-31 13:41 ` Mehmet Tekman 0 siblings, 1 reply; 77+ messages in thread From: Ihor Radchenko @ 2023-07-25 16:19 UTC (permalink / raw) To: Mehmet Tekman; +Cc: emacs-orgmode Mehmet Tekman <mtekman89@gmail.com> writes: >> Another meetup is on July 26. https://emacs-berlin.org/ >> Or we can arrange a jitsi call. > > Sure, I can be there tomorrow! Ok. There are several talks planned at the beginning, and we may discuss your patch after those. >> The tests are indeed not passing on vanilla Org. I assume that you >> tested them with your other patch applied. > > I tested them on upstream/main... or so I thought, though I now > realise that I needed to delete the =~/.emacs.d/straight/build/org= > directory to trigger a rebuild of org-mode for the current branch I > was on. (Live and learn...) That should not normally be necessary. Not sure what happened in your setup. > Problem: My testing patch is indeed not working on vanilla Org, but it > contains tests that will (presumably) work with the new > merge-params patch. > > Question: What is the order of submission of patches? > > Do I submit a testing patch first which works strictly with > vanilla Org first, and then a new merge-params function, and > then yet another testing patch that is compliant with the > new merge-params function? > > Or do I submit the new merge-params function, and then my > testing patch on top? Either way is fine. The latter should be easier for you though. -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-07-25 16:19 ` Ihor Radchenko @ 2023-07-31 13:41 ` Mehmet Tekman 2023-07-31 16:38 ` Ihor Radchenko 0 siblings, 1 reply; 77+ messages in thread From: Mehmet Tekman @ 2023-07-31 13:41 UTC (permalink / raw) To: Ihor Radchenko; +Cc: emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 1226 bytes --] Hello Ihor Radchenko <yantar92@posteo.net> writes: > > > Mehmet Tekman <mtekman89@gmail.com> writes: > > Question: What is the order of submission of patches? > > > > Do I submit a testing patch first which works strictly with > > vanilla Org first, and then a new merge-params function, and > > then yet another testing patch that is compliant with the > > new merge-params function? > > > > Or do I submit the new merge-params function, and then my > > testing patch on top? > > Either way is fine. The latter should be easier for you though. I went with the former way. I just want to get a set of standardized tests into org-mode that I can base a future branch on to really see what is working and what is not. > Also, see the attached modified version of your diff - I made it so > that `should' failures immediately provide some clue about expected > result value, that can help to identify which of the should clauses > fails. I've adapted this to write a git patch which currently passes with upstream/main, and contains a few `should-not` statements which are planned to be `should` statements after the merge function has been patched for the `:tangle' multi-parameter property. Best, Mehmet [-- Attachment #2: 0001-testing-lisp-test-ob.el-test-ob-merge-params-new-test-v4-mtekman.patch --] [-- Type: text/x-patch, Size: 6260 bytes --] From 76e8b80e46db863ded7f207c82dcbcfb5451ddd3 Mon Sep 17 00:00:00 2001 From: Mehmet Tekman <mtekman89@gmail.com> Date: Mon, 31 Jul 2023 15:19:48 +0200 Subject: [PATCH] * testing/lisp/test-ob.el: New tests for merge-params (test-ob/get-src-block-property): (test-ob/merge-params): add new tests for the merge-params source block header handling function. --- testing/lisp/test-ob.el | 143 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 143 insertions(+) diff --git a/testing/lisp/test-ob.el b/testing/lisp/test-ob.el index c8dbd44f4..c9d13f92e 100644 --- a/testing/lisp/test-ob.el +++ b/testing/lisp/test-ob.el @@ -314,6 +314,149 @@ this is simple" (org-babel-next-src-block) (should (= 14 (org-babel-execute-src-block))))) +(defun test-ob/get-src-block-property (properties) + "Get plist of PROPERTIES and values for the first src block in buffer. +PROPERTIES is a list of property keywords or a single keyword." + (org-with-wide-buffer + (goto-char (point-min)) + (org-babel-next-src-block) + (org-set-regexps-and-options) + (let ((all-props (nth 2 (org-babel-get-src-block-info)))) + (if (listp properties) + (apply #'nconc (mapcar (lambda (p) (list p (cdr (assoc p all-props)))) properties)) + (list properties (cdr (assoc properties all-props))))))) + +(ert-deftest test-ob/merge-params () + "Test the output of merging multiple header parameters. The +expected output is given in the contents of the source code block +in each test. The desired test header parameters are given +either as a symbol or a list in the `idtest-alist' variable. +Multiple header parameters must be separated by a newline and +exactly two spaces in the block contents." + (should ;; 1. inherit-document-header-args + (equal '(:tangle "/tmp/default_tangle.txt") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* One +#+begin_src conf +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should-not ;; 2. inherit-document-header-with-local-sync-action + ;; This should pass with newer merge function with multiple tangle parameters + (equal '(:tangle "/tmp/default_tangle.txt skip") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Two +#+begin_src conf :tangle skip +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 3. override-document-header-with-local-tfile + (equal '(:tangle "randomfile sync") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Three +#+begin_src conf :tangle randomfile sync +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 4. override-document-and-parent-header-with-local-tfile-and-action + (equal '(:tangle "randomfile sync") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Four +:PROPERTIES: +:header-args: :tangle \"newfile.txt\" import +:END: +** A +#+begin_src conf :tangle randomfile sync +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 5. test-tangle-and-default-results-param-together + (equal '(:tangle "randomfile" :results "replace") + (org-test-with-temp-text + "\ +* Five +#+begin_src conf :tangle randomfile +#+end_src" + (test-ob/get-src-block-property '(:tangle :results))))) + (should-not ;; 6. inherit-document-tfile-take-only-last-local-sync-action + ;; This should pass with newer merge function with multiple tangle parameters + (equal '(:tangle "/tmp/default_tangle.txt export") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Six +#+begin_src conf :tangle import export +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should-not ;; 7. ignore-document-header-take-last-tfile-and-sync-action + ;; This should pass with newer merge function with multiple tangle parameters + (equal '(:tangle "fname2 export") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Seven +#+begin_src conf :tangle fname1 fname2 sync export +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 8. test-results-and-exports + (equal '(:results "wrap file replace" :exports "code") + (org-test-with-temp-text + "\ +* Eight +#+begin_src sh :results file wrap +#+end_src" + (test-ob/get-src-block-property '(:results :exports))))) + (should ;; 9. do-not-tangle-this-block -- + (equal '(:tangle "no") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Nine +#+begin_src conf :tangle no +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 10. test-tangle-exports-and-comments + (equal '(:tangle "foo.txt" :exports "verbatim code" :comments "link") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Ten +:PROPERTIES: +:header-args: :tangle no :exports verbatim +:END: +#+begin_src conf :tangle \"foo.txt\" :comments link +#+end_src" + (test-ob/get-src-block-property '(:tangle :exports :comments))))) + (should-not ;; 11. override-document-and-heading-tfile-with-yes + ;; This should pass with newer merge function with multiple tangle parameters + (equal '(:tangle "foo.txt") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Eleven +:PROPERTIES: +:header-args: :tangle \"foo.txt\" +:END: +#+begin_src conf :tangle yes +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 12. tangle-file-with-spaces + (equal '(:tangle "file with spaces.txt") + (org-test-with-temp-text + "\ +* Twelve +:PROPERTIES: +:header-args: :tangle \"foo.txt\" +:END: +** A +#+begin_src conf :tangle \"file with spaces.txt\" +#+end_src" + (test-ob/get-src-block-property :tangle))))) + (ert-deftest test-ob/inline-src-blocks () (should (= 1 -- 2.41.0 ^ permalink raw reply related [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-07-31 13:41 ` Mehmet Tekman @ 2023-07-31 16:38 ` Ihor Radchenko 2023-07-31 20:11 ` Mehmet Tekman 0 siblings, 1 reply; 77+ messages in thread From: Ihor Radchenko @ 2023-07-31 16:38 UTC (permalink / raw) To: Mehmet Tekman; +Cc: emacs-orgmode Mehmet Tekman <mtekman89@gmail.com> writes: > I've adapted this to write a git patch which currently passes with > upstream/main, and contains a few `should-not` statements which are > planned to be `should` statements after the merge function has been > patched for the `:tangle' multi-parameter property. I do not like the idea of putting unrelated tests to upsteam. They may confuse people reading the sources before your follow-up patch is merged. Is there any specific reason why you want to merge these tests now and not later, when you submit the full patch? > +(ert-deftest test-ob/merge-params () > + "Test the output of merging multiple header parameters. The > +expected output is given in the contents of the source code block > +in each test. The desired test header parameters are given > +either as a symbol or a list in the `idtest-alist' variable. > +Multiple header parameters must be separated by a newline and > +exactly two spaces in the block contents." Note that you did not update the docstring after changes in the tests structure. -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-07-31 16:38 ` Ihor Radchenko @ 2023-07-31 20:11 ` Mehmet Tekman 2023-08-01 7:54 ` Ihor Radchenko 2023-08-02 14:46 ` Mehmet Tekman 0 siblings, 2 replies; 77+ messages in thread From: Mehmet Tekman @ 2023-07-31 20:11 UTC (permalink / raw) To: Ihor Radchenko; +Cc: emacs-orgmode Ihor Radchenko <yantar92@posteo.net> writes: > Is there any specific reason why you want to merge these tests now and > not later, when you submit the full patch? Convenience on my part, and closure of the many many patch branches that I have open. But I think you're right, and tomorrow evening I will just bite the bullet and submit a merged-params patch. That is: * An updated merged params function * New testing suite for the merge params Till then ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-07-31 20:11 ` Mehmet Tekman @ 2023-08-01 7:54 ` Ihor Radchenko 2023-08-01 8:49 ` Mehmet Tekman 2023-08-02 14:46 ` Mehmet Tekman 1 sibling, 1 reply; 77+ messages in thread From: Ihor Radchenko @ 2023-08-01 7:54 UTC (permalink / raw) To: Mehmet Tekman; +Cc: emacs-orgmode Mehmet Tekman <mtekman89@gmail.com> writes: > But I think you're right, and tomorrow evening I will just bite the bullet and > submit a merged-params patch. That is: > * An updated merged params function > * New testing suite for the merge params Thanks! May I know if you have FSF copyright assignment? (see https://orgmode.org/worg/org-contribute.html#copyright) -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-08-01 7:54 ` Ihor Radchenko @ 2023-08-01 8:49 ` Mehmet Tekman 2023-08-01 9:30 ` Ihor Radchenko 0 siblings, 1 reply; 77+ messages in thread From: Mehmet Tekman @ 2023-08-01 8:49 UTC (permalink / raw) To: Ihor Radchenko; +Cc: emacs-orgmode Yep, signed and received May 10 2023, before I did my initial patch submission RT: 1938590 Best ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-08-01 8:49 ` Mehmet Tekman @ 2023-08-01 9:30 ` Ihor Radchenko 2023-08-01 18:19 ` Bastien Guerry 0 siblings, 1 reply; 77+ messages in thread From: Ihor Radchenko @ 2023-08-01 9:30 UTC (permalink / raw) To: Mehmet Tekman, Bastien; +Cc: emacs-orgmode Mehmet Tekman <mtekman89@gmail.com> writes: > Yep, signed and received May 10 2023, before I did my initial > patch submission > > RT: 1938590 Bastien, may you please confirm? -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-08-01 9:30 ` Ihor Radchenko @ 2023-08-01 18:19 ` Bastien Guerry 2023-08-02 7:29 ` Ihor Radchenko 0 siblings, 1 reply; 77+ messages in thread From: Bastien Guerry @ 2023-08-01 18:19 UTC (permalink / raw) To: Ihor Radchenko; +Cc: Mehmet Tekman, emacs-orgmode Ihor Radchenko <yantar92@posteo.net> writes: > Mehmet Tekman <mtekman89@gmail.com> writes: > >> Yep, signed and received May 10 2023, before I did my initial >> patch submission >> >> RT: 1938590 > > Bastien, may you please confirm? Yes, I do confirm. Thanks Mehmet for contributing! -- Bastien Guerry ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-08-01 18:19 ` Bastien Guerry @ 2023-08-02 7:29 ` Ihor Radchenko 0 siblings, 0 replies; 77+ messages in thread From: Ihor Radchenko @ 2023-08-02 7:29 UTC (permalink / raw) To: Bastien Guerry; +Cc: Mehmet Tekman, emacs-orgmode Bastien Guerry <bzg@gnu.org> writes: >> Bastien, may you please confirm? > > Yes, I do confirm. Thanks Mehmet for contributing! Thanks! Updated our records. https://git.sr.ht/~bzg/worg/commit/70026249 -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-07-31 20:11 ` Mehmet Tekman 2023-08-01 7:54 ` Ihor Radchenko @ 2023-08-02 14:46 ` Mehmet Tekman 2023-08-03 6:32 ` Mehmet Tekman 2023-08-03 7:35 ` Ihor Radchenko 1 sibling, 2 replies; 77+ messages in thread From: Mehmet Tekman @ 2023-08-02 14:46 UTC (permalink / raw) To: Ihor Radchenko; +Cc: emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 3218 bytes --] Hello again, I've attached two patches: 1. Tests for the existing merge parameters function, based on your last edits to my previous patch. 2. A rewrite of the entire merge parameters function, with the new tangle sync actions. It's a big patch mostly, because there were no intermediate commits in which the org framework wouldn't be broken. Hope that's okay! ** Brief Explanation of (2) This attempts to address the problem of having an :any keyword in a mutually exclusive group such as: #+begin_src elisp '((tangle yes no :any) (import export skip sync)) #+end_src Since every parameter (e.g. "export") is tested against both of these groups, and since a parameter that matches `:any' could be anything, then a parameter like "export" will match in both groups. To get around this, a merge strategy (see: `org-babel--merge-headers') builds an alist of exclusion groups that a parameter could belong to. e.g for a header with `:tangle myfile.txt skip' would give an alist of #+begin_src elisp (setq params-alist '(("skip" "tangle" "import") ("myfile.txt" "tangle"))) #+end_src Note that the exclusion groups are referenced by the first element in the group, acting as an alist key of sorts. This assumes that each exclusion group has a unique car. Parameters with two or more cdr elements are stripped of the exclusion group that have the `:any' parameter (i.e. "tangle"), resulting in: #+begin_src elisp (setq params-alist '(("skip" "import") ("myfile.txt" "tangle"))) #+end_src This alist is then inverted so that the group exclusion car becomes the key, and rearranged so that it follows the order of exclusion group definition (in this case, "tangle" group first then then "import" group). #+begin_src elisp (setq group-alist '(("tangle" "myfile.txt") ("import" "skip"))) #+end_src As the merge parameters function is called repeatedly for the same header, this builds to hold the whole related hierarchy of the org: #+begin_src org ,#+PROPERTIES: header-args :tangle topfile.txt import ,* One :PROPERTIES: :header-args: :tangle skip :END: ,#+begin_src bash :tangle myfile.txt ,#+end_src #+end_src which would yield: #+begin_src elisp (setq group-alist '(("tangle" "myfile.txt" "no" "topfile.txt") ("import" "skip" "import"))) #+end_src Assuming the hierarchy is given in the reverse order of this alist, the correct action is then taken as the car of these groups: `:tangle myfile.txt skip' ** Problems It seems to work well for most tests, except for the "file with spaces.txt" which I'm not sure how to proceed with. I feel like patching at the level `org-babel--merge-params' is the wrong way to go, and that I should patch it further upstream. That upstream leads to `org-babel-read' which currently chomps the quotes off anything it gets: #+begin_src elisp (org-babel-read " \"my file with quotes\" ") #+end_src I don't know if this is the expected behaviour for quoted strings with spaces, so before I proceed trying to patch this I thought I would check in with you and ask whether I should start patching here or not. Best, Mehmet [-- Attachment #2: 0001-testing-lisp-test-ob.el-New-tests-for-merge-params.patch --] [-- Type: text/x-patch, Size: 5951 bytes --] From dc2bb34b62b3725015ee9457c74397a9b0f11109 Mon Sep 17 00:00:00 2001 From: MT <mtekman89@gmail.com> Date: Tue, 1 Aug 2023 05:14:46 +0200 Subject: [PATCH 1/2] * testing/lisp/test-ob.el: New tests for merge-params (test-ob/get-src-block-property): (test-ob/merge-params): add new tests for the merge-params source block header handling function. --- testing/lisp/test-ob.el | 138 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 138 insertions(+) diff --git a/testing/lisp/test-ob.el b/testing/lisp/test-ob.el index c8dbd44f4..f12533258 100644 --- a/testing/lisp/test-ob.el +++ b/testing/lisp/test-ob.el @@ -314,6 +314,144 @@ this is simple" (org-babel-next-src-block) (should (= 14 (org-babel-execute-src-block))))) +(defun test-ob/get-src-block-property (properties) + "Get plist of PROPERTIES and values for the first src block in buffer. +PROPERTIES is a list of property keywords or a single keyword." + (org-with-wide-buffer + (goto-char (point-min)) + (org-babel-next-src-block) + (org-set-regexps-and-options) + (let ((all-props (nth 2 (org-babel-get-src-block-info)))) + (if (listp properties) + (apply #'nconc (mapcar (lambda (p) (list p (cdr (assoc p all-props)))) properties)) + (list properties (cdr (assoc properties all-props))))))) + +(ert-deftest test-ob/merge-params () + "Test the output of merging multiple header parameters." + (should ;; 1. inherit-document-header-args + (equal '(:tangle "/tmp/default_tangle.txt") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* One +#+begin_src conf +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should-not ;; 2. inherit-document-header-with-local-sync-action + ;; This should pass with newer merge function with multiple tangle parameters + (equal '(:tangle "/tmp/default_tangle.txt skip") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Two +#+begin_src conf :tangle skip +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 3. override-document-header-with-local-tfile + (equal '(:tangle "randomfile sync") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Three +#+begin_src conf :tangle randomfile sync +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 4. override-document-and-parent-header-with-local-tfile-and-action + (equal '(:tangle "randomfile sync") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Four +:PROPERTIES: +:header-args: :tangle \"newfile.txt\" import +:END: +** A +#+begin_src conf :tangle randomfile sync +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 5. test-tangle-and-default-results-param-together + (equal '(:tangle "randomfile" :results "replace") + (org-test-with-temp-text + "\ +* Five +#+begin_src conf :tangle randomfile +#+end_src" + (test-ob/get-src-block-property '(:tangle :results))))) + (should-not ;; 6. inherit-document-tfile-take-only-last-local-sync-action + ;; This should pass with newer merge function with multiple tangle parameters + (equal '(:tangle "/tmp/default_tangle.txt export") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Six +#+begin_src conf :tangle import export +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should-not ;; 7. ignore-document-header-take-last-tfile-and-sync-action + ;; This should pass with newer merge function with multiple tangle parameters + (equal '(:tangle "fname2 export") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Seven +#+begin_src conf :tangle fname1 fname2 sync export +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 8. test-results-and-exports + (equal '(:results "wrap file replace" :exports "code") + (org-test-with-temp-text + "\ +* Eight +#+begin_src sh :results file wrap +#+end_src" + (test-ob/get-src-block-property '(:results :exports))))) + (should ;; 9. do-not-tangle-this-block -- + (equal '(:tangle "no") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Nine +#+begin_src conf :tangle no +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 10. test-tangle-exports-and-comments + (equal '(:tangle "foo.txt" :exports "verbatim code" :comments "link") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Ten +:PROPERTIES: +:header-args: :tangle no :exports verbatim +:END: +#+begin_src conf :tangle \"foo.txt\" :comments link +#+end_src" + (test-ob/get-src-block-property '(:tangle :exports :comments))))) + (should-not ;; 11. override-document-and-heading-tfile-with-yes + ;; This should pass with newer merge function with multiple tangle parameters + (equal '(:tangle "foo.txt") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Eleven +:PROPERTIES: +:header-args: :tangle \"foo.txt\" +:END: +#+begin_src conf :tangle yes +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 12. tangle-file-with-spaces + (equal '(:tangle "file with spaces.txt") + (org-test-with-temp-text + "\ +* Twelve +:PROPERTIES: +:header-args: :tangle \"foo.txt\" +:END: +** A +#+begin_src conf :tangle \"file with spaces.txt\" +#+end_src" + (test-ob/get-src-block-property :tangle))))) + (ert-deftest test-ob/inline-src-blocks () (should (= 1 -- 2.41.0 [-- Attachment #3: 0002-lisp-ob-core.el-Rewrite-of-merge-babel-headers.patch --] [-- Type: text/x-patch, Size: 11910 bytes --] From 203153bfb3e49c82995f721518596a2b00127467 Mon Sep 17 00:00:00 2001 From: MT <mtekman89@gmail.com> Date: Wed, 10 May 2023 17:38:22 +0200 Subject: [PATCH 2/2] * lisp/ob-core.el: Rewrite of merge babel headers (org-babel-merge-params): merge headers strategy split out into new dedicated function `--merge-headers' (org-babel--merge-headers): new function that resolves headers based on their mutually exclusive groups, with better support for groups with `:any' keywords. (org-babel-common-header-args-w-values): Added mutually exclusive tangle groups relating to desired tangle sync actions * testing/lisp/test-ob.el: update tests according to new merge strategy, with emphasis on `:tangle' headers for syncing actions. --- lisp/ob-core.el | 142 +++++++++++++++++++++++++++++++--------- testing/lisp/test-ob.el | 11 ++-- 2 files changed, 116 insertions(+), 37 deletions(-) diff --git a/lisp/ob-core.el b/lisp/ob-core.el index 606c3efec..48337406a 100644 --- a/lisp/ob-core.el +++ b/lisp/ob-core.el @@ -437,7 +437,8 @@ then run `org-babel-switch-to-session'." (sep . :any) (session . :any) (shebang . :any) - (tangle . ((tangle yes no :any))) + (tangle . ((tangle yes no :any) + (import export skip sync))) (tangle-mode . ((#o755 #o555 #o444 :any))) (var . :any) (wrap . :any)) @@ -2836,6 +2837,89 @@ specified as an an \"attachment:\" style link." (goto-char body-start) (insert body)))) +(defun org-babel--merge-headers (exclusive-groups &rest result-params) + "Maintain exclusivity of mutually exclusive parameters, as defined +in EXCLUSIVE-GROUPS while merging lists in RESULT-PARAMS." + (cl-flet ((alist-append (alist key val) + (let ((existing (cdr (assoc key alist)))) + (if existing + (setcdr (assoc key alist) (append existing (list val))) + (setq alist (cons (cons key (list val)) alist))) + alist))) + ;; + ;; Problem: Having an :any keyword in an exclusion group means + ;; that a parameter of "yes" could match to an exclusion + ;; group that contains both "yes" AND ":any". + ;; + ;; Solution: For each parameter, build a list of exclusion groups + ;; it could belong to. If a parameter belongs to two + ;; groups, eliminate it from the group that contains the + ;; ":any" keyword. + ;; + ;; Details: Exclusion groups (e.g.'("tangle" "yes" "no" ":any") ) + ;; are referenced to by their car ("tangle"), acting as + ;; a key for the entire group. + ;; + ;; Assumption: The order of RESULT-PARAMS dictate the hierarchy of + ;; the cascading headers. + ;; + (let ((any-group-key ;; exclusion group key for group with :any keyword + (caar (cl-remove-if-not (lambda (x) (member ":any" x)) exclusive-groups))) + params-alist ;; param -> list( exclusion group keys ) + unexplained-params) ;; any params that were not caught + + ;; Iterate parameters, across each exclusion group. + ;; - Populate params-alist + (dolist (new-params result-params) + (dolist (new-param new-params) + (dolist (exclusive-group exclusive-groups) + (if (or (member new-param exclusive-group) + (and (string= (car exclusive-group) any-group-key) + ;; param *doesn't* match a keyword in this + ;; :any group? Could be :any. + (not (member new-param exclusive-group)))) + (let ((group-key (car exclusive-group))) + (setq params-alist (alist-append params-alist new-param group-key))) + ;; Some parameters fit into no groups, store them and process later. + (push new-param unexplained-params))))) + + (delete-dups unexplained-params) + + ;; Find parameters listed in 2 or more exclusive groups, and kick + ;; them out of any non-":any" group. + ;; - Update params-alist + ;; - Remove uniquely known params from unexplained-params + (dolist (parm-vals params-alist) + (let ((parm (car parm-vals)) + (group-keys (cdr parm-vals))) + (if (member parm unexplained-params) + (setq unexplained-params (delete parm unexplained-params))) + (if (> (length group-keys) 1) + (dolist (gkey group-keys) + (if (string= gkey any-group-key) + (setcdr (assoc parm params-alist) (delete gkey group-keys))))))) + + ;; Collapse parameters into exclusion groups + ;; - convert params → list(exclusion group keys) to exclusion-group-key → list(params) + ;; - e.g.'(("sync" "import")("/tmp/sublow" "tangle")("/tmp/low" "tangle")("no" "tangle")) + (let (group-alist) + (mapcar (lambda (x) (setq group-alist (alist-append group-alist (cadr x) (car x)))) params-alist) + ;; - e.g. '(("tangle" "/tmp/sublow" "/tmp/low" "no") ("import" "sync"))) + ;;;;(setq group-alist (mapcar #'cadr group-alist)) + + ;; Set values in the same order that the exclusion groups are defined + (let ((group-key-order (mapcar #'car exclusive-groups))) + (setq group-alist (cl-remove-if-not #'identity + (mapcar (lambda (x) (car (alist-get x group-alist))) + group-key-order)))) + + ;; Final Sanity Check: were all parameters explained? + ;; - if not, append to result + (if unexplained-params + (setq group-alist (append unexplained-params group-alist))) + group-alist)))) + + (defun org-babel-merge-params (&rest plists) "Combine all parameter association lists in PLISTS. Later elements of PLISTS override the values of previous elements. @@ -2847,26 +2931,15 @@ parameters when merging lists." (exports-exclusive-groups (mapcar (lambda (group) (mapcar #'symbol-name group)) (cdr (assq 'exports org-babel-common-header-args-w-values)))) - (merge - (lambda (exclusive-groups &rest result-params) - ;; Maintain exclusivity of mutually exclusive parameters, - ;; as defined in EXCLUSIVE-GROUPS while merging lists in - ;; RESULT-PARAMS. - (let (output) - (dolist (new-params result-params (delete-dups output)) - (dolist (new-param new-params) - (dolist (exclusive-group exclusive-groups) - (when (member new-param exclusive-group) - (setq output (cl-remove-if - (lambda (o) (member o exclusive-group)) - output)))) - (push new-param output)))))) + (tangle-exclusive-groups + (mapcar (lambda (group) (mapcar #'symbol-name group)) + (cdr (assq 'tangle org-babel-common-header-args-w-values)))) (variable-index 0) ;Handle positional arguments. clearnames params ;Final parameters list. ;; Some keywords accept multiple values. We need to treat ;; them specially. - vars results exports) + vars results exports tangle) (dolist (plist plists) (dolist (pair plist) (pcase pair @@ -2901,22 +2974,28 @@ parameters when merging lists." (t (error "Variable \"%s\" must be assigned a default value" (cdr pair)))))) (`(:results . ,value) - (setq results (funcall merge - results-exclusive-groups - results - (split-string - (cond ((stringp value) value) - ((functionp value) (funcall value)) + (setq results (org-babel--merge-headers + results-exclusive-groups + results + (split-string + (cond ((stringp value) value) + ((functionp value) (funcall value)) ;; FIXME: Arbitrary code evaluation. - (t (eval value t))))))) + (t (eval value t))))))) (`(:exports . ,value) - (setq exports (funcall merge - exports-exclusive-groups - exports - (split-string - (cond ((and value (functionp value)) (funcall value)) - (value value) - (t "")))))) + (setq exports (org-babel--merge-headers + exports-exclusive-groups + exports + (split-string + (cond ((and value (functionp value)) (funcall value)) + (value value) + (t "")))))) + (`(:tangle . ,value) + (setq tangle (org-babel--merge-headers + tangle-exclusive-groups + tangle + (split-string + (or value ""))))) ((or '(:dir . attach) '(:dir . "'attach")) (unless (org-attach-dir nil t) (error "No attachment directory for element (add :ID: or :DIR: property)")) @@ -2942,7 +3021,8 @@ parameters when merging lists." params))))) ;; Handle other special keywords, which accept multiple values. (setq params (nconc (list (cons :results (mapconcat #'identity results " ")) - (cons :exports (mapconcat #'identity exports " "))) + (cons :exports (mapconcat #'identity exports " ")) + (cons :tangle (mapconcat #'identity tangle " "))) params)) ;; Return merged params. (org-babel-eval-headers params))) diff --git a/testing/lisp/test-ob.el b/testing/lisp/test-ob.el index f12533258..a58a47052 100644 --- a/testing/lisp/test-ob.el +++ b/testing/lisp/test-ob.el @@ -337,7 +337,7 @@ PROPERTIES is a list of property keywords or a single keyword." #+begin_src conf #+end_src" (test-ob/get-src-block-property :tangle)))) - (should-not ;; 2. inherit-document-header-with-local-sync-action + (should ;; 2. inherit-document-header-with-local-sync-action ;; This should pass with newer merge function with multiple tangle parameters (equal '(:tangle "/tmp/default_tangle.txt skip") (org-test-with-temp-text @@ -377,7 +377,7 @@ PROPERTIES is a list of property keywords or a single keyword." #+begin_src conf :tangle randomfile #+end_src" (test-ob/get-src-block-property '(:tangle :results))))) - (should-not ;; 6. inherit-document-tfile-take-only-last-local-sync-action + (should ;; 6. inherit-document-tfile-take-only-last-local-sync-action ;; This should pass with newer merge function with multiple tangle parameters (equal '(:tangle "/tmp/default_tangle.txt export") (org-test-with-temp-text @@ -387,7 +387,7 @@ PROPERTIES is a list of property keywords or a single keyword." #+begin_src conf :tangle import export #+end_src" (test-ob/get-src-block-property :tangle)))) - (should-not ;; 7. ignore-document-header-take-last-tfile-and-sync-action + (should ;; 7. ignore-document-header-take-last-tfile-and-sync-action ;; This should pass with newer merge function with multiple tangle parameters (equal '(:tangle "fname2 export") (org-test-with-temp-text @@ -426,9 +426,8 @@ PROPERTIES is a list of property keywords or a single keyword." #+begin_src conf :tangle \"foo.txt\" :comments link #+end_src" (test-ob/get-src-block-property '(:tangle :exports :comments))))) - (should-not ;; 11. override-document-and-heading-tfile-with-yes - ;; This should pass with newer merge function with multiple tangle parameters - (equal '(:tangle "foo.txt") + (should ;; 11. override-document-and-heading-tfile-with-yes + (equal '(:tangle "yes") (org-test-with-temp-text "\ #+PROPERTY: header-args :tangle /tmp/default_tangle.txt -- 2.41.0 ^ permalink raw reply related [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-08-02 14:46 ` Mehmet Tekman @ 2023-08-03 6:32 ` Mehmet Tekman 2023-08-03 7:35 ` Ihor Radchenko 1 sibling, 0 replies; 77+ messages in thread From: Mehmet Tekman @ 2023-08-03 6:32 UTC (permalink / raw) To: Ihor Radchenko; +Cc: emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 332 bytes --] Okay, and now I've patched the related string splitting functions so that ("let's get \"ready to rumble\" now") is split into: ("let's" "get" "ready to rumble" "now") All my tests are passing but a few unrelated ones are failing so I'll need to dig more into what's going on there. Attached is the 3rd patch Best [-- Attachment #2: 0003-lisp-ob-core.el.patch --] [-- Type: text/x-patch, Size: 2760 bytes --] From 3f39671f3ec55e6db076f2b1b5ccad12380eb32a Mon Sep 17 00:00:00 2001 From: Mehmet Tekman <mtekman89@gmail.com> Date: Thu, 3 Aug 2023 08:22:03 +0200 Subject: [PATCH 3/3] * lisp/ob-core.el (org-babel-merge-params): (org-babel--split-str-quoted): Added quotes-aware string split function (org-babel-read): patched to add quotes to words with spaces --- lisp/ob-core.el | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/lisp/ob-core.el b/lisp/ob-core.el index 48337406a..0c36bcf00 100644 --- a/lisp/ob-core.el +++ b/lisp/ob-core.el @@ -2919,6 +2919,21 @@ in EXCLUSIVE-GROUPS while merging lists in RESULT-PARAMS." (setq group-alist (append unexplained-params group-alist))) group-alist)))) +(defun org-babel--split-str-quoted (str) + "Splits a string that may or may not contain quotes" + (let (result pos) + (while (string-match (rx (or (seq "\"" (group (* (not "\""))) "\"") + (group (+ (not space))))) + str pos) + (let ((match-str1 (match-string 1 str)) + (match-str2 (match-string 2 str))) + (if match-str1 + (progn + (push match-str1 result) + (setq pos (1+ (match-end 1)))) + (push match-str2 result) + (setq pos (match-end 2))))) + (nreverse result))) (defun org-babel-merge-params (&rest plists) "Combine all parameter association lists in PLISTS. @@ -2994,7 +3009,7 @@ parameters when merging lists." (setq tangle (org-babel--merge-headers tangle-exclusive-groups tangle - (split-string + (org-babel--split-str-quoted (or value ""))))) ((or '(:dir . attach) '(:dir . "'attach")) (unless (org-attach-dir nil t) @@ -3319,9 +3334,14 @@ situations in which is it not appropriate." ;; FIXME: Arbitrary code evaluation. (eval (read cell) t)) ((save-match-data - (and (string-match "^[[:space:]]*\"\\(.*\\)\"[[:space:]]*$" cell) - (not (string-match "[^\\]\"" (match-string 1 cell))))) - (read cell)) + (and (string-match (rx bol (* space) "\"" (group (* any)) "\"" (* space) eol) + cell) + (not (string-match (rx (not "\\") "\"") (match-string 1 cell))))) + (let ((res (read cell))) + ;; If the matched string contains spaces, surround it in quotes. + (if (string-match (rx (+ space)) res) + (concat "\"" res "\"") + res))) (t (org-no-properties cell)))) (defun org-babel--string-to-number (string) -- 2.41.0 ^ permalink raw reply related [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-08-02 14:46 ` Mehmet Tekman 2023-08-03 6:32 ` Mehmet Tekman @ 2023-08-03 7:35 ` Ihor Radchenko 2023-08-03 8:08 ` Mehmet Tekman 1 sibling, 1 reply; 77+ messages in thread From: Ihor Radchenko @ 2023-08-03 7:35 UTC (permalink / raw) To: Mehmet Tekman; +Cc: emacs-orgmode Mehmet Tekman <mtekman89@gmail.com> writes: > It's a big patch mostly, because there were no intermediate commits > in which the org framework wouldn't be broken. Hope that's okay! Sure it is. > ** Problems > > It seems to work well for most tests, except for the "file with > spaces.txt" which I'm not sure how to proceed with. I feel like > patching at the level `org-babel--merge-params' is the wrong way to > go, and that I should patch it further upstream. > > That upstream leads to `org-babel-read' which currently chomps the > quotes off anything it gets: > > #+begin_src elisp > (org-babel-read " \"my file with quotes\" ") > #+end_src > I don't know if this is the expected behaviour for quoted strings with > spaces, so before I proceed trying to patch this I thought I would > check in with you and ask whether I should start patching here or not. This is indeed a problem. Org cannot distinguish between #+begin_src lang :foo value with spaces and #+begin_src lang :foo "value with spaces" What we can do it make `org-babel-read' assign a text property to the resulting string when it is a string with quotes: (org-babel-read "my file with quotes") ; => "my file with quotes" (org-babel-read "\"my file with quotes\"") ; => #("my file with quotes" 0 19 (org-babel-quote t)) Then, we can later use this information in `org-babel-merge-params'. We will not call `split-string', when 'org-babel-quote text property is present. Also, `split-string' won't work when we have something like "yes \"my file with quotes\"" Instead, we should use (mapcar #'org-babel-read (org-babel-balanced-split "yes \"my file with quotes\"" ?\s)) > - (tangle . ((tangle yes no :any))) > + (tangle . ((tangle yes no :any) > + (import export skip sync))) We should not yet change this before the code using the new tangle values is in place. > +(defun org-babel--merge-headers (exclusive-groups &rest result-params) > + "Maintain exclusivity of mutually exclusive parameters, as defined > +in EXCLUSIVE-GROUPS while merging lists in RESULT-PARAMS." Please update the docstring, following conventions: 1. First line should be a short full sentence explaining what function does briefly. 2. Each argument should be explained - what it is and what is the structure. Also, RESULT-PARAMS is confusing because it is no longer just used for :results. Please rename. > + (cl-flet ((alist-append (alist key val) > + (let ((existing (cdr (assoc key alist)))) > + (if existing > + (setcdr (assoc key alist) (append existing (list val))) > + (setq alist (cons (cons key (list val)) alist))) > + alist))) Can simply (push val (alist-get key alist nil nil #'equal)) > + ;; Problem: Having an :any keyword in an exclusion group means > + ;; that a parameter of "yes" could match to an exclusion > + ;; group that contains both "yes" AND ":any". > + ;; Solution: For each parameter, build a list of exclusion groups > + ;; it could belong to. If a parameter belongs to two > + ;; groups, eliminate it from the group that contains the > + ;; ":any" keyword. > + ;; > + ;; Details: Exclusion groups (e.g.'("tangle" "yes" "no" ":any") ) > + ;; are referenced to by their car ("tangle"), acting as > + ;; a key for the entire group. > + ;; This is not a "problem" - just something to consider. You can omit "Problem:", "Solution:", and "Details:" converting into ordinary paragraphs. > + ;; Assumption: The order of RESULT-PARAMS dictate the hierarchy of > + ;; the cascading headers. The expected function parameters should be described in the docstring, not in a comment. > + (let ((any-group-key ;; exclusion group key for group with :any keyword > + (caar (cl-remove-if-not (lambda (x) (member ":any" x)) exclusive-groups))) `cl-find' will be simpler. > + (delete-dups unexplained-params) Should be (setq unexplained-params (delete-dups unexplained-params)) `delete-dups' _returns_ the modified list, which may or may not preserve the initial pointer to the first cons cell. > + ;; Set values in the same order that the exclusion groups are defined > + (let ((group-key-order (mapcar #'car exclusive-groups))) > + (setq group-alist (cl-remove-if-not #'identity Or just delq nil > + (mapcar (lambda (x) (car (alist-get x group-alist))) > + group-key-order)))) -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-08-03 7:35 ` Ihor Radchenko @ 2023-08-03 8:08 ` Mehmet Tekman 2023-08-03 8:16 ` Ihor Radchenko 0 siblings, 1 reply; 77+ messages in thread From: Mehmet Tekman @ 2023-08-03 8:08 UTC (permalink / raw) To: Ihor Radchenko; +Cc: emacs-orgmode Thanks for the review! > setq unexplained-params (delete-dups unexplained-params)) The help mentions that it stores the result in the list, and it looks like it works #+BEGIN_SRC elisp (let ((x '(1 2 1 2 3))) (delete-dups x) x) #+END_SRC > > What we can do it make `org-babel-read' assign a text property to the > resulting string when it is a string with quotes: > > (org-babel-read "my file with quotes") ; => "my file with quotes" > (org-babel-read "\"my file with quotes\"") ; => #("my file with quotes" 0 19 (org-babel-quote t)) > > Then, we can later use this information in `org-babel-merge-params'. > We will not call `split-string', when 'org-babel-quote text property is > present. > Also, `split-string' won't work when we have something like > > "yes \"my file with quotes\"" > > Instead, we should use > (mapcar #'org-babel-read (org-babel-balanced-split "yes \"my file with quotes\"" ?\s)) See my next patch, it implements a quote aware string splitting function which is used just for the tangle headers. Is my implementation there enough, without having to use text properties? > > - (tangle . ((tangle yes no :any))) > > + (tangle . ((tangle yes no :any) > > + (import export skip sync))) > > We should not yet change this before the code using the new tangle > values is in place. This is a big source of confusion for me, since I don't really know how to implement this extra merging behaviour *without* first having tangle headers implemented, since no other element in `org-babel-common-header-args-w-values' has multiple exclusion groups where at least one has `:any' ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-08-03 8:08 ` Mehmet Tekman @ 2023-08-03 8:16 ` Ihor Radchenko [not found] ` <CAHHeYzL6Z5_gGbTUrNzKDh5swgCSQiYsSj3Cs0gFy_d=eXbSBA@mail.gmail.com> 0 siblings, 1 reply; 77+ messages in thread From: Ihor Radchenko @ 2023-08-03 8:16 UTC (permalink / raw) To: Mehmet Tekman; +Cc: emacs-orgmode Mehmet Tekman <mtekman89@gmail.com> writes: >> setq unexplained-params (delete-dups unexplained-params)) > > The help mentions that it stores the result in the list, and it looks > like it works > > #+BEGIN_SRC elisp > (let ((x '(1 2 1 2 3))) (delete-dups x) x) > #+END_SRC This is just implementation detail. As a general good practice, destructive list manipulation commands should follow (setq foo (command foo)) pattern. >> Instead, we should use >> (mapcar #'org-babel-read (org-babel-balanced-split "yes \"my file with quotes\"" ?\s)) > > See my next patch, it implements a quote aware string splitting > function which is used just for the tangle headers. Is my > implementation there enough, without having to use text > properties? We already have `org-babel-balanced-split'. Your patch will be a code duplication. Also, your patch will not handle #+begin_src lang :tangle "tangle yes file" vs. #+begin_src lang :tangle tangle yes file >> > - (tangle . ((tangle yes no :any))) >> > + (tangle . ((tangle yes no :any) >> > + (import export skip sync))) >> >> We should not yet change this before the code using the new tangle >> values is in place. > > This is a big source of confusion for me, since I don't really > know how to implement this extra merging behaviour *without* > first having tangle headers implemented, since no other element > in `org-babel-common-header-args-w-values' has multiple > exclusion groups where at least one has `:any' The new merging behavior does not have be used just yet. We first implement the capability to handle exclusive groups with :any and later, in another patch, make use of the new capability. Does it make sense? -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 77+ messages in thread
[parent not found: <CAHHeYzL6Z5_gGbTUrNzKDh5swgCSQiYsSj3Cs0gFy_d=eXbSBA@mail.gmail.com>]
[parent not found: <87o7jo1q2s.fsf@localhost>]
* Re: [ANN] lisp/ob-tangle-sync.el [not found] ` <87o7jo1q2s.fsf@localhost> @ 2023-08-03 8:46 ` Mehmet Tekman 2023-08-04 7:41 ` Mehmet Tekman 0 siblings, 1 reply; 77+ messages in thread From: Mehmet Tekman @ 2023-08-03 8:46 UTC (permalink / raw) To: Ihor Radchenko; +Cc: emacs-orgmode > You dropped mailing list from the CC. Was it intentional? Whoops, no. Still figuring out an email solution in emacs. > If I understand your concern correctly, you may simply let-bind > `org-babel-common-header-args- > w-values' in the tests to make sure that > your new parameter merging code is working. Okay, I will try this ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-08-03 8:46 ` Mehmet Tekman @ 2023-08-04 7:41 ` Mehmet Tekman 2023-08-04 8:09 ` Ihor Radchenko 0 siblings, 1 reply; 77+ messages in thread From: Mehmet Tekman @ 2023-08-04 7:41 UTC (permalink / raw) To: Ihor Radchenko; +Cc: emacs-orgmode Pre-weekend update: I have a `merge-params' solution, and it's passing tests. I will need to refactor/cherry pick my branch somehow to make the patch less "tangle sync"-centric, but I'm sure I can make it work. On Thu, 3 Aug 2023 at 10:46, Mehmet Tekman <mtekman89@gmail.com> wrote: > > > You dropped mailing list from the CC. Was it intentional? > > Whoops, no. Still figuring out an email solution in emacs. > > > If I understand your concern correctly, you may simply let-bind > > `org-babel-common-header-args- > > w-values' in the tests to make sure that > > your new parameter merging code is working. > > Okay, I will try this ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-08-04 7:41 ` Mehmet Tekman @ 2023-08-04 8:09 ` Ihor Radchenko 2023-08-04 13:14 ` Mehmet Tekman 0 siblings, 1 reply; 77+ messages in thread From: Ihor Radchenko @ 2023-08-04 8:09 UTC (permalink / raw) To: Mehmet Tekman; +Cc: emacs-orgmode Mehmet Tekman <mtekman89@gmail.com> writes: > Pre-weekend update: I have a `merge-params' solution, and it's passing tests. Side note: I had one newly added test failing on the latest main. You may need to rebase your branch and check if tests are still passing. > I will need to refactor/cherry pick my branch somehow to make the patch > less "tangle sync"-centric, but I'm sure I can make it work. FYI: Interactive rebasing with magit is a lot easier. -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-08-04 8:09 ` Ihor Radchenko @ 2023-08-04 13:14 ` Mehmet Tekman 2023-08-04 16:34 ` Mehmet Tekman 2023-08-05 8:48 ` Ihor Radchenko 0 siblings, 2 replies; 77+ messages in thread From: Mehmet Tekman @ 2023-08-04 13:14 UTC (permalink / raw) To: Ihor Radchenko; +Cc: emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 2647 bytes --] Ihor Radchenko <yantar92@posteo.net> writes: > > Side note: I had one newly added test failing on the latest main. You > may need to rebase your branch and check if tests are still passing. > The rebase was fine, but I'm having a problem aligning my patch branch to work without any tangle involvement. The first hurdle is that I'm a little bit unsure about the validity of one of the tests. * Under vanilla `org-babel-merge-params', any number of :tangle header values are permitted and concatenated together. e.g 4: __________ ´ (should ;; 4. override-document-and-parent-header-with-local-tfile-and-action (equal '(:tangle "randomfile sync") (org-test-with-temp-text "\ #+PROPERTY: header-args :tangle /tmp/default_tangle.txt * Four :PROPERTIES: :header-args: :tangle \"newfile.txt\" import :END: ** A #+begin_src conf :tangle randomfile sync #+end_src" (test-ob/get-src-block-property :tangle)))) `----------- This result of "randomfile sync" for the :tangle header is seen as valid.... but it shouldn't, should it? Tangle can only take one value, so the action of `:any` should also just output one value? From the docstring text: > Symbol `:any' in the value list implies that any value is allowed. It's not clear to me if this means that `:any' or `:any :any` or `":any :any"` is permitted. In my rewrite, I very much take the `:any` or `":any :any"` single value interpretation when merging the headers together. * Sometimes the value of another header is caught in the value of another e.g 8: "wrap" is caught in the output of `:results' despite it not being a valid results keyword in `org-babel-common-header-args-w-values'. __________ ´ (should ;; 8. test-results-and-exports (equal '(:results "wrap file replace" :exports "code") (org-test-with-temp-text "\ * Eight #+begin_src sh :results file wrap #+end_src" (test-ob/get-src-block-property '(:results :exports))))) (should ;; 9. do-not-tangle-this-block -- `----------- This test results in "true" for both my rewrite, and the vanilla function, but I'm not sure how accurate or value this is. I've been reordering and splitting commits for a while now, but I think it's really not easy to seperate the new tangle-sync component from my merge-params rewrite. I've attached my patches, including fixes from the last review you gave - I hope you can make more sense of this than I can. Best, Mehmet [-- Attachment #2: 0001-testing-lisp-test-ob.el-New-tests-for-merge-params.patch --] [-- Type: text/x-patch, Size: 5962 bytes --] From bcbd184d7b7effa6cda1f6c3680a1e4a28b8be30 Mon Sep 17 00:00:00 2001 From: Mehmet Tekman <mtekman89@gmail.com> Date: Tue, 1 Aug 2023 05:14:46 +0200 Subject: [PATCH 1/4] * testing/lisp/test-ob.el: New tests for merge-params (test-ob/get-src-block-property): (test-ob/merge-params): add new tests for the merge-params source block header handling function. --- testing/lisp/test-ob.el | 138 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 138 insertions(+) diff --git a/testing/lisp/test-ob.el b/testing/lisp/test-ob.el index c8dbd44f4..f12533258 100644 --- a/testing/lisp/test-ob.el +++ b/testing/lisp/test-ob.el @@ -314,6 +314,144 @@ this is simple" (org-babel-next-src-block) (should (= 14 (org-babel-execute-src-block))))) +(defun test-ob/get-src-block-property (properties) + "Get plist of PROPERTIES and values for the first src block in buffer. +PROPERTIES is a list of property keywords or a single keyword." + (org-with-wide-buffer + (goto-char (point-min)) + (org-babel-next-src-block) + (org-set-regexps-and-options) + (let ((all-props (nth 2 (org-babel-get-src-block-info)))) + (if (listp properties) + (apply #'nconc (mapcar (lambda (p) (list p (cdr (assoc p all-props)))) properties)) + (list properties (cdr (assoc properties all-props))))))) + +(ert-deftest test-ob/merge-params () + "Test the output of merging multiple header parameters." + (should ;; 1. inherit-document-header-args + (equal '(:tangle "/tmp/default_tangle.txt") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* One +#+begin_src conf +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should-not ;; 2. inherit-document-header-with-local-sync-action + ;; This should pass with newer merge function with multiple tangle parameters + (equal '(:tangle "/tmp/default_tangle.txt skip") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Two +#+begin_src conf :tangle skip +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 3. override-document-header-with-local-tfile + (equal '(:tangle "randomfile sync") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Three +#+begin_src conf :tangle randomfile sync +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 4. override-document-and-parent-header-with-local-tfile-and-action + (equal '(:tangle "randomfile sync") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Four +:PROPERTIES: +:header-args: :tangle \"newfile.txt\" import +:END: +** A +#+begin_src conf :tangle randomfile sync +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 5. test-tangle-and-default-results-param-together + (equal '(:tangle "randomfile" :results "replace") + (org-test-with-temp-text + "\ +* Five +#+begin_src conf :tangle randomfile +#+end_src" + (test-ob/get-src-block-property '(:tangle :results))))) + (should-not ;; 6. inherit-document-tfile-take-only-last-local-sync-action + ;; This should pass with newer merge function with multiple tangle parameters + (equal '(:tangle "/tmp/default_tangle.txt export") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Six +#+begin_src conf :tangle import export +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should-not ;; 7. ignore-document-header-take-last-tfile-and-sync-action + ;; This should pass with newer merge function with multiple tangle parameters + (equal '(:tangle "fname2 export") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Seven +#+begin_src conf :tangle fname1 fname2 sync export +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 8. test-results-and-exports + (equal '(:results "wrap file replace" :exports "code") + (org-test-with-temp-text + "\ +* Eight +#+begin_src sh :results file wrap +#+end_src" + (test-ob/get-src-block-property '(:results :exports))))) + (should ;; 9. do-not-tangle-this-block -- + (equal '(:tangle "no") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Nine +#+begin_src conf :tangle no +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 10. test-tangle-exports-and-comments + (equal '(:tangle "foo.txt" :exports "verbatim code" :comments "link") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Ten +:PROPERTIES: +:header-args: :tangle no :exports verbatim +:END: +#+begin_src conf :tangle \"foo.txt\" :comments link +#+end_src" + (test-ob/get-src-block-property '(:tangle :exports :comments))))) + (should-not ;; 11. override-document-and-heading-tfile-with-yes + ;; This should pass with newer merge function with multiple tangle parameters + (equal '(:tangle "foo.txt") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Eleven +:PROPERTIES: +:header-args: :tangle \"foo.txt\" +:END: +#+begin_src conf :tangle yes +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 12. tangle-file-with-spaces + (equal '(:tangle "file with spaces.txt") + (org-test-with-temp-text + "\ +* Twelve +:PROPERTIES: +:header-args: :tangle \"foo.txt\" +:END: +** A +#+begin_src conf :tangle \"file with spaces.txt\" +#+end_src" + (test-ob/get-src-block-property :tangle))))) + (ert-deftest test-ob/inline-src-blocks () (should (= 1 -- 2.41.0 [-- Attachment #3: 0004-org-babel-common-header-args-w-values-Added-mutually.patch --] [-- Type: text/x-patch, Size: 847 bytes --] From 1a2aae5d89aeb665f5e6a4420f6f6a1a00e5056f Mon Sep 17 00:00:00 2001 From: Mehmet Tekman <mtekman89@gmail.com> Date: Fri, 4 Aug 2023 13:07:14 +0200 Subject: [PATCH 4/4] (org-babel-common-header-args-w-values): Added mutually exclusive tangle groups relating to desired tangle sync actions --- lisp/ob-core.el | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lisp/ob-core.el b/lisp/ob-core.el index cd6c6708c..f901a75ee 100644 --- a/lisp/ob-core.el +++ b/lisp/ob-core.el @@ -441,7 +441,8 @@ then run `org-babel-switch-to-session'." (sep . :any) (session . :any) (shebang . :any) - (tangle . ((tangle yes no :any))) + (tangle . ((tangle yes no :any) + (import export skip sync))) (tangle-mode . ((#o755 #o555 #o444 :any))) (var . :any) (wrap . :any)) -- 2.41.0 [-- Attachment #4: 0002-lisp-ob-core.el-Rewrite-of-merge-babel-headers.patch --] [-- Type: text/x-patch, Size: 10413 bytes --] From c8f8e1cf6dbf88162a11b520578b32b4ddd8441a Mon Sep 17 00:00:00 2001 From: Mehmet Tekman <mtekman89@gmail.com> Date: Wed, 10 May 2023 17:38:22 +0200 Subject: [PATCH 2/4] * lisp/ob-core.el: Rewrite of merge babel headers (org-babel-merge-params): merge headers strategy split out into new dedicated function `--merge-headers' (org-babel--merge-headers): new function that resolves headers based on their mutually exclusive groups, with better support for groups with `:any' keywords. added ob-core ihors suggestions implemented --- lisp/ob-core.el | 154 +++++++++++++++++++++++++++++++++++++----------- 1 file changed, 121 insertions(+), 33 deletions(-) diff --git a/lisp/ob-core.el b/lisp/ob-core.el index fb9df80c3..cd6c6708c 100644 --- a/lisp/ob-core.el +++ b/lisp/ob-core.el @@ -2843,6 +2843,88 @@ specified as an an \"attachment:\" style link." (goto-char body-start) (insert body)))) +(defun org-babel--merge-headers (exclusive-groups &rest related-params) + "Merge related cascading header parameters whilst upholding group exclusivity. + +A given header parameter (prop) can have multiple values +depending on where it is referenced in the outline path (olp) of +an Org document. The RELATED-PARAMS argument is a list that +contains the value of the prop at the current level of the olp, +as well the resolution of the last time this function was called +for the prop. The EXCLUSIVE-GROUPS argument contains the related +prop entry from `org-babel-common-header-args-w-values' and +dictates the valid values a prop can take. If the prop can have +more than one value, it defines consistent mutually exclusive +keywords that the prop can take." + ;; + ;; Having an :any keyword in an exclusion group means that a + ;; parameter of "yes" could match to an exclusion group that + ;; contains both "yes" AND ":any". + ;; + ;; For each parameter, build a list of exclusion groups it could + ;; belong to. If a parameter belongs to two groups, eliminate it + ;; from the group that contains the ":any" keyword. + ;; + ;; Exclusion groups are referenced to by their car, acting as a key + ;; for the entire group. + ;; + (let ((any-group-key ;; exclusion group key for group with :any keyword + ;; Ihor: cl-find is suggested here, but not sure how to apply. + (caar (cl-remove-if-not (lambda (x) (member ":any" x)) exclusive-groups))) + params-alist ;; param -> list( exclusion group keys ) + unexplained-params) ;; any params that were not caught + ;; Iterate parameters, across each exclusion group. + ;; - Populate params-alist + (dolist (new-params related-params) + (dolist (new-param new-params) + (dolist (exclusive-group exclusive-groups) + (if (or (member new-param exclusive-group) + (and (string= (car exclusive-group) any-group-key) + ;; param *doesn't* match a keyword in this + ;; :any group? Could be :any. + (not (member new-param exclusive-group)))) + (let ((group-key (car exclusive-group))) + (push group-key (alist-get new-param params-alist nil nil #'equal))) + ;; Some parameters fit into no groups, store them and process later. + (push new-param unexplained-params))))) + + (setq unexplained-params (delete-dups unexplained-params)) + ;; Find parameters listed in 2 or more exclusive groups, and kick + ;; them out of any non-":any" group. + ;; - Update params-alist + ;; - Remove uniquely known params from unexplained-params + (dolist (parm-vals params-alist) + (let ((parm (car parm-vals)) + (group-keys (delete-dups (cdr parm-vals)))) + (if (member parm unexplained-params) + (setq unexplained-params (delete parm unexplained-params))) + (if (> (length group-keys) 1) + (dolist (gkey group-keys) + (if (string= gkey any-group-key) + (setcdr (assoc parm params-alist) (delete gkey group-keys))))))) + ;; Collapse parameters into exclusion groups + ;; - convert params → list(exclusion group keys) to exclusion-group-key → list(params) + (let (group-alist) + (mapcar (lambda (x) + (let* ((key (cadr x)) + (val (car x)) + (existing (cdr (assoc key group-alist)))) + ;;(push val (alist-get key group-alist nil nil #'equal)) + (if existing + (setcdr (assoc key group-alist) (append existing (list val))) + (setq group-alist (cons (cons key (list val)) group-alist))))) + params-alist) + ;; Set values in the same order that the exclusion groups are defined + (let ((group-key-order (mapcar #'car exclusive-groups))) + (setq group-alist (delq nil (mapcar (lambda (x) (car (alist-get x group-alist))) + group-key-order)))) + + ;; Final Sanity Check: were all parameters explained? + ;; - if not, append to result + (if unexplained-params + (setq group-alist (append unexplained-params group-alist))) + group-alist))) + (defun org-babel-merge-params (&rest plists) "Combine all parameter association lists in PLISTS. Later elements of PLISTS override the values of previous elements. @@ -2854,26 +2936,15 @@ parameters when merging lists." (exports-exclusive-groups (mapcar (lambda (group) (mapcar #'symbol-name group)) (cdr (assq 'exports org-babel-common-header-args-w-values)))) - (merge - (lambda (exclusive-groups &rest result-params) - ;; Maintain exclusivity of mutually exclusive parameters, - ;; as defined in EXCLUSIVE-GROUPS while merging lists in - ;; RESULT-PARAMS. - (let (output) - (dolist (new-params result-params (delete-dups output)) - (dolist (new-param new-params) - (dolist (exclusive-group exclusive-groups) - (when (member new-param exclusive-group) - (setq output (cl-remove-if - (lambda (o) (member o exclusive-group)) - output)))) - (push new-param output)))))) + (tangle-exclusive-groups + (mapcar (lambda (group) (mapcar #'symbol-name group)) + (cdr (assq 'tangle org-babel-common-header-args-w-values)))) (variable-index 0) ;Handle positional arguments. clearnames params ;Final parameters list. ;; Some keywords accept multiple values. We need to treat ;; them specially. - vars results exports) + vars results exports tangle) (dolist (plist plists) (dolist (pair plist) (pcase pair @@ -2908,22 +2979,33 @@ parameters when merging lists." (t (error "Variable \"%s\" must be assigned a default value" (cdr pair)))))) (`(:results . ,value) - (setq results (funcall merge - results-exclusive-groups - results - (split-string - (cond ((stringp value) value) - ((functionp value) (funcall value)) + (setq results (org-babel--merge-headers + results-exclusive-groups + results + (split-string + (cond ((stringp value) value) + ((functionp value) (funcall value)) ;; FIXME: Arbitrary code evaluation. - (t (eval value t))))))) + (t (eval value t))))))) (`(:exports . ,value) - (setq exports (funcall merge - exports-exclusive-groups - exports - (split-string - (cond ((and value (functionp value)) (funcall value)) - (value value) - (t "")))))) + (setq exports (org-babel--merge-headers + exports-exclusive-groups + exports + (split-string + (cond ((and value (functionp value)) (funcall value)) + (value value) + (t "")))))) + (`(:tangle . ,value) + (setq tangle (org-babel--merge-headers + tangle-exclusive-groups + tangle + (mapcar #'org-babel-read + (org-babel-balanced-split + (or value "") + (if (eq (car (text-properties-at 0 value)) + 'org-babel-quote) + ?\" + ?\s)))))) ((or '(:dir . attach) '(:dir . "'attach")) (unless (org-attach-dir nil t) (error "No attachment directory for element (add :ID: or :DIR: property)")) @@ -2949,7 +3031,8 @@ parameters when merging lists." params))))) ;; Handle other special keywords, which accept multiple values. (setq params (nconc (list (cons :results (mapconcat #'identity results " ")) - (cons :exports (mapconcat #'identity exports " "))) + (cons :exports (mapconcat #'identity exports " ")) + (cons :tangle (mapconcat #'substring-no-properties tangle " "))) params)) ;; Return merged params. (org-babel-eval-headers params))) @@ -3246,9 +3329,14 @@ situations in which is it not appropriate." ;; FIXME: Arbitrary code evaluation. (eval (read cell) t)) ((save-match-data - (and (string-match "^[[:space:]]*\"\\(.*\\)\"[[:space:]]*$" cell) - (not (string-match "[^\\]\"" (match-string 1 cell))))) - (read cell)) + (and (string-match (rx bol (* space) "\"" (group (* any)) "\"" (* space) eol) + cell) + (not (string-match (rx (not "\\") "\"") (match-string 1 cell))))) + (let ((res (read cell))) + ;; If the matched string contains quotes, add quote property + (if (string-match (rx bol "\"" (* any) "\"" eol) cell) + (put-text-property 0 (length res) 'org-babel-quote 't res)) + res)) (t (org-no-properties cell)))) (defun org-babel--string-to-number (string) -- 2.41.0 [-- Attachment #5: 0003-testing-lisp-test-ob.el-update-tests-according-to-ne.patch --] [-- Type: text/x-patch, Size: 4316 bytes --] From 90c2e30cd27e783670856ff618efc5c0d4674bdd Mon Sep 17 00:00:00 2001 From: Mehmet Tekman <mtekman89@gmail.com> Date: Fri, 4 Aug 2023 13:06:58 +0200 Subject: [PATCH 3/4] * testing/lisp/test-ob.el: update tests according to new merge strategy, with emphasis on `:tangle' headers for syncing actions. updated test --- testing/lisp/test-ob.el | 43 ++++++++++++++++++++++++++++++++--------- 1 file changed, 34 insertions(+), 9 deletions(-) diff --git a/testing/lisp/test-ob.el b/testing/lisp/test-ob.el index f12533258..d51937b69 100644 --- a/testing/lisp/test-ob.el +++ b/testing/lisp/test-ob.el @@ -337,7 +337,7 @@ PROPERTIES is a list of property keywords or a single keyword." #+begin_src conf #+end_src" (test-ob/get-src-block-property :tangle)))) - (should-not ;; 2. inherit-document-header-with-local-sync-action + (should ;; 2. inherit-document-header-with-local-sync-action ;; This should pass with newer merge function with multiple tangle parameters (equal '(:tangle "/tmp/default_tangle.txt skip") (org-test-with-temp-text @@ -377,7 +377,7 @@ PROPERTIES is a list of property keywords or a single keyword." #+begin_src conf :tangle randomfile #+end_src" (test-ob/get-src-block-property '(:tangle :results))))) - (should-not ;; 6. inherit-document-tfile-take-only-last-local-sync-action + (should ;; 6. inherit-document-tfile-take-only-last-local-sync-action ;; This should pass with newer merge function with multiple tangle parameters (equal '(:tangle "/tmp/default_tangle.txt export") (org-test-with-temp-text @@ -387,7 +387,7 @@ PROPERTIES is a list of property keywords or a single keyword." #+begin_src conf :tangle import export #+end_src" (test-ob/get-src-block-property :tangle)))) - (should-not ;; 7. ignore-document-header-take-last-tfile-and-sync-action + (should ;; 7. ignore-document-header-take-last-tfile-and-sync-action ;; This should pass with newer merge function with multiple tangle parameters (equal '(:tangle "fname2 export") (org-test-with-temp-text @@ -426,9 +426,8 @@ PROPERTIES is a list of property keywords or a single keyword." #+begin_src conf :tangle \"foo.txt\" :comments link #+end_src" (test-ob/get-src-block-property '(:tangle :exports :comments))))) - (should-not ;; 11. override-document-and-heading-tfile-with-yes - ;; This should pass with newer merge function with multiple tangle parameters - (equal '(:tangle "foo.txt") + (should ;; 11. override-document-and-heading-tfile-with-yes + (equal '(:tangle "yes") (org-test-with-temp-text "\ #+PROPERTY: header-args :tangle /tmp/default_tangle.txt @@ -439,18 +438,44 @@ PROPERTIES is a list of property keywords or a single keyword." #+begin_src conf :tangle yes #+end_src" (test-ob/get-src-block-property :tangle)))) - (should ;; 12. tangle-file-with-spaces + (should ;; 12. simple-file-with-spaces + (equal '(:tangle "file with spaces.txt import") + (org-test-with-temp-text "\ +#+PROPERTY: header-args :tangle \"file with spaces.txt\" sync +* Twelve +#+begin_src conf :tangle import +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 13. hierarchical-tangle-file-with-spaces (equal '(:tangle "file with spaces.txt") (org-test-with-temp-text "\ -* Twelve +* Thirteen :PROPERTIES: :header-args: :tangle \"foo.txt\" :END: ** A #+begin_src conf :tangle \"file with spaces.txt\" #+end_src" - (test-ob/get-src-block-property :tangle))))) + (test-ob/get-src-block-property :tangle)))) + (should ;; 14. do-not-tangle + (equal '(:tangle "no") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Fourteen +#+begin_src emacs-lisp :tangle no +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 15. headers-dont-cancel-out + (equal '(:tangle "relative.el") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle relative.el +* Fifteen +#+begin_src emacs-lisp :tangle relative.el +#+end_src" + (test-ob/get-src-block-property :tangle))))) (ert-deftest test-ob/inline-src-blocks () (should -- 2.41.0 ^ permalink raw reply related [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-08-04 13:14 ` Mehmet Tekman @ 2023-08-04 16:34 ` Mehmet Tekman 2023-08-06 9:07 ` Ihor Radchenko 2023-08-05 8:48 ` Ihor Radchenko 1 sibling, 1 reply; 77+ messages in thread From: Mehmet Tekman @ 2023-08-04 16:34 UTC (permalink / raw) To: Ihor Radchenko; +Cc: emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 341 bytes --] Ignore the patches in that last email, (but please do address some of the concerns I had about the number of `:any` outputs). Anyway, I finally did it - I got it to work. Every single patch now pasts tests. Please review these patches and *not the patches in the last email*. (Apologies for the frequency of my emails today) Best, Mehmet [-- Attachment #2: 0001-testing-lisp-test-ob.el-New-tests-for-merge-params.patch --] [-- Type: text/x-patch, Size: 5253 bytes --] From 020992b46b5a7c2ae18c677afb5c29c00de58059 Mon Sep 17 00:00:00 2001 From: Mehmet Tekman <mtekman89@gmail.com> Date: Tue, 1 Aug 2023 05:14:46 +0200 Subject: [PATCH 1/3] * testing/lisp/test-ob.el: New tests for merge-params (test-ob/get-src-block-property): (test-ob/merge-params): add new tests for the merge-params source block header handling function. (test-ob/merge-params-tangle): add new tests for tangle specific headers. --- testing/lisp/test-ob.el | 146 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 146 insertions(+) diff --git a/testing/lisp/test-ob.el b/testing/lisp/test-ob.el index c8dbd44f4..460afbf9b 100644 --- a/testing/lisp/test-ob.el +++ b/testing/lisp/test-ob.el @@ -314,6 +314,152 @@ this is simple" (org-babel-next-src-block) (should (= 14 (org-babel-execute-src-block))))) +(defun test-ob/get-src-block-property (properties) + "Get plist of PROPERTIES and values for the first src block in buffer. +PROPERTIES is a list of property keywords or a single keyword." + (org-with-wide-buffer + (goto-char (point-min)) + (org-babel-next-src-block) + (org-set-regexps-and-options) + (let ((all-props (nth 2 (org-babel-get-src-block-info)))) + (if (listp properties) + (apply #'nconc (mapcar (lambda (p) (list p (cdr (assoc p all-props)))) properties)) + (list properties (cdr (assoc properties all-props))))))) + +(ert-deftest test-ob/merge-params() + "Test the output of merging multiple header parameters." + (should + (equal '(:file-mode "#o755") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :file-mode #o755 +* One +#+begin_src conf +#+end_src" + (test-ob/get-src-block-property :file-mode)))) + (should + (equal '(:file-mode "anything" :exports "both") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :file-mode #o755 :exports both +* One +#+begin_src conf :file-mode anything +#+end_src" + (test-ob/get-src-block-property '(:file-mode :exports))))) + (should + (equal '(:mkdirp "no" :dir nil :padline "yes") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :dir /tmp/ :padline no +* One +:PROPERTIES: +:header-args: :mkdirp no +:END: +#+begin_src conf :padline yes +#+end_src" + (test-ob/get-src-block-property '(:mkdirp :dir :padline))))) + (should + (equal '(:results "value code replace file" :file "file") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :results file :file file +* One +#+begin_src conf :results code value +#+end_src" + (test-ob/get-src-block-property '(:results :file))))) + (should + (equal '(:results "value html silent") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :results file +* One +:PROPERTIES: +:header-args: :results html silent +:END: +#+begin_src conf :results value +#+end_src" + (test-ob/get-src-block-property :results))))) + +(ert-deftest test-ob/merge-params-tangle() + "Test the output of merging multiple header parameters." + (should + (equal '(:tangle "/tmp/default_tangle.txt") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* One +#+begin_src conf +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 2. inherit-document-header-with-local-sync-action + (equal '(:tangle "skip") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Two +#+begin_src conf :tangle skip +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should + (equal '(:tangle "randomfile") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Three +#+begin_src conf :tangle randomfile +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should + (equal '(:tangle "newfile.txt") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Four +:PROPERTIES: +:header-args: :tangle \"newfile.txt\" +:END: +** A +#+begin_src conf +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should + (equal '(:tangle "./headfile.txt") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle ./headfile.txt +* Five +:PROPERTIES: +:header-args: :tangle ./headfile.txt +:END: +** A +#+begin_src conf +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should + (equal '(:tangle "yes") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Eleven +:PROPERTIES: +:header-args: :tangle \"foo.txt\" +:END: +#+begin_src conf :tangle yes +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should + (equal '(:tangle "file with spaces.txt") + (org-test-with-temp-text + "\ +* Twelve +:PROPERTIES: +:header-args: :tangle \"foo.txt\" +:END: +** A +#+begin_src conf :tangle \"file with spaces.txt\" +#+end_src" + (test-ob/get-src-block-property :tangle))))) + (ert-deftest test-ob/inline-src-blocks () (should (= 1 -- 2.41.0 [-- Attachment #3: 0003-lisp-ob-core.el.patch --] [-- Type: text/x-patch, Size: 8135 bytes --] From 033c5ef74f54c1a83a9f3de2590700d852b9a344 Mon Sep 17 00:00:00 2001 From: Mehmet Tekman <mtekman89@gmail.com> Date: Fri, 4 Aug 2023 18:17:34 +0200 Subject: [PATCH 3/3] * lisp/ob-core.el: (org-babel-common-header-args-w-values): Added mutually exclusive tangle groups relating to desired tangle sync actions * testing/lisp/test-ob.el: (test-ob/merge-params-tangle): Updated test function to take into account the new headers --- lisp/ob-core.el | 3 +- testing/lisp/test-ob.el | 133 +++++++++++++++++++++++++++++++++++++--- 2 files changed, 127 insertions(+), 9 deletions(-) diff --git a/lisp/ob-core.el b/lisp/ob-core.el index cd6c6708c..f09540f18 100644 --- a/lisp/ob-core.el +++ b/lisp/ob-core.el @@ -441,7 +441,8 @@ then run `org-babel-switch-to-session'." (sep . :any) (session . :any) (shebang . :any) - (tangle . ((tangle yes no :any))) + (tangle . ((tangle yes no :any) + (import export skip sync))) (tangle-mode . ((#o755 #o555 #o444 :any))) (var . :any) (wrap . :any)) diff --git a/testing/lisp/test-ob.el b/testing/lisp/test-ob.el index 48cb60e10..60f385663 100644 --- a/testing/lisp/test-ob.el +++ b/testing/lisp/test-ob.el @@ -382,7 +382,7 @@ PROPERTIES is a list of property keywords or a single keyword." (ert-deftest test-ob/merge-params-tangle() "Test the output of merging multiple header parameters." - (should + (should ;; 1. inherit-document-header-args (equal '(:tangle "/tmp/default_tangle.txt") (org-test-with-temp-text "\ @@ -391,8 +391,8 @@ PROPERTIES is a list of property keywords or a single keyword." #+begin_src conf #+end_src" (test-ob/get-src-block-property :tangle)))) - (should ;; 2. inherit-document-header-with-local-sync-action - (equal '(:tangle "skip") + (should ;; 2. inherit-document-header + (equal '(:tangle "/tmp/default_tangle.txt skip") (org-test-with-temp-text "\ #+PROPERTY: header-args :tangle /tmp/default_tangle.txt @@ -400,7 +400,7 @@ PROPERTIES is a list of property keywords or a single keyword." #+begin_src conf :tangle skip #+end_src" (test-ob/get-src-block-property :tangle)))) - (should + (should ;; 3. override-document-header-with-local-tfile (equal '(:tangle "randomfile") (org-test-with-temp-text "\ @@ -409,7 +409,16 @@ PROPERTIES is a list of property keywords or a single keyword." #+begin_src conf :tangle randomfile #+end_src" (test-ob/get-src-block-property :tangle)))) - (should + (should ;; 3b. override-document-header-with-local-tfile-sync + (equal '(:tangle "randomfile sync") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* ThreeA +#+begin_src conf :tangle randomfile sync +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 4. override-document-and-parent-header-with-local-tfile (equal '(:tangle "newfile.txt") (org-test-with-temp-text "\ @@ -422,7 +431,20 @@ PROPERTIES is a list of property keywords or a single keyword." #+begin_src conf #+end_src" (test-ob/get-src-block-property :tangle)))) - (should + (should ;; 4a. override-document-and-parent-header-with-local-tfile-action + (equal '(:tangle "randomfile sync") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Four +:PROPERTIES: +:header-args: :tangle \"newfile.txt\" import +:END: +** A +#+begin_src conf :tangle randomfile sync +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 5. tangle-file-name-duplicate (equal '(:tangle "./headfile.txt") (org-test-with-temp-text "\ @@ -435,7 +457,63 @@ PROPERTIES is a list of property keywords or a single keyword." #+begin_src conf #+end_src" (test-ob/get-src-block-property :tangle)))) - (should + (should ;; 5a. test-tangle-and-default-results-param-together + (equal '(:tangle "randomfile" :results "replace") + (org-test-with-temp-text + "\ +* FiveA +#+begin_src conf :tangle randomfile +#+end_src" + (test-ob/get-src-block-property '(:tangle :results))))) + (should ;; 6. inherit-document-tfile-take-only-last-local-sync-action + (equal '(:tangle "/tmp/default_tangle.txt export") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Six +#+begin_src conf :tangle import export +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 7. ignore-document-header-take-last-tfile-and-sync-action + ;; This should pass with newer merge function with multiple tangle parameters + (equal '(:tangle "fname2 export") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Seven +#+begin_src conf :tangle fname1 fname2 sync export +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 8. test-results-and-exports + (equal '(:results "wrap file replace" :exports "code") + (org-test-with-temp-text + "\ +* Eight +#+begin_src sh :results file wrap +#+end_src" + (test-ob/get-src-block-property '(:results :exports))))) + (should ;; 9. do-not-tangle-this-block -- + (equal '(:tangle "no") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Nine +#+begin_src conf :tangle no +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 10. test-tangle-exports-and-comments + (equal '(:tangle "foo.txt" :exports "verbatim code" :comments "link") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Ten +:PROPERTIES: +:header-args: :tangle no :exports verbatim +:END: +#+begin_src conf :tangle \"foo.txt\" :comments link +#+end_src" + (test-ob/get-src-block-property '(:tangle :exports :comments))))) + (should ;; 11. override-document-and-heading-tfile-with-yes (equal '(:tangle "yes") (org-test-with-temp-text "\ @@ -447,7 +525,7 @@ PROPERTIES is a list of property keywords or a single keyword." #+begin_src conf :tangle yes #+end_src" (test-ob/get-src-block-property :tangle)))) - (should + (should ;; 12. simple-file-with-space (equal '(:tangle "file with spaces.txt") (org-test-with-temp-text "\ @@ -457,9 +535,48 @@ PROPERTIES is a list of property keywords or a single keyword." :END: ** A #+begin_src conf :tangle \"file with spaces.txt\" +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 12a. simple-file-with-spaces-action + (equal '(:tangle "file with spaces.txt import") + (org-test-with-temp-text "\ +#+PROPERTY: header-args :tangle \"file with spaces.txt\" sync +* TwelveA +#+begin_src conf :tangle import +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 13. hierarchical-tangle-file-with-spaces + (equal '(:tangle "file with spaces.txt") + (org-test-with-temp-text + "\ +* Thirteen +:PROPERTIES: +:header-args: :tangle \"foo.txt\" +:END: +** A +#+begin_src conf :tangle \"file with spaces.txt\" +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 14. do-not-tangle + (equal '(:tangle "no") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Fourteen +#+begin_src emacs-lisp :tangle no +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 15. headers-dont-cancel-out + (equal '(:tangle "relative.el") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle relative.el +* Fifteen +#+begin_src emacs-lisp :tangle relative.el #+end_src" (test-ob/get-src-block-property :tangle))))) + (ert-deftest test-ob/inline-src-blocks () (should (= 1 -- 2.41.0 [-- Attachment #4: 0002-lisp-ob-core.el-Rewrite-of-merge-babel-headers.patch --] [-- Type: text/x-patch, Size: 11408 bytes --] From 1866c082547f75f38e221117e4eff2f0124d69f7 Mon Sep 17 00:00:00 2001 From: Mehmet Tekman <mtekman89@gmail.com> Date: Wed, 10 May 2023 17:38:22 +0200 Subject: [PATCH 2/3] * lisp/ob-core.el: Rewrite of merge babel headers (org-babel-merge-params): merge headers strategy split out into new dedicated function `--merge-headers' (org-babel--merge-headers): new function that resolves headers based on their mutually exclusive groups, with better support for groups with `:any' keywords. * lisp/test-ob.el (test-ob/merge-params): Very minor test value update due to rewrite --- lisp/ob-core.el | 154 +++++++++++++++++++++++++++++++--------- testing/lisp/test-ob.el | 4 +- 2 files changed, 123 insertions(+), 35 deletions(-) diff --git a/lisp/ob-core.el b/lisp/ob-core.el index fb9df80c3..cd6c6708c 100644 --- a/lisp/ob-core.el +++ b/lisp/ob-core.el @@ -2843,6 +2843,88 @@ specified as an an \"attachment:\" style link." (goto-char body-start) (insert body)))) +(defun org-babel--merge-headers (exclusive-groups &rest related-params) + "Merge related cascading header parameters whilst upholding group exclusivity. + +A given header parameter (prop) can have multiple values +depending on where it is referenced in the outline path (olp) of +an Org document. The RELATED-PARAMS argument is a list that +contains the value of the prop at the current level of the olp, +as well the resolution of the last time this function was called +for the prop. The EXCLUSIVE-GROUPS argument contains the related +prop entry from `org-babel-common-header-args-w-values' and +dictates the valid values a prop can take. If the prop can have +more than one value, it defines consistent mutually exclusive +keywords that the prop can take." + ;; + ;; Having an :any keyword in an exclusion group means that a + ;; parameter of "yes" could match to an exclusion group that + ;; contains both "yes" AND ":any". + ;; + ;; For each parameter, build a list of exclusion groups it could + ;; belong to. If a parameter belongs to two groups, eliminate it + ;; from the group that contains the ":any" keyword. + ;; + ;; Exclusion groups are referenced to by their car, acting as a key + ;; for the entire group. + ;; + (let ((any-group-key ;; exclusion group key for group with :any keyword + ;; Ihor: cl-find is suggested here, but not sure how to apply. + (caar (cl-remove-if-not (lambda (x) (member ":any" x)) exclusive-groups))) + params-alist ;; param -> list( exclusion group keys ) + unexplained-params) ;; any params that were not caught + ;; Iterate parameters, across each exclusion group. + ;; - Populate params-alist + (dolist (new-params related-params) + (dolist (new-param new-params) + (dolist (exclusive-group exclusive-groups) + (if (or (member new-param exclusive-group) + (and (string= (car exclusive-group) any-group-key) + ;; param *doesn't* match a keyword in this + ;; :any group? Could be :any. + (not (member new-param exclusive-group)))) + (let ((group-key (car exclusive-group))) + (push group-key (alist-get new-param params-alist nil nil #'equal))) + ;; Some parameters fit into no groups, store them and process later. + (push new-param unexplained-params))))) + + (setq unexplained-params (delete-dups unexplained-params)) + ;; Find parameters listed in 2 or more exclusive groups, and kick + ;; them out of any non-":any" group. + ;; - Update params-alist + ;; - Remove uniquely known params from unexplained-params + (dolist (parm-vals params-alist) + (let ((parm (car parm-vals)) + (group-keys (delete-dups (cdr parm-vals)))) + (if (member parm unexplained-params) + (setq unexplained-params (delete parm unexplained-params))) + (if (> (length group-keys) 1) + (dolist (gkey group-keys) + (if (string= gkey any-group-key) + (setcdr (assoc parm params-alist) (delete gkey group-keys))))))) + ;; Collapse parameters into exclusion groups + ;; - convert params → list(exclusion group keys) to exclusion-group-key → list(params) + (let (group-alist) + (mapcar (lambda (x) + (let* ((key (cadr x)) + (val (car x)) + (existing (cdr (assoc key group-alist)))) + ;;(push val (alist-get key group-alist nil nil #'equal)) + (if existing + (setcdr (assoc key group-alist) (append existing (list val))) + (setq group-alist (cons (cons key (list val)) group-alist))))) + params-alist) + ;; Set values in the same order that the exclusion groups are defined + (let ((group-key-order (mapcar #'car exclusive-groups))) + (setq group-alist (delq nil (mapcar (lambda (x) (car (alist-get x group-alist))) + group-key-order)))) + + ;; Final Sanity Check: were all parameters explained? + ;; - if not, append to result + (if unexplained-params + (setq group-alist (append unexplained-params group-alist))) + group-alist))) + (defun org-babel-merge-params (&rest plists) "Combine all parameter association lists in PLISTS. Later elements of PLISTS override the values of previous elements. @@ -2854,26 +2936,15 @@ parameters when merging lists." (exports-exclusive-groups (mapcar (lambda (group) (mapcar #'symbol-name group)) (cdr (assq 'exports org-babel-common-header-args-w-values)))) - (merge - (lambda (exclusive-groups &rest result-params) - ;; Maintain exclusivity of mutually exclusive parameters, - ;; as defined in EXCLUSIVE-GROUPS while merging lists in - ;; RESULT-PARAMS. - (let (output) - (dolist (new-params result-params (delete-dups output)) - (dolist (new-param new-params) - (dolist (exclusive-group exclusive-groups) - (when (member new-param exclusive-group) - (setq output (cl-remove-if - (lambda (o) (member o exclusive-group)) - output)))) - (push new-param output)))))) + (tangle-exclusive-groups + (mapcar (lambda (group) (mapcar #'symbol-name group)) + (cdr (assq 'tangle org-babel-common-header-args-w-values)))) (variable-index 0) ;Handle positional arguments. clearnames params ;Final parameters list. ;; Some keywords accept multiple values. We need to treat ;; them specially. - vars results exports) + vars results exports tangle) (dolist (plist plists) (dolist (pair plist) (pcase pair @@ -2908,22 +2979,33 @@ parameters when merging lists." (t (error "Variable \"%s\" must be assigned a default value" (cdr pair)))))) (`(:results . ,value) - (setq results (funcall merge - results-exclusive-groups - results - (split-string - (cond ((stringp value) value) - ((functionp value) (funcall value)) + (setq results (org-babel--merge-headers + results-exclusive-groups + results + (split-string + (cond ((stringp value) value) + ((functionp value) (funcall value)) ;; FIXME: Arbitrary code evaluation. - (t (eval value t))))))) + (t (eval value t))))))) (`(:exports . ,value) - (setq exports (funcall merge - exports-exclusive-groups - exports - (split-string - (cond ((and value (functionp value)) (funcall value)) - (value value) - (t "")))))) + (setq exports (org-babel--merge-headers + exports-exclusive-groups + exports + (split-string + (cond ((and value (functionp value)) (funcall value)) + (value value) + (t "")))))) + (`(:tangle . ,value) + (setq tangle (org-babel--merge-headers + tangle-exclusive-groups + tangle + (mapcar #'org-babel-read + (org-babel-balanced-split + (or value "") + (if (eq (car (text-properties-at 0 value)) + 'org-babel-quote) + ?\" + ?\s)))))) ((or '(:dir . attach) '(:dir . "'attach")) (unless (org-attach-dir nil t) (error "No attachment directory for element (add :ID: or :DIR: property)")) @@ -2949,7 +3031,8 @@ parameters when merging lists." params))))) ;; Handle other special keywords, which accept multiple values. (setq params (nconc (list (cons :results (mapconcat #'identity results " ")) - (cons :exports (mapconcat #'identity exports " "))) + (cons :exports (mapconcat #'identity exports " ")) + (cons :tangle (mapconcat #'substring-no-properties tangle " "))) params)) ;; Return merged params. (org-babel-eval-headers params))) @@ -3246,9 +3329,14 @@ situations in which is it not appropriate." ;; FIXME: Arbitrary code evaluation. (eval (read cell) t)) ((save-match-data - (and (string-match "^[[:space:]]*\"\\(.*\\)\"[[:space:]]*$" cell) - (not (string-match "[^\\]\"" (match-string 1 cell))))) - (read cell)) + (and (string-match (rx bol (* space) "\"" (group (* any)) "\"" (* space) eol) + cell) + (not (string-match (rx (not "\\") "\"") (match-string 1 cell))))) + (let ((res (read cell))) + ;; If the matched string contains quotes, add quote property + (if (string-match (rx bol "\"" (* any) "\"" eol) cell) + (put-text-property 0 (length res) 'org-babel-quote 't res)) + res)) (t (org-no-properties cell)))) (defun org-babel--string-to-number (string) diff --git a/testing/lisp/test-ob.el b/testing/lisp/test-ob.el index 460afbf9b..48cb60e10 100644 --- a/testing/lisp/test-ob.el +++ b/testing/lisp/test-ob.el @@ -359,7 +359,7 @@ PROPERTIES is a list of property keywords or a single keyword." #+end_src" (test-ob/get-src-block-property '(:mkdirp :dir :padline))))) (should - (equal '(:results "value code replace file" :file "file") + (equal '(:results "file code replace value" :file "file") (org-test-with-temp-text "\ #+PROPERTY: header-args :results file :file file @@ -368,7 +368,7 @@ PROPERTIES is a list of property keywords or a single keyword." #+end_src" (test-ob/get-src-block-property '(:results :file))))) (should - (equal '(:results "value html silent") + (equal '(:results "html silent value") (org-test-with-temp-text "\ #+PROPERTY: header-args :results file -- 2.41.0 ^ permalink raw reply related [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-08-04 16:34 ` Mehmet Tekman @ 2023-08-06 9:07 ` Ihor Radchenko 2023-08-08 19:41 ` Mehmet Tekman 0 siblings, 1 reply; 77+ messages in thread From: Ihor Radchenko @ 2023-08-06 9:07 UTC (permalink / raw) To: Mehmet Tekman; +Cc: emacs-orgmode Mehmet Tekman <mtekman89@gmail.com> writes: > Anyway, I finally did it - I got it to work. Every single patch now > pasts tests. Consider the following edge case: #+PROPERTY: header-args :tangle "file name with spaces and no word.txt" #+begin_src emacs-lisp :tangle sync #+end_src With your patch, I got :tangle "file name with spaces and no word.txt sync". Also, there is actually a big problem across Org where :tangle parameter is often checked using expressions like (cdr (assq :tangle (nth 2 info))) No splitting is done, and the assumption is made that the value cannot be compound. It looks like we are opening a Pandora box with this feature. And rather than fixing all the places that use :tangle across Org, we may need to create a new API to retrieve header argument values uniformly, without knowing the details about how to split PARAMS string. We may have to use the approach employed for :results header argument where in addition to (:results . "string value"), we have (:result-params "sub-value" "sub-value" ...). :result-params is now handled by `org-babel-process-params'. We may need to extend this function to work with all the possible header arguments that have multiple exclusive groups and do not have a single allowed value. -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-08-06 9:07 ` Ihor Radchenko @ 2023-08-08 19:41 ` Mehmet Tekman 2023-08-08 19:51 ` Ihor Radchenko 0 siblings, 1 reply; 77+ messages in thread From: Mehmet Tekman @ 2023-08-08 19:41 UTC (permalink / raw) To: Ihor Radchenko; +Cc: emacs-orgmode Hello, I'm a bit overworked at the moment, sorry for the delay in response. > We may have to use the approach employed for :results header argument > where in addition to (:results . "string value"), we have > (:result-params "sub-value" "sub-value" ...). Isn't this similar to my `:tangle-sync' parameter, i.e, completely separate from the `:tangle' header itself but complementary to it? Why can't we just go back to that? Best ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-08-08 19:41 ` Mehmet Tekman @ 2023-08-08 19:51 ` Ihor Radchenko 2023-08-08 20:04 ` Mehmet Tekman 0 siblings, 1 reply; 77+ messages in thread From: Ihor Radchenko @ 2023-08-08 19:51 UTC (permalink / raw) To: Mehmet Tekman; +Cc: emacs-orgmode Mehmet Tekman <mtekman89@gmail.com> writes: > Hello, I'm a bit overworked at the moment, sorry for the delay in > response. No worries. We are not in hurry. Take your time as needed and comfortable. >> We may have to use the approach employed for :results header argument >> where in addition to (:results . "string value"), we have >> (:result-params "sub-value" "sub-value" ...). > > Isn't this similar to my `:tangle-sync' parameter, i.e, completely separate > from the `:tangle' header itself but complementary to it? No. :result-params is not an actual header argument. It is implementation detail - parsing :results header arg internally produces (:results . "all the values concatenated") and _also_ (:result-params "val 1" "val 2" ...). > Why can't we just go back to that? We can. I did not expect that we would need to go this far into the rabbit hole. Although, I still think that unifying header argument parsing is something worth doing, even if I need to implement it myself. -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-08-08 19:51 ` Ihor Radchenko @ 2023-08-08 20:04 ` Mehmet Tekman 2023-08-09 8:04 ` Ihor Radchenko 0 siblings, 1 reply; 77+ messages in thread From: Mehmet Tekman @ 2023-08-08 20:04 UTC (permalink / raw) To: Ihor Radchenko; +Cc: emacs-orgmode Ihor Radchenko <yantar92@posteo.net> writes: > No. :result-params is not an actual header argument. It is > implementation detail - parsing :results header arg internally produces > (:results . "all the values concatenated") and _also_ (:result-params > "val 1" "val 2" ...). Hmm... what is the benefit if they encode the same information? (e.g. :tangle "one two \"three and four\"" vs :tangle-params "one" "two "three and four"). Edit: Oh, I see -- Org parses the action from the `:tangle-params' and Users can extend tangle as much as they want via `:tangle' > > Why can't we just go back to that? > > We can. I did not expect that we would need to go this far into the > rabbit hole. Although, I still think that unifying header argument > parsing is something worth doing, even if I need to implement it myself. Hah, me either. At the same time, it doesn't make sense to have `:tangle-sync' present in one release of Org-mode and then deprecated in the next (after your header rewrite). It would just confuse users. I'm happy to have another go at a unified header approach, I will just need to explore the code base a bit more. ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-08-08 20:04 ` Mehmet Tekman @ 2023-08-09 8:04 ` Ihor Radchenko 0 siblings, 0 replies; 77+ messages in thread From: Ihor Radchenko @ 2023-08-09 8:04 UTC (permalink / raw) To: Mehmet Tekman; +Cc: emacs-orgmode Mehmet Tekman <mtekman89@gmail.com> writes: > Ihor Radchenko <yantar92@posteo.net> writes: > >> No. :result-params is not an actual header argument. It is >> implementation detail - parsing :results header arg internally produces >> (:results . "all the values concatenated") and _also_ (:result-params >> "val 1" "val 2" ...). > > Hmm... what is the benefit if they encode the same > information? (e.g. :tangle "one two \"three and four\"" vs > :tangle-params "one" "two "three and four"). > > Edit: Oh, I see -- Org parses the action from the `:tangle-params' > and Users can extend tangle as much as they want via `:tangle' No, not really. The main purpose is to avoid calling `split-string' every time we need to check :results settings. >> > Why can't we just go back to that? >> >> We can. I did not expect that we would need to go this far into the >> rabbit hole. Although, I still think that unifying header argument >> parsing is something worth doing, even if I need to implement it myself. > > Hah, me either. At the same time, it doesn't make sense to have > `:tangle-sync' present in one release of Org-mode and then deprecated in > the next (after your header rewrite). It would just confuse users. > > I'm happy to have another go at a unified header approach, I will > just need to explore the code base a bit more. Thanks! Feel free to ask anything. Babel code is not always easy to read. -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-08-04 13:14 ` Mehmet Tekman 2023-08-04 16:34 ` Mehmet Tekman @ 2023-08-05 8:48 ` Ihor Radchenko 2023-08-05 22:54 ` Mehmet Tekman 1 sibling, 1 reply; 77+ messages in thread From: Ihor Radchenko @ 2023-08-05 8:48 UTC (permalink / raw) To: Mehmet Tekman; +Cc: emacs-orgmode Mehmet Tekman <mtekman89@gmail.com> writes: > * Under vanilla `org-babel-merge-params', any number of :tangle header > values are permitted and concatenated together. > > e.g 4: > __________ > ´ > (should ;; 4. > override-document-and-parent-header-with-local-tfile-and-action > (equal '(:tangle "randomfile sync") > (org-test-with-temp-text > "\ > #+PROPERTY: header-args :tangle /tmp/default_tangle.txt > * Four > :PROPERTIES: > :header-args: :tangle \"newfile.txt\" import > :END: > ** A > #+begin_src conf :tangle randomfile sync > #+end_src" > (test-ob/get-src-block-property :tangle)))) > `----------- > > This result of "randomfile sync" for the :tangle header is seen as > valid.... but it shouldn't, should it? Tangle can only take one value, > so the action of `:any` should also just output one value? > > From the docstring text: > > > Symbol `:any' in the value list implies that any value is allowed. > > It's not clear to me if this means that `:any' or `:any :any` or > `":any :any"` is permitted. > > In my rewrite, I very much take the `:any` or `":any :any"` single > value interpretation when merging the headers together. It is not possible to distinguish between `:any' and `:any any' without reading the user's mind :) So, we can choose arbitrarily. Since the previous version of the code used split-string and unconditionally split the arguments at whitespace, I am inclined to prefer `:any :any' over greedy version. So, #+begin_src lang :tangle yes foo no bar baz "foo bar" should yield '("yes" "foo" "no" "bar" "baz" #("foo bar" 0 7 (org-babel-quote t))) We will allow using quotes to include whitespace. > * Sometimes the value of another header is caught in the value of > another > > e.g 8: "wrap" is caught in the output of `:results' despite it not being > a valid results keyword in `org-babel-common-header-args-w-values'. > > __________ > ´ > (should ;; 8. test-results-and-exports > (equal '(:results "wrap file replace" :exports "code") > (org-test-with-temp-text > "\ > * Eight > #+begin_src sh :results file wrap > #+end_src" > (test-ob/get-src-block-property '(:results :exports))))) > (should ;; 9. do-not-tangle-this-block -- > `----------- > > This test results in "true" for both my rewrite, and the vanilla > function, but I'm not sure how accurate or value this is. This is basically a question about what to do when user supplies incorrect value of a header arg. In such scenarios, we still capture this unknown value. This is useful for some third-party backends that extend standard header args with new possible values. Hope the above makes sense. -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-08-05 8:48 ` Ihor Radchenko @ 2023-08-05 22:54 ` Mehmet Tekman 2023-11-10 9:41 ` Ihor Radchenko 0 siblings, 1 reply; 77+ messages in thread From: Mehmet Tekman @ 2023-08-05 22:54 UTC (permalink / raw) To: Ihor Radchenko; +Cc: emacs-orgmode Hello > #+begin_src lang :tangle yes foo no bar baz "foo bar" > should yield > '("yes" "foo" "no" "bar" "baz" #("foo bar" 0 7 (org-babel-quote t))) > We will allow using quotes to include whitespace. > In such scenarios, we still capture this unknown value. > This is useful for some third-party backends that extend standard header > args with new possible values. Okay, I will definitely need to refactor my patches then, but I'm quite happy with the state of my current patch branch, so it will be an additional patch instead of a brand new set of patches I think. Best, M ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-08-05 22:54 ` Mehmet Tekman @ 2023-11-10 9:41 ` Ihor Radchenko 2023-11-10 9:53 ` Mehmet Tekman 0 siblings, 1 reply; 77+ messages in thread From: Ihor Radchenko @ 2023-11-10 9:41 UTC (permalink / raw) To: Mehmet Tekman; +Cc: emacs-orgmode Mehmet Tekman <mtekman89@gmail.com> writes: > Okay, I will definitely need to refactor my patches then, but I'm > quite happy with the state of my current patch branch, so it will > be an additional patch instead of a brand new set of patches > I think. It has been a while since the most recent message in this thread. May I know if you need any further help with the patch? -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-11-10 9:41 ` Ihor Radchenko @ 2023-11-10 9:53 ` Mehmet Tekman 2023-12-11 13:40 ` Ihor Radchenko 0 siblings, 1 reply; 77+ messages in thread From: Mehmet Tekman @ 2023-11-10 9:53 UTC (permalink / raw) To: Ihor Radchenko; +Cc: emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 921 bytes --] Ihor Radchenko <yantar92@posteo.net> writes: > Mehmet Tekman <mtekman89@gmail.com> writes: > >> Okay, I will definitely need to refactor my patches then, but I'm >> quite happy with the state of my current patch branch, so it will >> be an additional patch instead of a brand new set of patches >> I think. > > It has been a while since the most recent message in this thread. > May I know if you need any further help with the patch? > Hey, thanks for reaching out. No, I know what to do -- it's just finding the time to do it that is proving difficult. I made some small edits a few months ago, to reform the header arguments so that tangle actions would be encoded in the `:tangle-params' header and would largely leave the `:tangle' header alone… but when I tested it, it was failing all tests for reasons I wasn't to sure about. Here is the current state of my header-reform branch: [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: header reform patch --] [-- Type: text/x-patch, Size: 3241 bytes --] From 9ba64d149aac6c807e233e34474ffe022488efe6 Mon Sep 17 00:00:00 2001 From: Mehmet Tekman <mtekman89@gmail.com> Date: Wed, 20 Sep 2023 11:35:00 +0200 Subject: [PATCH] header refs --- lisp/ob-core.el | 48 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/lisp/ob-core.el b/lisp/ob-core.el index a7c4f2cab..0b0d1c18c 100644 --- a/lisp/ob-core.el +++ b/lisp/ob-core.el @@ -1753,6 +1753,42 @@ HEADER-ARGUMENTS is alist of all the arguments." header-arguments) (nreverse results))) + +(defun org-babel--split-str-quoted (str) + "Splits a string that may or may not contain quotes." + (let (result pos) + (while (string-match (rx (or (seq "\"" (group (* (not "\""))) "\"") + (group (+ (not space))))) + str pos) + (let ((match-str1 (match-string 1 str)) + (match-str2 (match-string 2 str))) + (if match-str1 + (progn + (push match-str1 result) + (setq pos (1+ (match-end 1)))) + (push match-str2 result) + (setq pos (match-end 2))))) + (nreverse result))) + +(defun org-babel--tangle-split (raw-tangle) + "Split a :tangle headerby filename and sync action." + (let* ((valid-sync-actions '("import" "export" "sync")) + (file-action (org-babel--split-str-quoted raw-tangle)) + (file (car file-action)) + (action (nth (1- (length file-action)) file-action))) + (if (member action valid-sync-actions) + ;; If last word matches, assume the previous are all part of + ;; the filename + (setq file (mapconcat #'identity (nreverse (cdr (nreverse file-action))) " ")) + ;; Otherwise set the default action and assume that the full + ;; string has no action + (if (or file action) + (setq action "export" + file (read-char raw-tangle)))) + (if (or file action) + (list file action) + (list nil)))) + (defun org-babel-process-params (params) "Expand variables in PARAMS and add summary parameters." (let* ((processed-vars (mapcar (lambda (el) @@ -1775,7 +1811,13 @@ HEADER-ARGUMENTS is alist of all the arguments." raw-result ;; FIXME: Arbitrary code evaluation. (eval raw-result t))) - (cdr (assq :result-params params)))))) + (cdr (assq :result-params params))))) + (raw-tangle (or (cdr (assq :tangle params)) "")) + (tangle-params (delete-dups + (append + (org-babel--tangle-split raw-tangle) + (cdr (assq :tangle-params params))))) + ) (append (mapcar (lambda (var) (cons :var var)) (car vars-and-names)) (list @@ -1786,7 +1828,9 @@ HEADER-ARGUMENTS is alist of all the arguments." (cons :result-params result-params) (cons :result-type (cond ((member "output" result-params) 'output) ((member "value" result-params) 'value) - (t 'value)))) + (t 'value))) + (cons :tangle-params tangle-params) + ) (cl-remove-if (lambda (x) (memq (car x) '(:colname-names :rowname-names :result-params :result-type :var))) -- 2.42.1 [-- Attachment #3: Type: text/plain, Size: 98 bytes --] If you have any insight into why the tests are failing, I'd greatly appreciate it. Best, Mehmet ^ permalink raw reply related [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-11-10 9:53 ` Mehmet Tekman @ 2023-12-11 13:40 ` Ihor Radchenko 2023-12-11 14:28 ` Mehmet Tekman 0 siblings, 1 reply; 77+ messages in thread From: Ihor Radchenko @ 2023-12-11 13:40 UTC (permalink / raw) To: Mehmet Tekman; +Cc: emacs-orgmode Sorry for the late reply. Just got around to process your email. Mehmet Tekman <mtekman89@gmail.com> writes: > I made some small edits a few months ago, to reform the header arguments > so that tangle actions would be encoded in the `:tangle-params' header > and would largely leave the `:tangle' header alone… but when I tested > it, it was failing all tests for reasons I wasn't to sure about. Do you mean that tests are not running at all? If yes, it is likely because of > +(defun org-babel--tangle-split (raw-tangle) > + "Split a :tangle headerby filename and sync action." > ... > + ;; Otherwise set the default action and assume that the full > + ;; string has no action > + (if (or file action) > + (setq action "export" > + file (read-char raw-tangle)))) ^^^^^^^^^ this `read-char' tries to read a character input from keyboard. That's why you get tests not running. Maybe just (setq action "export" file raw-tangle)? -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-12-11 13:40 ` Ihor Radchenko @ 2023-12-11 14:28 ` Mehmet Tekman 2024-04-29 5:16 ` João Pedro 0 siblings, 1 reply; 77+ messages in thread From: Mehmet Tekman @ 2023-12-11 14:28 UTC (permalink / raw) To: Ihor Radchenko; +Cc: emacs-orgmode Hey, Ihor Radchenko <yantar92@posteo.net> writes: > Sorry for the late reply. Just got around to process your email. All good, I know how it is, especially the grind towards the end of the year. >> + (setq action "export" >> + file (read-char raw-tangle)))) > ^^^^^^^^^ this Brilliant, everything seems to at least be working now on this branch. I think I can springboard off this and try to put together something a bit more refined, though probably over the winter/holiday period. Cheers, Mehmet ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-12-11 14:28 ` Mehmet Tekman @ 2024-04-29 5:16 ` João Pedro 2024-04-29 7:43 ` Mehmet Tekman ` (2 more replies) 0 siblings, 3 replies; 77+ messages in thread From: João Pedro @ 2024-04-29 5:16 UTC (permalink / raw) To: Mehmet Tekman, Ihor Radchenko; +Cc: emacs-orgmode Hey Mehmet and Org! I'd like to ask if I could contribute to this. I have followed the discussion to a point, but would be more than happy to try and contribute to this so it gets merged for the next Org major version, as I've been waiting for this for quite a while now. It solves the major gripe I had with your original org-tangle-sync.el package, which is the ability to sync blocks that aren't whole files. So, what still needs to be done and where is development happening (I assumed it was on the main Org-mode git repository, but I see no branch there)? Cheers, -- João Pedro de A. Paula IT bachelors at Universidade Federal do Rio Grande do Norte (UFRN) ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2024-04-29 5:16 ` João Pedro @ 2024-04-29 7:43 ` Mehmet Tekman 2024-04-29 16:21 ` João Pedro 2024-06-23 10:48 ` Ihor Radchenko 2024-07-23 8:47 ` Ihor Radchenko 2 siblings, 1 reply; 77+ messages in thread From: Mehmet Tekman @ 2024-04-29 7:43 UTC (permalink / raw) To: João Pedro, Ihor Radchenko; +Cc: emacs-orgmode Hello! João Pedro <jpedrodeamorim@gmail.com> writes: > I'd like to ask if I could contribute to this. I have followed the > discussion to a point, but would be more than happy to try and > contribute to this so it gets merged for the next Org major version, That's the plan, and I'll happily take any help I can get. > It solves the major gripe I had with your original org-tangle-sync.el > package, which is the ability to sync blocks that aren't whole files. Yep, and the fix for that was really quite trivial. If I knew what I know now about org source blocks, I would have rewritten that entire library very differently. > So, what still needs to be done and where is development happening (I > assumed it was on the main Org-mode git repository, but I see no branch > there)? The development is happening chaotically on my own repo (gitlab.com/mtekman/org-mode) across several incomplete branches. I've actually been working on it on and off for the last two weeks, but I've been facing some rebasing issues that I need to tackle first before I actually push anything to a branch that anyone can work on. Can you wait until Sunday for me to resolve this, and then we can discuss? Best, Mehmet ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2024-04-29 7:43 ` Mehmet Tekman @ 2024-04-29 16:21 ` João Pedro 2024-05-05 16:47 ` Mehmet Tekman 0 siblings, 1 reply; 77+ messages in thread From: João Pedro @ 2024-04-29 16:21 UTC (permalink / raw) To: Mehmet Tekman, Ihor Radchenko; +Cc: emacs-orgmode Em segunda, 29/04/2024 às 09:43 (+02), Mehmet Tekman <mtekman89@gmail.com> escreveu: > The development is happening chaotically on my own repo > (gitlab.com/mtekman/org-mode) across several incomplete branches. > > I've actually been working on it on and off for the last two weeks, but > I've been facing some rebasing issues that I need to tackle first before > I actually push anything to a branch that anyone can work on. > > Can you wait until Sunday for me to resolve this, and then we can > discuss? Yeah, of course! Cheers, -- João Pedro de A. Paula IT bachelors at Universidade Federal do Rio Grande do Norte (UFRN) ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2024-04-29 16:21 ` João Pedro @ 2024-05-05 16:47 ` Mehmet Tekman 2024-05-06 1:56 ` João Pedro 2024-05-06 12:45 ` Ihor Radchenko 0 siblings, 2 replies; 77+ messages in thread From: Mehmet Tekman @ 2024-05-05 16:47 UTC (permalink / raw) To: João Pedro, Ihor Radchenko; +Cc: emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 9204 bytes --] Hello João and org! Mehmet Tekman <mtekman89@gmail.com> on 29/04/2024 wrote: >> Can you wait until Sunday for me to resolve this, and then we can >> discuss? Okay, I've cleaned up my branches and rebased where I could, and now I think my code is at a place where you can jump in. This will be a long email summarizing what's been done so far, where the pitfalls were, where we are now, and what needs to be done: So, my code is hosted at =https://gitlab.com/mtekman/org-mode= and with the exception of the =header-reform= branch, is about 1 year behind upstream. * Ancient History Here we detail what some of ideas and pitfalls were. The main takeaway message is that we want to keep all sync actions everything within ":tangle", and we want to unify mutual exclusivity and hierarchical merging with ":results". You're probably already familiar with all this, but if not, here goes: *** Initial PR: `:tangle-sync' adds a sync parameter This was the initial PR that implemented two-way syncing via a new :tangle-sync header parameter. The work was performed in the =ob-tangle-sync-2023= branch, specifically the commit range: =34727abb6...2f0f54d68=, and specifically localized mainly in the file =ob-tangle-sync.el=. This :tangle-sync parameter described in what direction a block should be synced: either "export", "import", "skip", or "sync". e.g.1 :tangle filename :tangle-sync action #+begin_src bash :tangle export_this_file.bash :tangle-sync export echo file and sync-action as seperate header args #+end_src That git branch also reworked the detangle framework in the =ob-tangle.el= file, allowing it to detangle on a more block basis than it was doing before. *** New effort: Rework the existing `:tangle' to take filename and sync parameter As the code developed and started impacting more of =ob-tangle.el= code base, it was suggested that instead of creating an entirely new header argument (which would add to the complexity of the org code base), why shouldn't we simply compound the sync command to the existing :tangle header? e.g.2 :tangle filename sync-action #+begin_src bash :tangle export_this_file.bash export echo file and sync-action under the same banner #+end_src with the implication that the last word in the :tangle string would be a sync keyword. This now meant that instead of working on the =ob-tangle-sync.el= and =ob.tangle.el= files, that we would also need to start affecting the =ob-core.el= code. This is what commits =4b9d05d8c...HEAD= in the =ob-tangle-sync-2023= branch attempted to do, by making use of the mutual-exclusive keyword framework already present for :results #+begin_src elisp ;; see: org-babel-common-header-args-w-values '(results . ((file list vector table scalar verbatim) (raw html latex org code pp drawer link graphics) (replace silent none discard append prepend) (output value))) ;; (tangle . ((tangle yes no :any) (import export skip sync))) ;; added #+end_src This way someone can specify mutually exclusive values in headers such as: e.g.3 #+begin_src bash :results file raw replace output echo multiple compounding results values which are all valid #+end_src or, hopefully, in the case for :tangle, something like: e.g.4: #+begin_example :tangle yes sync ;; filename inherited :tangle no export ;; nothing should happen :tangle my_custom_file.sh import ;; imports specifically from that file #+end_example *** New problem: `:results' and `:tangle' mutually exclusive groups are not alike You may notice that the `:results' mutually exclusive values and the `:tangle' mutually exclusive values differ on one problematic value: ":any" #+begin_src elisp (tangle . ((tangle yes no :any) (import export skip sync))) #+end_src This `:any' parameter means that the first argument of the tangle header could be of any arbitrary length and take on any value… including those specific values that we need as the sync-action in the second argument. Oh dear! e.g.5: #+begin_example :tangle filename.sh export ;; easily resolvable :tangle import import ;; uh... we import? and assume the filename comes up the hirarchy :tangle import export ;; uh... we assume they are both sync-actions but only last is valid? #+end_example And this gets even worse if we have to consider filenames with spaces: e.g.6: #+begin_example :tangle filename with spaces.sh ;; filename only, inherit sync action from elsewhere :tangle filename with spaces sync ;; Um. We sync, and assume the filename is "filename with spaces"? #+end_example *** Solution: Custom splitting tangle function Okay, so the problems shown in examples 5 and 6 can be fixed quite easily - you just split the whole tangle header by spaces, and test if the last word is a valid sync-action or not. This was implemented in the splitting function =org-babel--handle-tangle-args= which specifically handles that header. But once again, we see the code base splitting into custom header-keyword-specific solutions instead of making one coherent unified approach for handling mutually exclusive arguments. That is, ideally, :results and :tangle should be handled by the same methods, especially in regards to: 1. Processing parameters `org-babel-process-params' evaluates header values (I think?), and in the case of :results, also splits it into parameters, namely a somewhat hidden parameter named `:result-params'. This has the benefit that the `:results' header arg is correctly split into its component mutually exclusive groups (stored in `:result-params'), but that the initial `:results' raw string is still kept and widely processed for the majority of cases where the value of this parameter is not complex. 2. Merging parameters `org-babel-merge-params' merges parameters with the same header keyword across hierarchies, so that the final block knows what its final parameters are. e.g.7: #+begin_src org ,#+PROPERTY: header-args :tangle /tmp/default_tangle.txt ,#+begin_src conf :tangle import Import text ,#+end_src #+end_src The tangle parameters at the top-level PROPERTY are merged down into the source code block, giving a final :tangle result of: #+begin_src elisp (merge ":tangle /tmp/default_tangle.txt export" ;; the export sync-action is implied ":tangle nil import") ;; the nil filename is somehow inferred #+end_src which should eventually result in final parameters ":tangle /tmp/default_tangle.txt import" for that block. *** Unified Merge function I have several (unimaginatively named) branches like merge-params-first, merge-params-second, ..., merge-params-fif that attempted to implement a unified merge strategy for the `:tangle filename sync-action' idea. The latest (read: cleanest branch) is the `merge-params-fif' branch with diff =1866c0825= implementing a new =org-babel--merge-headers= function which tries to act as a unified approach to merging :tangle :results and :export headers. It worked (to some degree), but it tripped up on issue of having "filenames with spaces", so I think this might be dead code at this point, especially since we pivoted towards `:tangle-params' being a solution for resolving sync actions. (Essentially, you can ignore any branch prefixed with "merge-params-*") * Where we are now The branch I have most recently worked on (I rebased it earlier this week) is called =header-reform=. It splits a :tangle header such as this =:tangle filename with space.txt export= into =:tangle-params ("filename with space.txt" "export")= such that it does not interfere with the rest of org headers. This is where I would greatly need you help, João. The work still to be done from my perspective: 1. Implement tests <<- we are here We need to know what the desired behaviour of :tangle and :tangle-params will be in different contexts. I have written 12 test cases as a start, but some contexts are still unclear to me and I cannot do this alone. Please see the second and third patches attached to this email. 2. Unified merge function Something that handles tangle-params and result-params, as well something that can handle mutually exclusive groups such as tangle and results, especially when the :any keyword is involved. 3. Rebase/Edit the tangle-sync changes from the =ob-tangle-sync-2023= branch Finally add the two way syncing functionality for tangled blocks. I've attached the two working patches from the =header-reform= branch to this mail, and one floating WIP inline patch which should get things started. Apologies for the length, I just wanted to summarize the efforts and show where you could come in. @Ihor please feel free to clarify anything I've written that sounds not quite right. (And thanks to all for the infinite patience!) [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-lisp-ob-core.el-org-babel-split-str-quoted-org-babel.patch --] [-- Type: text/x-patch, Size: 6037 bytes --] From 7a0b71fc519ec20b68ae75985085530750ad6e17 Mon Sep 17 00:00:00 2001 From: Mehmet Tekman <mtekman89@gmail.com> Date: Wed, 20 Sep 2023 11:35:00 +0200 Subject: [PATCH 1/3] * lisp/ob-core.el (org-babel--split-str-quoted org-babel--tangle-split org-babel-process-params): functions to assist splitting a :tangle header containing a filename with spaces as well as a sync-action into two parameter components of strictly filename and sync-action for use with :tangle-params. * lisp/ob-exp.el (org-babel-exp-code): fix to function to not include :tangle-params as as real header * testing/lisp/test-ob.el (test-ob/process-params-no-duplicates): fix to include :tangle-params as part of process-params test --- lisp/ob-core.el | 65 +++++++++++++++++++++++++++++++++++++---- lisp/ob-exp.el | 2 +- testing/lisp/test-ob.el | 2 ++ 3 files changed, 63 insertions(+), 6 deletions(-) diff --git a/lisp/ob-core.el b/lisp/ob-core.el index 73fb70c26..019f3d88d 100644 --- a/lisp/ob-core.el +++ b/lisp/ob-core.el @@ -1785,6 +1785,52 @@ HEADER-ARGUMENTS is alist of all the arguments." header-arguments) (nreverse results))) + +(defun org-babel--split-str-quoted (str) + "Splits a string that may or may not contain quotes." + (let (result pos) + (while (string-match (rx (or (seq "\"" (group (* (not "\""))) "\"") + (group (+ (not space))))) + str pos) + (let ((match-str1 (match-string 1 str)) + (match-str2 (match-string 2 str))) + (if match-str1 + (progn + (push match-str1 result) + (setq pos (1+ (match-end 1)))) + (push match-str2 result) + (setq pos (match-end 2))))) + (nreverse result))) + +(defun org-babel--tangle-split (raw-tangle) + "Split a :tangle header into two: filename of multiple words, and +sync action. The result does not take into account any merged properties. + +If the filename is actually a tangle keyword such as 'no', then specify no sync action. + +Actions can be + :any (specific filename) import/export/sync + yes (inherited filename) import/export/sync + no (will not export) (no sync action)" + (let* ((valid-sync-actions '("import" "export" "sync")) + (file-action (org-babel--split-str-quoted raw-tangle)) + (file (car file-action)) + (sync-action (nth (1- (length file-action)) file-action))) + (if (member sync-action valid-sync-actions) + ;; If last word matches, assume the previous are all part of + ;; the filename + (setq file (mapconcat #'identity (nreverse (cdr (nreverse file-action))) " ")) + + ;; Otherwise a sync action was not given, and we default to normal tangle keywords + ;; such as a filename (assumes export), yes (assumes export), no (no sync-action) + (if (string-equal sync-action "no") + (setq sync-action nil) + ;; No sync action at all given? Assume the default: export + (setq sync-action "export" + file raw-tangle))) + (list file sync-action))) + + (defun org-babel-process-params (params) "Expand variables in PARAMS and add summary parameters." (let* ((processed-vars (mapcar (lambda (el) @@ -1807,7 +1853,13 @@ HEADER-ARGUMENTS is alist of all the arguments." raw-result ;; FIXME: Arbitrary code evaluation. (eval raw-result t))) - (cdr (assq :result-params params)))))) + (cdr (assq :result-params params))))) + (raw-tangle (cdr (assq :tangle params))) + (tangle-params (if raw-tangle + (delete-dups + (append + (org-babel--tangle-split raw-tangle) + (cdr (assq :tangle-params params))))))) (append (mapcar (lambda (var) (cons :var var)) (car vars-and-names)) (list @@ -1815,14 +1867,17 @@ HEADER-ARGUMENTS is alist of all the arguments." (cadr vars-and-names))) (cons :rowname-names (or (cdr (assq :rowname-names params)) (cl-caddr vars-and-names))) + (cons :tangle-params tangle-params) ;; always a list of two: tangle-keyword sync-action (cons :result-params result-params) (cons :result-type (cond ((member "output" result-params) 'output) ((member "value" result-params) 'value) - (t 'value)))) + (t 'value))) + ) (cl-remove-if - (lambda (x) (memq (car x) '(:colname-names :rowname-names :result-params - :result-type :var))) - params)))) + (lambda (x) (memq (car x) '(:colname-names :rowname-names :tangle-params + :result-params :result-type + :var))) + params)))) ;; row and column names (defun org-babel-del-hlines (table) diff --git a/lisp/ob-exp.el b/lisp/ob-exp.el index 80eaeeb27..05ac01c39 100644 --- a/lisp/ob-exp.el +++ b/lisp/ob-exp.el @@ -448,7 +448,7 @@ replaced with its value." ;; Special parameters that are not real header ;; arguments. (memq (car pair) - '( :result-params :result-type + '( :result-params :result-type :tangle-params ;; This is an obsolete parameter still ;; used in some tests. :flags)) diff --git a/testing/lisp/test-ob.el b/testing/lisp/test-ob.el index c088af7c8..e0de5a3ad 100644 --- a/testing/lisp/test-ob.el +++ b/testing/lisp/test-ob.el @@ -2131,10 +2131,12 @@ default-directory (:rowname-names) (:result-params) (:result-type) + (:tangle-params) (:var . "\"foo\""))) '((:var) (:colname-names) (:rowname-names) + (:tangle-params) (:result-params) (:result-type . value))))) -- 2.41.0 [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #3: 0002-testing-lisp-test-ob.el-New-tests-for-merge-params.patch --] [-- Type: text/x-patch, Size: 6263 bytes --] From 22cf61fcaf75d4fa3b65c71f05d2ea8d71adaca5 Mon Sep 17 00:00:00 2001 From: Mehmet Tekman <mtekman89@gmail.com> Date: Fri, 3 May 2024 15:11:38 +0200 Subject: [PATCH 2/3] * testing/lisp/test-ob.el: New tests for merge-params (test-ob/get-src-block-property): (test-ob/merge-params): add new tests for the merge-params source block header handling function. --- testing/lisp/test-ob.el | 143 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 143 insertions(+) diff --git a/testing/lisp/test-ob.el b/testing/lisp/test-ob.el index e0de5a3ad..604f5ac92 100644 --- a/testing/lisp/test-ob.el +++ b/testing/lisp/test-ob.el @@ -314,6 +314,149 @@ this is simple" (org-babel-next-src-block) (should (= 14 (org-babel-execute-src-block))))) +(defun test-ob/get-src-block-property (properties) + "Get plist of PROPERTIES and values for the first src block in buffer. +PROPERTIES is a list of property keywords or a single keyword." + (org-with-wide-buffer + (goto-char (point-min)) + (org-babel-next-src-block) + (org-set-regexps-and-options) + (let ((all-props (nth 2 (org-babel-get-src-block-info)))) + (if (listp properties) + (apply #'nconc (mapcar (lambda (p) (list p (cdr (assoc p all-props)))) properties)) + (list properties (cdr (assoc properties all-props))))))) + +(ert-deftest test-ob/merge-params () + "Test the output of merging multiple header parameters. The +expected output is given in the contents of the source code block +in each test. The desired test header parameters are given +either as a symbol or a list in the `idtest-alist' variable. +Multiple header parameters must be separated by a newline and +exactly two spaces in the block contents." + (should ;; 1. inherit-document-header-args + (equal '(:tangle "/tmp/default_tangle.txt") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* One +#+begin_src conf +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should-not ;; 2. inherit-document-header-with-local-sync-action + ;; This should pass with newer merge function with multiple tangle parameters + (equal '(:tangle "/tmp/default_tangle.txt skip") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Two +#+begin_src conf :tangle skip +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 3. override-document-header-with-local-tfile + (equal '(:tangle "randomfile sync") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Three +#+begin_src conf :tangle randomfile sync +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 4. override-document-and-parent-header-with-local-tfile-and-action + (equal '(:tangle "randomfile sync") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Four +:PROPERTIES: +:header-args: :tangle \"newfile.txt\" import +:END: +** A +#+begin_src conf :tangle randomfile sync +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 5. test-tangle-and-default-results-param-together + (equal '(:tangle "randomfile" :results "replace") + (org-test-with-temp-text + "\ +* Five +#+begin_src conf :tangle randomfile +#+end_src" + (test-ob/get-src-block-property '(:tangle :results))))) + (should-not ;; 6. inherit-document-tfile-take-only-last-local-sync-action + ;; This should pass with newer merge function with multiple tangle parameters + (equal '(:tangle "/tmp/default_tangle.txt export") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Six +#+begin_src conf :tangle import export +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should-not ;; 7. ignore-document-header-take-last-tfile-and-sync-action + ;; This should pass with newer merge function with multiple tangle parameters + (equal '(:tangle "fname2 export") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Seven +#+begin_src conf :tangle fname1 fname2 sync export +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 8. test-results-and-exports + (equal '(:results "wrap file replace" :exports "code") + (org-test-with-temp-text + "\ +* Eight +#+begin_src sh :results file wrap +#+end_src" + (test-ob/get-src-block-property '(:results :exports))))) + (should ;; 9. do-not-tangle-this-block -- + (equal '(:tangle "no") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Nine +#+begin_src conf :tangle no +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 10. test-tangle-exports-and-comments + (equal '(:tangle "foo.txt" :exports "verbatim code" :comments "link") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Ten +:PROPERTIES: +:header-args: :tangle no :exports verbatim +:END: +#+begin_src conf :tangle \"foo.txt\" :comments link +#+end_src" + (test-ob/get-src-block-property '(:tangle :exports :comments))))) + (should-not ;; 11. override-document-and-heading-tfile-with-yes + ;; This should pass with newer merge function with multiple tangle parameters + (equal '(:tangle "foo.txt") + (org-test-with-temp-text + "\ +#+PROPERTY: header-args :tangle /tmp/default_tangle.txt +* Eleven +:PROPERTIES: +:header-args: :tangle \"foo.txt\" +:END: +#+begin_src conf :tangle yes +#+end_src" + (test-ob/get-src-block-property :tangle)))) + (should ;; 12. tangle-file-with-spaces + (equal '(:tangle "file with spaces.txt") + (org-test-with-temp-text + "\ +* Twelve +:PROPERTIES: +:header-args: :tangle \"foo.txt\" +:END: +** A +#+begin_src conf :tangle \"file with spaces.txt\" +#+end_src" + (test-ob/get-src-block-property :tangle))))) + (ert-deftest test-ob/inline-src-blocks () (should (= 1 -- 2.41.0 [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #4: floating patch, needs work --] [-- Type: text/x-patch, Size: 2218 bytes --] From 19d4689bb86ade87c0f6ef9338c33dee43a66cbe Mon Sep 17 00:00:00 2001 From: Mehmet Tekman <mtekman89@gmail.com> Date: Sun, 5 May 2024 17:32:59 +0200 Subject: [PATCH 3/3] Floating commit, implementing tangle-params into tests. Needs work --- testing/lisp/test-ob.el | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/testing/lisp/test-ob.el b/testing/lisp/test-ob.el index 604f5ac92..4ccb74d64 100644 --- a/testing/lisp/test-ob.el +++ b/testing/lisp/test-ob.el @@ -334,24 +334,29 @@ either as a symbol or a list in the `idtest-alist' variable. Multiple header parameters must be separated by a newline and exactly two spaces in the block contents." (should ;; 1. inherit-document-header-args - (equal '(:tangle "/tmp/default_tangle.txt") + (equal (cons '(:tangle "/tmp/default_tangle.txt") + ;;'(:tangle-params ("/tmp/default_tangle.txt" "export") ;; -- desired + '(:tangle-params ("" "export"))) (org-test-with-temp-text "\ #+PROPERTY: header-args :tangle /tmp/default_tangle.txt * One -#+begin_src conf +#+begin_src conf :tangle export #+end_src" - (test-ob/get-src-block-property :tangle)))) - (should-not ;; 2. inherit-document-header-with-local-sync-action + (cons (test-ob/get-src-block-property :tangle) + (test-ob/get-src-block-property :tangle-params))))) + (should ;; 2. inherit-document-header-with-local-sync-action ;; This should pass with newer merge function with multiple tangle parameters - (equal '(:tangle "/tmp/default_tangle.txt skip") + (equal (cons '(:tangle "no") + '(:tangle-params ("no" nil))) (org-test-with-temp-text "\ #+PROPERTY: header-args :tangle /tmp/default_tangle.txt * Two -#+begin_src conf :tangle skip +#+begin_src conf :tangle no #+end_src" - (test-ob/get-src-block-property :tangle)))) + (cons (test-ob/get-src-block-property :tangle) + (test-ob/get-src-block-property :tangle-params))))) (should ;; 3. override-document-header-with-local-tfile (equal '(:tangle "randomfile sync") (org-test-with-temp-text -- 2.41.0 [-- Attachment #5: Type: text/plain, Size: 16 bytes --] Best, Mehmet ^ permalink raw reply related [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2024-05-05 16:47 ` Mehmet Tekman @ 2024-05-06 1:56 ` João Pedro 2024-05-06 12:53 ` Ihor Radchenko 2024-05-06 16:28 ` Mehmet Tekman 2024-05-06 12:45 ` Ihor Radchenko 1 sibling, 2 replies; 77+ messages in thread From: João Pedro @ 2024-05-06 1:56 UTC (permalink / raw) To: Mehmet Tekman, Ihor Radchenko; +Cc: emacs-orgmode Em domingo, 05/05/2024 às 18:47 (+02), Mehmet Tekman <mtekman89@gmail.com> escreveu: > Hello João and org! Hi Mehmet! > This will be a long email summarizing what's been done so far, where the > pitfalls were, where we are now, and what needs to be done: Thank you for the summary. Albeit long, it was quite important to understand all of the discussions and decisions so far. I tried keeping track of every thing on this long, long thread, but couldn't follow up with everything, so it was incredibly helpful of you to give such a complete summary. > So, my code is hosted at =https://gitlab.com/mtekman/org-mode= and with > the exception of the =header-reform= branch, is about 1 year behind > upstream. So, if I understand correctly, =header-reform= is where we're at right now and after we're done with this overhauling of the parsing of (mutually exclusive) header params, we'll get back to the actual syncing logic, which should be mostly done. > * Where we are now > > The branch I have most recently worked on (I rebased it earlier this > week) is called =header-reform=. It splits a :tangle header such as > > this =:tangle filename with space.txt export= > into =:tangle-params ("filename with space.txt" "export")= > > such that it does not interfere with the rest of org headers. > > This is where I would greatly need you help, João. > > The work still to be done from my perspective: > > 1. Implement tests <<- we are here > > We need to know what the desired behaviour of :tangle and > :tangle-params will be in different contexts. > > I have written 12 test cases as a start, but some contexts are > still unclear to me and I cannot do this alone. > > Please see the second and third patches attached to this email. Ok I'll start here as I think writing tests could give me some more insights on the core problems that still need solution. > 2. Unified merge function I do have a couple of questions regarding this: > Something that handles tangle-params and result-params, 1. So the idea is to continue parsing multi-option header arguments into their separate components, but have a common place to, after parsing it into their components, handle them all in a single place? I mean, what does "handle" mean in this context? > as well > something that can handle mutually exclusive groups such as tangle > and results, especially when the :any keyword is involved. 2. Isn't that the same as the previous point? If we can handle any *-params doesn't that solve the problem of handling mutually exclusive groups? We can just define a pattern of parsing them into individual *-params and handling that in a unified manner. > I've attached the two working patches from the =header-reform= branch to > this mail, and one floating WIP inline patch which should get things > started. Are these patches also on the branch (apart from the WIP one)? > Apologies for the length, I just wanted to summarize the efforts and > show where you could come in. As I said in the beginning, I'm actually quite grateful for the summarizing! Best regards, -- João Pedro de A. Paula IT bachelors at Universidade Federal do Rio Grande do Norte (UFRN) ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2024-05-06 1:56 ` João Pedro @ 2024-05-06 12:53 ` Ihor Radchenko 2024-05-06 16:28 ` Mehmet Tekman 1 sibling, 0 replies; 77+ messages in thread From: Ihor Radchenko @ 2024-05-06 12:53 UTC (permalink / raw) To: João Pedro; +Cc: Mehmet Tekman, emacs-orgmode João Pedro <jpedrodeamorim@gmail.com> writes: >> Something that handles tangle-params and result-params, > > 1. So the idea is to continue parsing multi-option header arguments into > their separate components, but have a common place to, after parsing > it into their components, handle them all in a single place? I mean, > what does "handle" mean in this context? From my point of view, we need some API function to retrieve header argument values. Instead of the current approach with (result-params (cdr (assq :result-params params))) we need something like (org-babel-get-header-arg :results params) ; get parsed parameter (org-babel-get-header-arg :results params 'raw) ; get raw parameter Then, babel backends will not need to know the internal details about :results parameter being represented in its two forms stored as :results and as :result-params. -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2024-05-06 1:56 ` João Pedro 2024-05-06 12:53 ` Ihor Radchenko @ 2024-05-06 16:28 ` Mehmet Tekman 1 sibling, 0 replies; 77+ messages in thread From: Mehmet Tekman @ 2024-05-06 16:28 UTC (permalink / raw) To: João Pedro, Ihor Radchenko; +Cc: emacs-orgmode Hello! João Pedro <jpedrodeamorim@gmail.com> writes: > So, if I understand correctly, =header-reform= is where we're at right > now and after we're done with this overhauling of the parsing of > (mutually exclusive) header params, we'll get back to the actual syncing > logic, which should be mostly done. Exactly -- reform the headers first (branch: header-reform) and then add the sync stuff later (branch: ob-tangle-sync-2023). > Ok I'll start here as I think writing tests could give me some more > insights on the core problems that still need solution. Yeah, at least from my side I got really confused with what the expected actions of some :tangle parameters were. Let me know if you find any ambiguous cases too. > 1. So the idea is to continue parsing multi-option header arguments into > their separate components, but have a common place to, after parsing > it into their components, handle them all in a single place? I mean, > what does "handle" mean in this context? Hah, good question. I think by "handle" I mean three things: 1. Ensure that :tangle remains as the raw string :tangle so that other parts of Org can use it without needing to guess if there is a sync action in there or not. I quite like Ihor's suggestion: Ihor Radchenko <yantar92@posteo.net> writes: > we need something like > > (org-babel-get-header-arg :results params) ; get parsed parameter > (org-babel-get-header-arg :results params 'raw) ; get raw parameter > > Then, babel backends will not need to know the internal details about > :results parameter being represented in its two forms stored as :results > and as :result-params. 2. Ensure that :tangle or :tangle-params can merge both filenames and sync-actions down the hierarchy. In my last email I talk about "process-params" (splitting the filename and sync-action) and "merge-params" (resolve hierarchy) as two separate entities, where a :tangle header is first "merged" down to a header, then "merged" down to a subheading, then "merged" down to a source block, and then finally "processed"... ... but now that I think about it, it's quite likely that "merge" and "process" are interlinked, and we will need something that is aware of what the "filename sync-action" pair is, at EVERY level of the hierarchy. As in, it processes a header first, then merges it down, then processes the next header, and then merges that down, and so forth. Right now, process-params and merge-params are two independent code blocks, but I think one might need to start calling the other. 3. Do this in such a way that :results can also benefit from this process/merge strategy. > 2. Isn't that the same as the previous point? If we can handle any > *-params doesn't that solve the problem of handling mutually > exclusive groups? Yes, it's the same point :-) Creating a unified *-params handler for mutually exclusive groups would probably fix all 3 problems. > We can just define a pattern of parsing them into individual *-params > and handling that in a unified manner. If I'm understanding you correctly, do you mean we leave the normal header merging and processing alone, and we just treat :tangle-params and :result-params as their own special group, and handle them independently of :tangle and :results? If so, I'm not so sure how easy it is due to my second point above. > Are these patches also on the branch (apart from the WIP one)? Yep, those patches as well as the WIP one are also on that branch. My coding strategy right now is to develop chaotically on =header-reform= and then do cherry picking/rebasing afterwards. If you want I can give you push privileges to that branch, or you can clone it and work in your own tidier manner, or do you prefer working with email patches? Best, Mehmet ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2024-05-05 16:47 ` Mehmet Tekman 2024-05-06 1:56 ` João Pedro @ 2024-05-06 12:45 ` Ihor Radchenko 1 sibling, 0 replies; 77+ messages in thread From: Ihor Radchenko @ 2024-05-06 12:45 UTC (permalink / raw) To: Mehmet Tekman; +Cc: João Pedro, emacs-orgmode Mehmet Tekman <mtekman89@gmail.com> writes: > Okay, I've cleaned up my branches and rebased where I could, and now I > think my code is at a place where you can jump in. > > This will be a long email summarizing what's been done so far, where the > pitfalls were, where we are now, and what needs to be done: > ... Thanks a lot for taking a time to write this summary! -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2024-04-29 5:16 ` João Pedro 2024-04-29 7:43 ` Mehmet Tekman @ 2024-06-23 10:48 ` Ihor Radchenko 2024-07-23 8:47 ` Ihor Radchenko 2 siblings, 0 replies; 77+ messages in thread From: Ihor Radchenko @ 2024-06-23 10:48 UTC (permalink / raw) To: João Pedro; +Cc: Mehmet Tekman, emacs-orgmode João Pedro <jpedrodeamorim@gmail.com> writes: > I'd like to ask if I could contribute to this. I have followed the > discussion to a point, but would be more than happy to try and > contribute to this so it gets merged for the next Org major version, as > I've been waiting for this for quite a while now... It has been over a month since the last email in this thread. May I know if you need any further help from us? -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2024-04-29 5:16 ` João Pedro 2024-04-29 7:43 ` Mehmet Tekman 2024-06-23 10:48 ` Ihor Radchenko @ 2024-07-23 8:47 ` Ihor Radchenko 2 siblings, 0 replies; 77+ messages in thread From: Ihor Radchenko @ 2024-07-23 8:47 UTC (permalink / raw) To: João Pedro; +Cc: Mehmet Tekman, emacs-orgmode João Pedro <jpedrodeamorim@gmail.com> writes: > I'd like to ask if I could contribute to this. I have followed the > discussion to a point, but would be more than happy to try and > contribute to this so it gets merged for the next Org major version, as > I've been waiting for this for quite a while now. It solves the major > gripe I had with your original org-tangle-sync.el package, which is the > ability to sync blocks that aren't whole files. It has been a while since the last message in this thread. João, may I know if you are still interested to work on this patch? -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-04-26 14:48 [ANN] lisp/ob-tangle-sync.el Mehmet Tekman 2023-04-26 16:43 ` John Wiegley 2023-04-27 2:55 ` Ruijie Yu via General discussions about Org-mode. @ 2023-04-27 12:02 ` Ihor Radchenko 2023-04-27 13:01 ` Mehmet Tekman 2 siblings, 1 reply; 77+ messages in thread From: Ihor Radchenko @ 2023-04-27 12:02 UTC (permalink / raw) To: Mehmet Tekman; +Cc: emacs-orgmode Mehmet Tekman <mtekman89@gmail.com> writes: > I would like to contribute some a new library into org-mode, which > performs automatic synchronization between tangled files and their > org-mode source blocks via a global minor mode > `org-babel-tangle-sync-mode' which uses the after-save-hook. Thanks for your interest in contributing to Org mode! > By activating =org-babel-tangle-sync-mode=, I can edit either of those > buffers and every time that I save it would automatically update the > other. If I add the header argument =:tangle-sync <action>= then I > can specify an action of: > - skip :: do nothing, just save the buffer, even if the sync mode is active > - pull :: only pull changes from =~/.bashrc= into the Emacs_conf org-mode file > - export :: only export changes from Emacs_conf to =~/.bashrc=, even if called > from =~/.bashrc= > - both :: (default) synchronize freely between tangled file and source block > (the is also the nil value) Do I understand correctly that your package is adding the following new features: 1. Automatically manages balanced `org-babel-tangle'/`org-babel-detangle' in Org sources and target buffers. 2. Attempts to make more fine-grained tangling/detanging functionality by controlling tangling behaviour on per-src-block basis (via :tangle-sync skip/pull/export/both) If so, I think that (2) probably belongs to the main babel code (`org-babel-tangle' and `org-babel-detangle'). Since you are contributing to Org directly, you are free to modify these functions as you need. Note that your current approach with `org-babel-tangle-sync-synchronize' will give unexpected results when one edits multiple src blocks or the corresponding tangled source in multiple places - the :tangle-sync value is only checked at point and `org-babel-tangle'/`org-babel-detange' is then called for the whole buffer, not just for the current code block. -- Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92> ^ permalink raw reply [flat|nested] 77+ messages in thread
* Re: [ANN] lisp/ob-tangle-sync.el 2023-04-27 12:02 ` Ihor Radchenko @ 2023-04-27 13:01 ` Mehmet Tekman 0 siblings, 0 replies; 77+ messages in thread From: Mehmet Tekman @ 2023-04-27 13:01 UTC (permalink / raw) To: emacs-orgmode [-- Attachment #1: Type: text/plain, Size: 1637 bytes --] Ihor Radchenko <yantar92@posteo.net> writes: > Do I understand correctly that your package is adding the following new > features: > > 1. Automatically manages balanced > `org-babel-tangle'/`org-babel-detangle' in Org sources and target > buffers. > 2. Attempts to make more fine-grained tangling/detanging functionality > by controlling tangling behaviour on per-src-block basis (via > :tangle-sync skip/pull/export/both) > That is correct > If so, I think that (2) probably belongs to the main babel code > (`org-babel-tangle' and `org-babel-detangle'). Since you are > contributing to Org directly, you are free to modify these functions as > you need. > > Note that your current approach with `org-babel-tangle-sync-synchronize' > will give unexpected results when one edits multiple src blocks or the > corresponding tangled source in multiple places - the :tangle-sync value > is only checked at point and `org-babel-tangle'/`org-babel-detange' is > then called for the whole buffer, not just for the current code block. That's... a really good point. I didn't consider that I'm calling a tangle or detangle function on an entire file based on the =:tangle-sync action= of a single block... which might have different actions in another block for the same file! I've attached a toy.org file which should explain what the desired action would be in such a situation, and I fully agree that I will likely need to modify the current tangle and detangle code so that the sync direction is on a per-block basis, and not per-file. I will work on this a bit more and submit a full patch with this in mind. Best, Mehmet [-- Attachment #2: toy.org --] [-- Type: text/org, Size: 1561 bytes --] #+TITLE: Sync Many Blocks #+PROPERTY: header-args :tangle /tmp/default_tangle.txt :comments yes * Default Tangle The =:tangle= target here is given by the ~PROPERTY~ header. It is made up of three parts which synchronize with the tangled destination file in 3 different ways as given below: ** File Contents *** Free Head #+begin_src conf This is some text that can be synced both ways #+end_src *** Pull Middle #+begin_src conf :tangle-sync import The middle text only retrieves from the default tangle file #+end_src *** Push tail #+begin_src conf :tangle-sync export This last piece of text only exports from the source org mode file. #+end_src * Desired Function When =org-babel-tangle= is called, only the [[Free Head]] and [[Push tail]] sections are tangled. The middle section in the tangled file is _skipped_. When =org-babel-detangle= is called, only the [[Pull Middle]] section is updated here. The other two sections are _skipped_. * Why skipped? The functions suffixes explicitly state a direction (=tangle= / =detangle=), and it's probably important that we retain this directionality. If we want to synchronize all blocks (i.e. import the middle section from the tangled file during =org-babel-tangle=, and export the head and tail sections during =org-babel-detangle=), then perhaps we set a specific custom variable =(setq org-babel-tangle-alwayssync t)=? Or, we create a specific function =org-babel-sync= (for lack of better name) which tangles/detangles in whatever desired direction the source block specifies. ^ permalink raw reply [flat|nested] 77+ messages in thread
end of thread, other threads:[~2024-07-23 8:47 UTC | newest] Thread overview: 77+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-04-26 14:48 [ANN] lisp/ob-tangle-sync.el Mehmet Tekman 2023-04-26 16:43 ` John Wiegley 2023-04-26 18:43 ` Mehmet Tekman 2023-04-27 2:55 ` Ruijie Yu via General discussions about Org-mode. 2023-04-27 6:27 ` Mehmet Tekman 2023-04-28 10:57 ` Ruijie Yu via General discussions about Org-mode. 2023-04-28 11:28 ` Mehmet Tekman 2023-05-02 20:43 ` Mehmet Tekman 2023-05-03 2:31 ` Ruijie Yu via General discussions about Org-mode. 2023-05-03 7:53 ` Mehmet Tekman 2023-05-03 8:34 ` Mehmet Tekman 2023-05-03 8:44 ` Ihor Radchenko 2023-05-03 11:43 ` Ihor Radchenko 2023-05-03 13:54 ` Mehmet Tekman 2023-05-03 18:06 ` Ihor Radchenko 2023-05-03 15:05 ` Mehmet Tekman 2023-05-03 15:21 ` Ihor Radchenko [not found] ` <87lei577g4.fsf@gmail.com> [not found] ` <87lei5v1fg.fsf@localhost> [not found] ` <87fs8duyae.fsf@localhost> 2023-05-09 14:03 ` Mehmet Tekman 2023-05-10 9:46 ` Ihor Radchenko 2023-05-10 11:06 ` mtekman89 2023-05-10 11:32 ` Ihor Radchenko 2023-05-10 16:20 ` Mehmet Tekman 2023-05-12 12:33 ` Ihor Radchenko 2023-05-16 12:49 ` Mehmet Tekman 2023-05-16 18:57 ` Ihor Radchenko 2023-05-17 13:45 ` Mehmet Tekman 2023-05-18 10:30 ` Ihor Radchenko 2023-05-19 7:10 ` Mehmet Tekman 2023-07-15 12:38 ` Ihor Radchenko 2023-07-16 9:42 ` Mehmet Tekman 2023-07-17 11:29 ` Mehmet Tekman 2023-07-18 8:47 ` Ihor Radchenko 2023-07-21 8:48 ` Mehmet Tekman 2023-07-22 8:02 ` Ihor Radchenko 2023-07-25 11:19 ` Mehmet Tekman 2023-07-25 16:19 ` Ihor Radchenko 2023-07-31 13:41 ` Mehmet Tekman 2023-07-31 16:38 ` Ihor Radchenko 2023-07-31 20:11 ` Mehmet Tekman 2023-08-01 7:54 ` Ihor Radchenko 2023-08-01 8:49 ` Mehmet Tekman 2023-08-01 9:30 ` Ihor Radchenko 2023-08-01 18:19 ` Bastien Guerry 2023-08-02 7:29 ` Ihor Radchenko 2023-08-02 14:46 ` Mehmet Tekman 2023-08-03 6:32 ` Mehmet Tekman 2023-08-03 7:35 ` Ihor Radchenko 2023-08-03 8:08 ` Mehmet Tekman 2023-08-03 8:16 ` Ihor Radchenko [not found] ` <CAHHeYzL6Z5_gGbTUrNzKDh5swgCSQiYsSj3Cs0gFy_d=eXbSBA@mail.gmail.com> [not found] ` <87o7jo1q2s.fsf@localhost> 2023-08-03 8:46 ` Mehmet Tekman 2023-08-04 7:41 ` Mehmet Tekman 2023-08-04 8:09 ` Ihor Radchenko 2023-08-04 13:14 ` Mehmet Tekman 2023-08-04 16:34 ` Mehmet Tekman 2023-08-06 9:07 ` Ihor Radchenko 2023-08-08 19:41 ` Mehmet Tekman 2023-08-08 19:51 ` Ihor Radchenko 2023-08-08 20:04 ` Mehmet Tekman 2023-08-09 8:04 ` Ihor Radchenko 2023-08-05 8:48 ` Ihor Radchenko 2023-08-05 22:54 ` Mehmet Tekman 2023-11-10 9:41 ` Ihor Radchenko 2023-11-10 9:53 ` Mehmet Tekman 2023-12-11 13:40 ` Ihor Radchenko 2023-12-11 14:28 ` Mehmet Tekman 2024-04-29 5:16 ` João Pedro 2024-04-29 7:43 ` Mehmet Tekman 2024-04-29 16:21 ` João Pedro 2024-05-05 16:47 ` Mehmet Tekman 2024-05-06 1:56 ` João Pedro 2024-05-06 12:53 ` Ihor Radchenko 2024-05-06 16:28 ` Mehmet Tekman 2024-05-06 12:45 ` Ihor Radchenko 2024-06-23 10:48 ` Ihor Radchenko 2024-07-23 8:47 ` Ihor Radchenko 2023-04-27 12:02 ` Ihor Radchenko 2023-04-27 13:01 ` Mehmet Tekman
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).