[PATCH] Async sessions: Fix prompt removal regression in ob-R
Consider the following R block, which prints the occurrences of each element in a list, including NAs: #+begin_src R :session :results output :async table(c("ab","ab","c",NA,NA), useNA='always') #+end_src #+RESULTS: : abc :212 Since Org 9.7, it instead prints: #+RESULTS: : abc < : 212 The regression happens in commit: e9c288dfaccc2960e5b6889e6aabea700ad4e05a which made the prompt filtering more consistent between `org-babel-comint-with-output' and `org-babel-comint-async-filter'. However, it causes ob-R async session blocks to be over-aggressive in removing the prompt. Note that non-async ob-R blocks don't suffer from this problem, because ob-R let-binds `comint-prompt-regexp' around `org-babel-comint-with-output' (specifically, it adds a beginning-of-line at the front of the regexp). However, I don't see a good way to let-bind this around `org-babel-comint-async-filter'. The best solution I could think of was to define a new buffer-local variable, `org-babel-comint-prompt-regexp-override', which could be used to override `comint-prompt-regexp' for the purpose of filtering. I attach a patch with this solution. Additionally, the regression causes causes misalignment of the output due to removal of indentation. The fix for this is simpler, and involves replacing a call of `org-trim' with `org-babel-chomp'. I'm not sure if my patch is the best solution. But whatever solution we arrive at, I would like to request that it be applied to bugfix branch. >From 11177e57f8a0c77b6c6541b852c5d105d70afec0 Mon Sep 17 00:00:00 2001 From: Jack Kamm Date: Sun, 22 Sep 2024 13:48:45 -0700 Subject: [PATCH] ob-R: Fix over-aggressive async prompt removal * lisp/ob-comint.el (org-babel-comint-prompt-regexp-override): New variable to override `comint-prompt-regexp' in `org-babel-comint--prompt-filter'. (org-babel-comint-async-filter): Replace `org-trim' with `org-babel-chomp' to avoid removing leading indentation. * lisp/ob-R.el (org-babel-R-evaluate): Set `org-babel-comint-regexp-override' in session evaluation. (org-babel-R-evaluate-session): Remove let binding of `comint-prompt-regexp', since `org-babel-comint-regexp-override' is now set. * testing/lisp/test-ob-R.el (test-ob-R/async-prompt-filter): Test for over-aggressive prompt removal. --- lisp/ob-R.el | 25 ++--- lisp/ob-comint.el | 18 +++--- testing/lisp/test-ob-R.el | 28 3 files changed, 57 insertions(+), 14 deletions(-) diff --git a/lisp/ob-R.el b/lisp/ob-R.el index de2d27a9a..a9a58d0e4 100644 --- a/lisp/ob-R.el +++ b/lisp/ob-R.el @@ -375,11 +375,15 @@ (defun org-babel-R-evaluate (session body result-type result-params column-names-p row-names-p async) "Evaluate R code in BODY." (if session - (if async - (ob-session-async-org-babel-R-evaluate-session - session body result-type column-names-p row-names-p) -(org-babel-R-evaluate-session - session body result-type result-params column-names-p row-names-p)) + (progn +(with-current-buffer session + (setq org-babel-comint-prompt-regexp-override +(concat "^" comint-prompt-regexp))) +(if async +(ob-session-async-org-babel-R-evaluate-session + session body result-type column-names-p row-names-p) + (org-babel-R-evaluate-session + session body result-type result-params column-names-p row-names-p))) (org-babel-R-evaluate-external-process body result-type result-params column-names-p row-names-p))) @@ -456,12 +460,11 @@ (defun org-babel-R-evaluate-session (substring line (match-end 1)) line)) (with-current-buffer session - (let ((comint-prompt-regexp (concat "^" comint-prompt-regexp))) - (org-babel-comint-with-output (session org-babel-R-eoe-output) - (insert (mapconcat 'org-babel-chomp - (list body org-babel-R-eoe-indicator) - "\n")) - (inferior-ess-send-input "\n" + (org-babel-comint-with-output (session org-babel-R-eoe-output) + (insert (mapconcat 'org-babel-chomp + (list body org-babel-R-eoe-indicator) + "\n")) + (inferior-ess-send-input))) "\n" (defun org-babel-R-process-value-result (result column-names-p) "R-specific processing of return value. diff --git a/lisp/ob-comint.el b/lisp/ob-comint.el index 764927af7..7f1686035 100644 --- a/lisp/ob-comint.el +++ b/lisp/ob-comint.el @@ -75,11 +75,17 @@ (defun org-babel-comint--set-fallback-prompt () (setq comint-prompt-regexp org-babel-comint-prompt-regexp-old org-babel-comint-prompt-regexp-old tmp +(defvar-local org-babel-comint-prompt-regexp-overr
Re: [PATCH] Fix ox-icalendar export of diary timestamps
Ihor Radchenko writes: > I agree that it makes sense. > However, it is technically a breaking change. > May you please add a news entry as well? Thanks, I've updated the patch with a news entry now. >From 5c40741664402a5984803dc3de452ea949885887 Mon Sep 17 00:00:00 2001 From: Jack Kamm Date: Sat, 14 Sep 2024 22:48:44 -0700 Subject: [PATCH] ox-icalendar: Fix export of diary-style timestamps * lisp/ox-icalendar.el (org-icalendar-entry): Include timestamps of type diary when `:with-timestamps' is `active'. * lisp/ox.el (org-export--skip-p): Include timestamps of type diary when `:with-timestamps' is `active'. * testing/lisp/test-ox-icalendar.el (test-ox-icalendar/diary-timestamp): Unit test for exporting timestamps of type diary. --- etc/ORG-NEWS | 14 ++ lisp/ox-icalendar.el | 2 +- lisp/ox.el| 2 +- testing/lisp/test-ox-icalendar.el | 15 +++ 4 files changed, 31 insertions(+), 2 deletions(-) diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS index a9357aa28..d31c62721 100644 --- a/etc/ORG-NEWS +++ b/etc/ORG-NEWS @@ -57,6 +57,20 @@ remote resources in the =#+include:='s. Now, an error is thrown to avoid seemingly ignored =#+include= statements when publishing via batch scripts. +*** Diary-style timestamps are exported together with active timestamps + +~org-export-with-timestamps~ and ~org-icalendar-with-timestamps~ now +treat diary-style timestamps as a type of active timestamp for +purposes of export. + +This mainly affects iCalendar export, where diary timestamps will now +be included when only active timestamps are exported (the default). + +This should have minimal impact on non-iCalendar exporters, since +~org-export-with-timestamps~ was already ~t~ by default. However, +users who manually set ~org-export-with-timestamps~ to ~active~ will +now have diary timestamps included as well. + ** New features # We list the most important features, and the features that may diff --git a/lisp/ox-icalendar.el b/lisp/ox-icalendar.el index 858d146d6..e7ca6aafb 100644 --- a/lisp/ox-icalendar.el +++ b/lisp/ox-icalendar.el @@ -746,7 +746,7 @@ (defun org-icalendar-entry (entry contents info) (lambda (ts) (when (let ((type (org-element-property :type ts))) (cl-case (plist-get info :with-timestamps) - (active (memq type '(active active-range))) + (active (memq type '(active active-range diary))) (inactive (memq type '(inactive inactive-range))) ((t) t))) (let ((uid (format "TS%d-%s" (cl-incf counter) uid))) diff --git a/lisp/ox.el b/lisp/ox.el index 7a0ab4dc7..79a1f5cfb 100644 --- a/lisp/ox.el +++ b/lisp/ox.el @@ -1857,7 +1857,7 @@ (defun org-export--skip-p (datum options selected excluded) (cl-case (plist-get options :with-timestamps) ((nil) t) (active - (not (memq (org-element-property :type datum) '(active active-range + (not (memq (org-element-property :type datum) '(active active-range diary (inactive (not (memq (org-element-property :type datum) '(inactive inactive-range) diff --git a/testing/lisp/test-ox-icalendar.el b/testing/lisp/test-ox-icalendar.el index e631b2119..c7c74c526 100644 --- a/testing/lisp/test-ox-icalendar.el +++ b/testing/lisp/test-ox-icalendar.el @@ -128,5 +128,20 @@ (ert-deftest test-ox-icalendar/warn-unsupported-repeater () (when (file-exists-p tmp-ics) (delete-file tmp-ics +(ert-deftest test-ox-icalendar/diary-timestamp () + "Test icalendar export of diary timestamps." + (let* ((tmp-ics (org-test-with-temp-text-in-file + "* First Sunday of the month +<%%(diary-float t 0 1)>" + (expand-file-name (org-icalendar-export-to-ics) +(unwind-protect +(with-temp-buffer + (insert-file-contents tmp-ics) + (save-excursion +(should (search-forward "SUMMARY:First Sunday of the month"))) + (save-excursion +(should (search-forward "RRULE:FREQ=MONTHLY;BYDAY=1SU" + (when (file-exists-p tmp-ics) (delete-file tmp-ics) + (provide 'test-ox-icalendar) ;;; test-ox-icalendar.el ends here -- 2.46.0
[PATCH] Fix ox-icalendar export of diary timestamps
ox-icalendar skips the following event with a diary timestamp during export: * First Sunday of the month <%%(diary-float t 0 1)> ox-icalendar actually has longstanding code to handle diary timestamps [1], but that code path is not reached, in part because `org-export--skip-p' always skips timestamps with type `diary'. The attached patch fixes this by including diary timestamps when the export option `:with-timestamps' is `active'. I think it's reasonable to consider diary timestamps as a type of active timestamp during export, since they are included in the agenda, and have angle brackets. [1] https://git.savannah.gnu.org/cgit/emacs/org-mode.git/tree/lisp/ox-icalendar.el?id=07dd3bcae6b7b5e0692fc40dd307a7e841179b52#n826 >From 32a5afd2803aa54e1e21b11d9d1e832e99538e9b Mon Sep 17 00:00:00 2001 From: Jack Kamm Date: Sat, 14 Sep 2024 22:48:44 -0700 Subject: [PATCH] ox-icalendar: Fix export of diary-style timestamps * lisp/ox-icalendar.el (org-icalendar-entry): Include timestamps of type diary when `:with-timestamps' is `active'. * lisp/ox.el (org-export--skip-p): Include timestamps of type diary when `:with-timestamps' is `active'. * testing/lisp/test-ox-icalendar.el (test-ox-icalendar/diary-timestamp): Unit test for exporting timestamps of type diary. --- lisp/ox-icalendar.el | 2 +- lisp/ox.el| 2 +- testing/lisp/test-ox-icalendar.el | 15 +++ 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/lisp/ox-icalendar.el b/lisp/ox-icalendar.el index 858d146d6..e7ca6aafb 100644 --- a/lisp/ox-icalendar.el +++ b/lisp/ox-icalendar.el @@ -746,7 +746,7 @@ (defun org-icalendar-entry (entry contents info) (lambda (ts) (when (let ((type (org-element-property :type ts))) (cl-case (plist-get info :with-timestamps) - (active (memq type '(active active-range))) + (active (memq type '(active active-range diary))) (inactive (memq type '(inactive inactive-range))) ((t) t))) (let ((uid (format "TS%d-%s" (cl-incf counter) uid))) diff --git a/lisp/ox.el b/lisp/ox.el index 7a0ab4dc7..79a1f5cfb 100644 --- a/lisp/ox.el +++ b/lisp/ox.el @@ -1857,7 +1857,7 @@ (defun org-export--skip-p (datum options selected excluded) (cl-case (plist-get options :with-timestamps) ((nil) t) (active - (not (memq (org-element-property :type datum) '(active active-range + (not (memq (org-element-property :type datum) '(active active-range diary (inactive (not (memq (org-element-property :type datum) '(inactive inactive-range) diff --git a/testing/lisp/test-ox-icalendar.el b/testing/lisp/test-ox-icalendar.el index e631b2119..c7c74c526 100644 --- a/testing/lisp/test-ox-icalendar.el +++ b/testing/lisp/test-ox-icalendar.el @@ -128,5 +128,20 @@ (ert-deftest test-ox-icalendar/warn-unsupported-repeater () (when (file-exists-p tmp-ics) (delete-file tmp-ics +(ert-deftest test-ox-icalendar/diary-timestamp () + "Test icalendar export of diary timestamps." + (let* ((tmp-ics (org-test-with-temp-text-in-file + "* First Sunday of the month +<%%(diary-float t 0 1)>" + (expand-file-name (org-icalendar-export-to-ics) +(unwind-protect +(with-temp-buffer + (insert-file-contents tmp-ics) + (save-excursion +(should (search-forward "SUMMARY:First Sunday of the month"))) + (save-excursion +(should (search-forward "RRULE:FREQ=MONTHLY;BYDAY=1SU" + (when (file-exists-p tmp-ics) (delete-file tmp-ics) + (provide 'test-ox-icalendar) ;;; test-ox-icalendar.el ends here -- 2.46.0
Re: [BUG] ox-icalendar does not support custom export snippets (was: Turning all my "all day events" FREE in an ics export)
Ihor Radchenko writes: > CCing the maintainer. Thanks Ihor. This issue dovetails nicely with the other recent iCalendar issue [1] that I am still looking into (sorry for the tardiness) > In theory, you should be able to do something like > > #+ICALENDAR_VCALENDAR: X-MICROSOFT-CDO-INTENDEDSTATUS:FREE > or an equivalent export snippet, but ox-icalendar does not support such > thing. I believe that it should be considered a bug - we should provide > some means to produce text to be exported verbatim from inside Org files. Yes I agree we should support this functionality. I see two basic ways: first, as you suggest #+ICALENDAR_VEVENT: X-MICROSOFT-CDO-INTENDEDSTATUS:FREE Or alternatively, #+begin_export icalendar X-MICROSOFT-CDO-INTENDEDSTATUS:FREE #+end_export We could follow the example of the latex exporter which supports both usages. However I am not sure we can simply export verbatim -- we may need to add additional checks. In particular various Org properties or special keywords may also be exported as iCal properties (e.g. SCHEDULED as DTSTART, or TODO as STATUS:NEEDS-ACTION), and we may want to handle duplicate behavior -- for example, by default TODO could be exported as STATUS:NEEDS-ACTION unless explicitly overridden by #ICALENDAR_STATUS (this is also related to the discussion in [1]). We could possibly use icalendar.el to parse the icalendar export blocks/keywords to check for duplicate properties. icalendar.el provides many useful parsing functions, though some are private methods (e.g. icalendar--get-event-property) and may need to be copied (or we could consider asking emacs-devel to make those methods public). [1] https://list.orgmode.org/87r0cmu7lc.fsf@localhost/T/#t
Re: ox-icalendar: Filter todo-types
Ihor Radchenko writes: > Another concern is that we already have `org-export-with-tasks' where > you can specify which todo keywords should be exported and which > headings should be exported, according to their todo keyword. So, > "no-export" appears to unnecessary. That's a good point. Michael, does `org-export-with-tasks' suffice for your original need/request? > However, after I looked at the RFC in more details now, I can see that > the values of STATUS property depend on the entry type: > >statvalue-event = "TENTATIVE";Indicates event is tentative. >/ "CONFIRMED";Indicates event is definite. >/ "CANCELLED";Indicates event was cancelled. >;Status values for a "VEVENT" > >statvalue-todo = "NEEDS-ACTION" ;Indicates to-do needs action. >/ "COMPLETED";Indicates to-do completed. >/ "IN-PROCESS" ;Indicates to-do in process of. >/ "CANCELLED";Indicates to-do was cancelled. >;Status values for "VTODO". > >statvalue-jour = "DRAFT";Indicates journal is draft. >/ "FINAL";Indicates journal is final. >/ "CANCELLED";Indicates journal is removed. > ;Status values for "VJOURNAL". > > Maybe we can introduce separate variables mapping todo keyword to status > depending on the entry type (VTODO vs. VEVENT; we do not export VJOURNAL)? Sure, but we should also consider the STATUS for VEVENTs created from non-TODO entries. Perhaps these variables could map tags as well as todo keywords to status? E.g., in the following: (setq org-icalendar-event-status-map '((cancelled . ("KILLED" "cancelled" Then any VEVENT created from entries whose todo-keyword or tag matches the above would have STATUS set to CANCELLED. For example, both of the following entries would contain VEVENTs exported as such: * An event that was cancelled :cancelled: A non-todo entry with active timestamp <2024-06-23>. It will be exported with STATUS as CANCELLED. * KILLED A todo that was cancelled SCHEDULED: <2024-06-22> This todo entry has both a scheduling timestamp as well as an active timestamp <2024-06-23>, and may create both VEVENT and VTODO. Any VEVENTs created will have STATUS as CANCELLED, due to "KILLED" being in org-icalendar-event-status-map. Any VTODO created will have STATUS set according to org-icalendar-todo-status-map instead.
Re: ox-icalendar: Filter todo-types
Michaël Cadilhac writes: > I have a task that was recurring, which I KILLED a few weeks ago. It > now looks like: > > ** KILLED Do the right thing >SCHEDULED: <2023-07-14 Fri +1w> > > ox-icalendar still exports it every week, because I have > 'event-if-not-todo in org-icalendar-use-scheduled (which is the > behavior I want for some other headers). > > BEGIN:VEVENT > DTSTAMP:20240622T163042Z > UID:SC-671b3d13-f985-472a-be33-b4eeb298f2cd > DTSTART;VALUE=DATE:20230714 > DTEND;VALUE=DATE:20230715 > RRULE:FREQ=WEEKLY;INTERVAL=1 > SUMMARY:S: KILLED Do the right thing : > CATEGORIES:todos > END:VEVENT > > Would it be acceptable to add a variable to filter todo-types, e.g., > with a variable org-icalendar-excluded-todo-types? More generally, > one could think of introducing a variable: >org-icalendar-entry-filter > which receives the ENTRY argument of org-icalendar-entry, and would > return non-nil if the entry is to be treated. (This is basically what > I do using an advice.) I agree ox-icalendar should have an option to exclude certain todo-keywords from export. However I think the option should also be flexible to allow exporting todo-keywords as different STATUS: https://datatracker.ietf.org/doc/html/rfc5545#section-3.8.1.11 For example, in your case it might make more sense to export the task as CANCELLED in iCalendar. I think I'd rather have a customization option for this than a filter function. That way, other Lisp programs that import ICS to Org (such as org-caldav) could refer to this option to interoperate with ox-icalendar for 2-way sync. Perhaps we could have an option ox-icalendar-export-todo-keywords-as which could determine whether a keyword should be exported, and if so with what status, e.g.: (defcustom ox-icalendar-export-todo-keywords-as '((no-export . ("SKIP")) (cancelled . ("KILLED" "CANCELLED")) (in-process . ("PROG" By default, todo-keywords not in the list would be exported with status "NEEDS-ACTION" or "COMPLETED", depending on whether the keyword is a done state. What do you think?
Re: Asynchronous blocks for everything (was Re: [BUG] Unexpected result when evaluating python src block asynchronously [9.7-pre (release_9.6.17-1131-gc9ed03.dirty @ /home/yantar92/.emacs.d/straight/b
Matt writes: > AFAIK, aside from appending "&", =make-process= is the only way. It would > probably make sense to continue using =shell= though. If I understand > correctly, you (and others) jump between the Org buffer block and the comint > buffer? Yes, I typically use ob-R and ob-python in this way. Aside from the convenience of interacting directly with the REPL, the comint mode may provide other useful facilities, for example in ob-R the inferior ESS mode provides completion that is aware of the objects in the current session. However I think modifying `org-babel-eval' will mainly be useful to provide async to the nonsession users. Comint-based sessions mainly use functionality from ob-comint.el, not ob-eval.el, and I think that the use cases are different enough that they shouldn't be merged into a single implementation.
Re: Asynchronous blocks for everything (was Re: [BUG] Unexpected result when evaluating python src block asynchronously [9.7-pre (release_9.6.17-1131-gc9ed03.dirty @ /home/yantar92/.emacs.d/straight/b
Matt writes: > The challenge I've found with Babel is figuring out how to make the changes. > My current approach is to address bugs and to make changes that move us > toward something like the ob-blub implementation. I wonder if it might help > to discuss the core ideas and use a minimal reference implementation that > serves as a guide for the actual changes we make. > > Curious to hear other people's thoughts! I don't remember the details, but my past self [1] thought it would be good to find a way to replace `process-file' with `make-process' in `org-babel--shell-command-on-region' or `org-babel-eval', and it seems you are thinking along those lines with `my-org-babel-eval-async'. Hope you're able to make progress on this and get the improvements into ob-eval.el eventually. [1] https://list.orgmode.org/871rczg7bi@gmail.com/#t
Re: Asynchronous blocks for everything (was Re: [BUG] Unexpected result when evaluating python src block asynchronously [9.7-pre (release_9.6.17-1131-gc9ed03.dirty @ /home/yantar92/.emacs.d/straight/b
Bruno Barbier writes: > I'm not using it with official org backends (yet). I'm using it with > several custom backends that I'm working on. One of the backend > delegate the block executions to emacs subprocesses: so I have a kind of > asynchronous executions for free for any language, including elisp > itself. For sessions, wouldn't running in a subprocess prevent the user from directly interacting with the REPL outside of Org? If so, that's a problem. Org-babel sessions need to play nicely with inferior Python, inferior ESS, and other interactive comint modes. > So, here we go. You'll find attach a set of patchs. It works for me with > Emacsc 30.50 and 9.7-pre (from today). I suggest to keep these patches on a public branch somewhere, see: https://orgmode.org/worg/org-contribute.html#patches "When discussing important changes, it is sometimes not so useful to send long and/or numerous patches. In this case, you can maintain your changes on a public branch of a public clone of Org and send a link to the diff between your changes and the latest Org commit that sits in your clone." I tried running your example on emacs29 using emacs -q -L /path/to/org-mode/lisp my-async-tests.org but it fails with the error below. Also "make" gives a bunch of compilation warnings (which I've put at the bottom). > +You need to load: > + #+begin_src elisp :results silent > + (load-file "my-async-tests.el") > + #+end_src This raises the following error in *Org-Babel Error Output* void-variable (org-elib-async-process) [ Babel evaluation exited with code 127 ] All the subsequent blocks don't work because of that, for example: > +A simple execution: > +#+begin_src bash > + date > +#+end_src yields: org-babel-execute-src-block: No org-babel-execute function for bash: my-shell-babel-schedule! Finally here are the warnings when running "make": Compiling single /home/jack/src/org-mode/2024-02-brubar-async/lisp/ob-core.el... In org-babel--async-feedbacks: ob-core.el:851:2: Warning: docstring has wrong usage of unescaped single quotes (use \= or different quoting) ob-core.el:871:9: Warning: Unused lexical variable `result-indent' ob-core.el:906:18: Warning: Unused lexical variable `header' Compiling single /home/jack/src/org-mode/2024-02-brubar-async/lisp/org-elib-async.el... In toplevel form: org-elib-async.el:52:11: Warning: reference to free variable ‘org-elib-async-process’ org-elib-async.el:52:43: Warning: reference to free variable ‘&key’ org-elib-async.el:52:48: Warning: reference to free variable ‘input’ org-elib-async.el:52:54: Warning: reference to free variable ‘callback’ org-elib-async.el:78:29: Warning: reference to free variable ‘command’ org-elib-async.el:127:9: Warning: Variable ‘last-elapsed’ left uninitialized In org-elib-async-wait-condition: org-elib-async.el:137:12: Warning: ‘signal’ called with 3 arguments, but accepts only 2 In end of data: org-elib-async.el:262:16: Warning: the function ‘org-id-uuid’ is not known to be defined. org-elib-async.el:52:35: Warning: the function ‘command’ is not known to be defined. org-elib-async.el:52:2: Warning: the function ‘cl-defun’ might not be defined at runtime.
Re: Async Python src block behavior with :dir header property
Ihor Radchenko writes: >> Compiling >> /home/jack/src/org-mode/2024-01-async-file-results-dir/lisp/ob-core.el... >> >> In org-babel-session-buffer: >> ob-core.el:785:15: Warning: reference to free variable ‘buffer-name’ > > But this is a real problem. > See the attached updated version of the patch set. Looks good, thanks. I tested it out it works without errors/warnings now.
Re: [BUG] Unexpected result when evaluating python src block asynchronously [9.7-pre (release_9.6.17-1131-gc9ed03.dirty @ /home/yantar92/.emacs.d/straight/build/org/)]
Ihor Radchenko writes: >> I agree that it would be good to redesign it, but am not sure where to >> start. > > For example, > > 1. Change `org-babel-comint-async-register' to return UUID and to store >PARAMS as passed by the backend (current approach with PARAMS being >derived from src blocks prevents backends to transform src block >PARAMS dynamically). > 2. Change `org-babel-insert-result' to handle :async t specially, >inserting something reliable, like #+async: in place of result >without performing extra transformations. > 3. Change `org-babel-insert-result' to accept an internal parameter >that will make it replace #+async: keyword rather than perform >normal result insertion. > 4. Change `org-babel-comint-async-filter' to use the previously passed >PARAMS, remove :async t from them, and arrange the call to >`org-babel-insert-result' to replace the #+async: keyword. That all sounds reasonable...if you work on this, let me know if you want any help with testing.
Re: [BUG] Unexpected result when evaluating python src block asynchronously [9.7-pre (release_9.6.17-1131-gc9ed03.dirty @ /home/yantar92/.emacs.d/straight/build/org/)]
Bruno Barbier writes: > FWIW, I've been trying to use asynchronous blocks for everything, not > only the source blocks that are based on the comint mode. I think it > would be good if ob-core itself could provide an asynchronous API. I've > modified my Org so that it does have such an API. This is work in > progress; let me describe it. > > I've modified ob-core itself to allow asynchronicity. In the > asynchrosous case, instead of calling: > > (org-babel-execute:LANG body params) > > I'm calling: > > (org-babel-schedule:LANG body params handle-result) > > where `org-babel-schedule:LANG' is in charge of calling `handle-result' > with the result (or the error) when it is known; `handle-result' takes > care to call `org-babel-insert-result' at the correct place (and > `org-babel-insert-result' is only called with a real result). Sounds interesting, a couple questions: 1. Which languages have you implemented/tested this on? 2. Does it apply for sessions, nonsessions, or both? > While the execution is pending, I'm using the same technique that Org is > using when a source block is being edited: the result is left untouched, > but below an overlay. The overlay is used to know where to insert the > result and to display the status/progress of the execution. If the file > is closed and the execution fails, nothing is lost, the old result is > still available. Also interesting, I think it's worth exploring/testing this overlay idea out. Does that mean that output is asynchronously printing into the Org buffer? It sounds cool but I wonder if it might cause problems while trying to edit another part of the buffer.
Re: Async Python src block behavior with :dir header property
Ihor Radchenko writes: > +(defun org-babel-session-buffer (&optional info) > + "Return buffer name for session associated with current code block. > +Return nil when no such live buffer with process exists. > +When INFO is non-nil, it should be a list returned by > +`org-babel-get-src-block-info'. > +This function uses org-babel-session-buffer: function to > +retrieve backend-specific session buffer name." > + (when-let* ((info (or info (org-babel-get-src-block-info 'no-eval))) > + (lang (nth 0 info)) > + (session (cdr (assq :session (nth 2 info > + (cmd (intern (concat "org-babel-session-buffer:" lang))) > + buffer-name) On executing any python session block I am getting the following error which I think is caused by the above: Debugger entered--Lisp error: (void-variable buffer-name) Also, make shows a byte-compiler warning about this: Compiling /home/jack/src/org-mode/2024-01-async-file-results-dir/lisp/ob-core.el... In org-babel-session-buffer: ob-core.el:785:15: Warning: reference to free variable ‘buffer-name’
Re: Async Python src block behavior with :dir header property
Ihor Radchenko writes: > What we can do is to introduce a new backend template function > org-babel-session-buffer: that will be passed a session name and > src block params and return the session buffer name. > > If such function is not defined, we fall back to assumption that session > buffer is named the same as the session. > > WDYT? Sounds good -- I think this is the best solution.
Re: [BUG] Unexpected result when evaluating python src block asynchronously [9.7-pre (release_9.6.17-1131-gc9ed03.dirty @ /home/yantar92/.emacs.d/straight/build/org/)]
Ihor Radchenko writes: > Jack Kamm writes: > >> I think the correct solution would be for `org-babel-insert-result' to >> not insert file results (or any other special results) for async session >> blocks. Perhaps in this case, `org-babel-insert-result' could return a >> new result type, named "async", "future", or similar. > > That will not work. > `org-babel-comint-async-filter' expects a unique result to be inserted > into Org buffer, so that it can be located, and replaced by the async > evaluation output. > > So, we have to insert some kind of indicator for async result. I meant that we could return something like "async:uuid-abcd-1234" or "async:/path/to/tmpfile", so that `org-babel-comint-async-filter' could still find the result. > Of course, the existing scheme of coordination between > `org-babel-insert-result' and `org-babel-comint-async-filter' is > erroneous: > > 1. We have the problem with :results file value discussed here > 2. We have a worse problem with :results file :file foo when the result >may not be unique > 3. We have :results append/prepend completely broken because >`org-babel-comint-async-filter' simply calls >`org-babel-insert-result' implicitly assuming that the existing >indicator is replaced. > > The whole thing should be re-designed. I agree that it would be good to redesign it, but am not sure where to start. A bit of a tangent, but if you are thinking about re-designing this, then it may be worth considering ob-jupyter's implementation of async sessions [1]. In particular, I believe it leaves a marker [2] where it needs to insert the future result. I don't remember the details, e.g. how it keeps track of which marker is for which result. But it seems neat, and might work better for some cases such as appending/prepending results. [1] https://github.com/emacs-jupyter/jupyter [2] https://www.gnu.org/software/emacs/manual/html_node/elisp/Markers.html
Re: Async Python src block behavior with :dir header property
Ihor Radchenko writes: > Thanks! > Attaching the two patches combined with some fixed to my patch. > > Please check if these two patches solve the discussed bug. The original bug for async sessions looks fixed. But I encountered a complication regarding the non-async bug of changing :dir. In particular the following example: #+begin_src python :dir otherdir :session pysession :return figname :results file value :mkdirp yes import matplotlib.pyplot as plt plt.figure(figsize=(1, 1)) plt.plot([1, 2]) figname = 'fig.svg' plt.savefig(figname) #+end_src #+RESULTS: [[file:otherdir/fig.svg]] #+begin_src python :dir otherdir2 :session pysession :return figname :results file value :mkdirp yes import matplotlib.pyplot as plt plt.figure(figsize=(1, 1)) plt.plot([1, 2]) figname = 'fig5.svg' plt.savefig(figname) #+end_src #+RESULTS: [[file:otherdir2/fig5.svg]] As you can see the second result points to the wrong directory. However, if replacing ":session pysession" with ":session *pysession*", then it works. It's because ob-python starts the session in buffer "*pysession*" (it adds earmuffs around the session name when missing). So the following doesn't find the inferior Python: > + ((when-let ((session (cdr (assq :session params > + (when (org-babel-comint-buffer-livep session) Honestly, I'm not sure why ob-python insists on the session buffer having the earmuffs, that behavior is from before my time. In particular, I'm not yet sure if the motivation is just cosmetic (because of the general convention like *shell*, *Python*, *R*, etc), or whether it actually affects python.el behavior. But if it's just cosmetic, then we might consider removing this behavior to simplify things. Though note that ob-python is not the only one with this sort of behavior -- looks like ob-lua might behave similarly, and until recently ob-R also had some surprising behavior depending on whether the session name had earmuffs.
Re: Async Python src block behavior with :dir header property
Ihor Radchenko writes: >> I agree it's a problem -- if there are multiple blocks with the same >> session but different ":dir" arguments, then a ":file" result of the >> second block will be relative to the wrong :dir. >> >> This seems like a longstanding problem, and affects both async and >> non-async session blocks. > > Maybe something like the attached? Nice, that seems like the right way to do it. I updated the patch for `org-babel-comint-async-filter' to follow the same approach, setting default-directory based on the session buffer's value rather than the :dir header arg. >From 1a1a22e4f4a12ebe83c3fef26fe727066fb14476 Mon Sep 17 00:00:00 2001 From: Jack Kamm Date: Wed, 31 Jan 2024 20:06:00 -0800 Subject: [PATCH] ob-comint: Make file results from async sessions respect :dir header * lisp/ob-comint.el (org-babel-comint-async-filter): Set default-directory before calling `org-babel-insert-result' https://list.orgmode.org/875xz9o4nj.fsf@localhost/T/#t --- lisp/ob-comint.el | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lisp/ob-comint.el b/lisp/ob-comint.el index 7d258ea0e..349524701 100644 --- a/lisp/ob-comint.el +++ b/lisp/ob-comint.el @@ -224,6 +224,8 @@ (defun org-babel-comint-async-filter (string) (file-callback org-babel-comint-async-file-callback) (combined-string (concat org-babel-comint-async-dangling string)) (new-dangling combined-string) + ;; Assumes comint filter called with session buffer current + (session-dir default-directory) ;; list of UUID's matched by `org-babel-comint-async-indicator' uuid-list) (with-temp-buffer @@ -248,7 +250,8 @@ (defun org-babel-comint-async-filter (string) (let* ((info (org-babel-get-src-block-info)) (params (nth 2 info)) (result-params -(cdr (assq :result-params params +(cdr (assq :result-params params))) + (default-directory session-dir)) (org-babel-insert-result (funcall file-callback (nth @@ -291,7 +294,8 @@ (defun org-babel-comint-async-filter (string) (let* ((info (org-babel-get-src-block-info)) (params (nth 2 info)) (result-params - (cdr (assq :result-params params + (cdr (assq :result-params params))) + (default-directory session-dir)) (org-babel-insert-result res-str result-params info)) t -- 2.43.0
Re: [BUG] No result exporting combined org-agenda files to icalendar [9.6.15 (release_9.6.15 @ z:/emacs-i686/share/emacs/29.2/lisp/org/)]. It was: org-icalendar export problems
Ypo writes: > I can't share my private org files. And I find myself unable to isolate > the causes of the possible errors. One strategy that might help is to "bisect" the Org file by repeatedly splitting it into 2 files, and exporting each, to find where the problematic entries are. By splitting in half every time, you might be able to isolate the problem relatively quickly, and produce a minimal example from that.
Re: [BUG] Unexpected result when evaluating python src block asynchronously [9.7-pre (release_9.6.17-1131-gc9ed03.dirty @ /home/yantar92/.emacs.d/straight/build/org/)]
Bruno Barbier writes: > Hi Ihor, > > Ihor Radchenko writes: > >> >> This is most likely something about my current system setup - I can >> reproduce with other Org mode and Emacs versions. But I have no clue >> what is the cause. > > I'm getting the same as you with your MWE. > > The tag, used by ob-comint async, is: > >"/tmp/babel-zqh04P/python-GL5N5d" > > but, in "/tmp/bug.org" it becomes: > >"babel-zqh04P/python-GL5N5d" > > (`org-babel-result-to-file' transformed it into a simpler relative > path). > > The filter `org-babel-comint-async-filter' cannot spot it, because > it's searching for the exact string "/tmp/babel-zqh04P/python-tXsdFw". Thanks, I believe your diagnosis is correct, and can confirm the bug occurs when the Org file is in /tmp. The problem goes away when `org-link-file-path-type' is set to absolute. I think the correct solution would be for `org-babel-insert-result' to not insert file results (or any other special results) for async session blocks. Perhaps in this case, `org-babel-insert-result' could return a new result type, named "async", "future", or similar.
Re: Async Python src block behavior with :dir header property
Ihor Radchenko writes: > The patch generally looks reasonable, although I am slightly concerned > about interaction between :dir and session we describe in the manual: > > When ‘dir’ is used with ‘session’, Org sets the starting directory > for a new session. But Org does not alter the directory of an already > existing session. I agree it's a problem -- if there are multiple blocks with the same session but different ":dir" arguments, then a ":file" result of the second block will be relative to the wrong :dir. This seems like a longstanding problem, and affects both async and non-async session blocks. Ideally, ":dir" should be set at session level rather than block level. This could be done via PROPERTY if there is only one session in the file or subtree, but there isn't a mechanism for this more generally. Perhaps the simplest solution is to add a check to org-lint, requiring all blocks with the same session to have the same :dir.
Re: Async Python src block behavior with :dir header property
Ihor Radchenko writes: > [ CCing ob-python maintainer ] > > Nasser Alkmim writes: > >> Here is a clearer description of the unwanted behavior: >> >> 1. emacs -Q ~/Desktop/testasync.org >> >> Then evaluate (require 'ob-python) and paste this source block: >> >> #+begin_src python :dir otherdir :async yes :session pysession :return >> figname :results file value :mkdirp yes >> import matplotlib.pyplot as plt >> plt.figure(figsize=(1, 1)) >> plt.plot([1, 2]) >> figname = 'fig.svg' >> plt.savefig(figname) >> #+end_src >> >> Execute the source block, which results in >> >> #+RESULTS: >> [[file:fig.svg]] > > I don't even see this. > Confirmed. Does the attached patch fix the issue? It seems the problem is with async sessions generally (not just ob-python), and happens because `org-babel-comint-async-filter' does not set `default-directory' before calling `org-babel-insert-result', unlike `org-babel-execute-src-block'. >From 1430a27e4416d5e88094a64360015a6a2ae7315c Mon Sep 17 00:00:00 2001 From: Jack Kamm Date: Wed, 31 Jan 2024 20:06:00 -0800 Subject: [PATCH] ob-comint: Make file results from async sessions respect :dir header * lisp/ob-comint.el (org-babel-comint-async-filter): Set default-directory before calling `org-babel-insert-result' https://list.orgmode.org/875xz9o4nj.fsf@localhost/T/#t --- lisp/ob-comint.el | 17 +++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/lisp/ob-comint.el b/lisp/ob-comint.el index 7d258ea0e..ecce18a95 100644 --- a/lisp/ob-comint.el +++ b/lisp/ob-comint.el @@ -248,7 +248,14 @@ (defun org-babel-comint-async-filter (string) (let* ((info (org-babel-get-src-block-info)) (params (nth 2 info)) (result-params -(cdr (assq :result-params params +(cdr (assq :result-params params))) + (tmp-file (expand-file-name tmp-file)) + (dir (cdr (assq :dir params))) + (default-directory +(if dir +(file-name-as-directory + (expand-file-name dir)) + default-directory))) (org-babel-insert-result (funcall file-callback (nth @@ -291,7 +298,13 @@ (defun org-babel-comint-async-filter (string) (let* ((info (org-babel-get-src-block-info)) (params (nth 2 info)) (result-params - (cdr (assq :result-params params + (cdr (assq :result-params params))) + (dir (cdr (assq :dir params))) + (default-directory + (if dir + (file-name-as-directory +(expand-file-name dir)) + default-directory))) (org-babel-insert-result res-str result-params info)) t -- 2.43.0
Re: org-icalendar export problems
ypuntot writes: > Problem! Found 1 error > > Errors > Lines not delimited by CRLF sequence near line # 1 > Reference: RFC 5545 3.1. Content Lines Which version of Org are you using? (M-x org-version) Org 9.7 (unreleased) contains some fixes for how ox-icalendar handles newlines. If you are using a previous version of Org, please try out the latest main branch and see if it fixes the problem.
Re: org-icalendar export problems
Ihor Radchenko writes: > CCing ox-icalendar maintainer. > > ypuntot writes: > >> I am trying to create an android calendar based on the agenda, and the first >> problem I am finding it's that I can't import it into android (ICSx^5). I am >> receiving these errors when trying to do it: >> >> Couldn't parse iCalendar ... > > May you be able to share the problematic icalendar file? If you're not comfortable sharing the icalendar file on a public mail list, you could also try out the iCalendar validator, which may help to identify the issue with the ics file: https://icalendar.org/validator.html FWIW, I did try out ICSx5 with the exported ics from a simple Org file, and it worked. So we'll need more specific information to fix the problem.
Re: [PATCH] Set Python shell in Org edit buffer
Ihor Radchenko writes: > Applied, onto main, fixing the oversight in > org-src-associate-babel-session (now, it does not require session > running). > https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=319563cef > > Since ESS already released a new version with my patch for ESS included, > no advice is needed. Thank you!
Re: [PATCH] Set Python shell in Org edit buffer
Ihor Radchenko writes: > Liu Hui writes: > >> Yes, I have updated the text and you're welcome to improve it. Thanks! >> From c503b2ed5116e2abae25459b09abc919074ac54a Mon Sep 17 00:00:00 2001 >> From: Liu Hui >> Date: Tue, 5 Dec 2023 11:40:38 +0800 >> Subject: [PATCH] Set Python shell in Org edit buffer > > Now, after amending `org-src-associate-babel-session' to execute even > when no session is active, we can use > `org-babel-python-associate-session'. > > Attaching tentative patch that should be equivalent to yours. Thanks Ihor. I tested the patch and it seems to work well.
Re: [patch] improved: add TTL as defcustom to ox-icalendar
Ihor Radchenko writes: > LGTM. Thanks! Thanks, I've applied now: https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=da2b61b09e1eff957e6b2560a2f9c8509de6beac
Re: [patch] improved: add TTL as defcustom to ox-icalendar
Ihor Radchenko writes: > This does not look right. Did you try to export anything with this patch > applied? You are passing multiple string arguments to `format'. > > Finally, I think that we can document the new keyword in the manual. I've updated the patch so that it applies cleanly onto the latest main, and also fixed a few issues including the ones Ihor pointed out above. Let me know if there are any issues -- otherwise I'll apply it to main in the near future. >From 24a6bc78c74d16a6aa7dbd081be9f626b20bbf20 Mon Sep 17 00:00:00 2001 From: Detlef Steuer Date: Sat, 4 Feb 2023 21:40:09 +0100 Subject: [PATCH] lisp/ox-icalendar.el: Add time-to-live functionality to ox-icalendar This commit adds functionality for ox-icalendar to set X-PUBLISHED-TTL in the exported ICS, which advises a subscriber to the exported ICS file to reload after the given time interval. * lisp/ox-icalendar.el (org-icalendar-ttl): New option to set X-PUBLISHED-TTL in exported ICS (icalendar): Add ICAL-TTL export keyword (org-icalendar--vcalendar): Add argument for TTL (org-icalendar-template, org-icalendar-export-current-agenda, org-icalendar--combine-files): Pass TTL to `org-icalendar--vcalendar' Co-authored-by: Ihor Radchenko Co-authored-by: Jack Kamm --- doc/org-manual.org | 9 + etc/ORG-NEWS | 17 lisp/ox-icalendar.el | 47 ++-- 3 files changed, 67 insertions(+), 6 deletions(-) diff --git a/doc/org-manual.org b/doc/org-manual.org index 7e5ac0673..8fe79ee45 100644 --- a/doc/org-manual.org +++ b/doc/org-manual.org @@ -16369,6 +16369,15 @@ information. The iCalendar standard defines three visibility classes: The server should treat unknown class properties the same as =PRIVATE=. +#+cindex: @samp{ICAL-TTL}, keyword +#+vindex: org-icalendar-ttl +The exported iCalendar file can advise clients how often to check for +updates. This duration can be set globally with the +~org-icalendar-ttl~ variable, or on a per-document basis with the +=ICAL-TTL= keyword. This option should be set using the iCalendar +notation for time durations; consult the docstring of +~org-icalendar-ttl~ for more details. + Exporting to iCalendar format depends in large part on the capabilities of the destination application. Some are more lenient than others. Consult the Org mode FAQ for advice on specific diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS index 1bf7eb5b4..5a93788e9 100644 --- a/etc/ORG-NEWS +++ b/etc/ORG-NEWS @@ -424,6 +424,23 @@ The change is breaking when ~org-use-property-inheritance~ is set to ~t~. The =TEST= parameter is better served by Emacs debugging tools. ** New and changed options +*** New custom setting ~org-icalendar-ttl~ for the ~ox-icalendar~ backend + +The option ~org-icalendar-ttl~ allows to advise a subscriber to the +exported =.ics= file to reload after the given time interval. + +This is useful i.e. if a calendar server subscribes to your exported +file and that file is updated regularly. + +See IETF RFC 5545, Section 3.3.6 Duration and +https://en.wikipedia.org/wiki/ICalendar#Other_component_types for +details. + +Default for ~org-icalendar-ttl~ is ~nil~. In that case the setting +will not be used in the exported ICS file. + +The option may also be set using the =ICAL-TTL= keyword. + *** The default value of ~org-attach-store-link-p~ is now ~attached~ Now, after attaching a file, =[[attach:...]]= link to the attached file diff --git a/lisp/ox-icalendar.el b/lisp/ox-icalendar.el index cb416a68f..584c78fcf 100644 --- a/lisp/ox-icalendar.el +++ b/lisp/ox-icalendar.el @@ -331,6 +331,32 @@ (defcustom org-icalendar-date-time-format ":%Y%m%dT%H%M%S" (const :tag "Universal time" ":%Y%m%dT%H%M%SZ") (string :tag "Explicit format"))) +(defcustom org-icalendar-ttl nil + "Time to live for the exported calendar. + +Subscribing clients to the exported ics file can derive the time +interval to read the file again from the server. One example of such +client is Nextcloud calendar, which respects the setting of +X-PUBLISHED-TTL in ICS files. Setting `org-icalendar-ttl' to \"PT1H\" +would advise a server to reload the file every hour. + +See https://icalendar.org/iCalendar-RFC-5545/3-8-2-5-duration.html +for a complete description of possible specifications of this +option. For example, \"PT1H\" stands for 1 hour and +\"PT0H27M34S\" stands for 0 hours, 27 minutes and 34 seconds. + +The default value is nil, which means no such option is set in +the ICS file. This option can also be set on a per-document basis +with the ICAL-TTL export keyword." + :group 'org-export-icalendar + :type '(choice + (const :tag "No refresh period" nil) + (const :tag "One hour" "PT1H") + (const :tag "One day" "PT1D") + (const :tag "
Re: Inconsistent handling of multi-line properties
Ihor Radchenko writes: > Thanks! > You can replace Maintainer: line in ox-icalendar.el with your name. Done. > Sure. > I checked my records and found one outstanding issue related to > ox-icalendar - > https://list.orgmode.org/orgmode/20211230225919.1a660...@hsu-hh.de/ > > The patch proposed by Detlef still has outstanding issues to address, > but the author does not have free time to work on this since the > beginning of last year (I did several follow-ups off list; latest in > September). > > At this point, we should either cancel that patch or amend it ourselves > and apply. Thanks -- I will amend the patch, and send an update to that thread before applying.
Re: Inconsistent handling of multi-line properties
Ihor Radchenko writes: > Ihor Radchenko writes: > >>> I have noticed that properties that stretch over multiple lines using >>> the :value+: syntax are ignored by org-element-property and therefore >>> also by e.g. org-export-get-node-property when exporting to ics via >>> ox-icalendar.el (see example below). I was wondering now whether this is >>> intentional and to be expected or a bug? > >> 3. Change ox-icalendar to consider :LOCATION+ properties and merge them >>during export. > > I went with this approach. > See the attached tentative patch. Thanks Ihor -- this is a nice improvement for ox-icalendar. My only suggestion would be to add this information and your small location example to the manual. > CCing Jack - you expressed interest in ox-icalendar in the past. > > P.S. We need a maintainer for ox-icalendar ;) Sure, I'm happy to help with this :) Please forward/CC any issues you'd like me to look at. I'll setup an email filter to try and catch iCalendar related subjects, but sometimes I miss things on the list.
Re: [PATCH] Set Python shell in Org edit buffer
Ihor Radchenko writes: >> My concern is that advising `ess-request-a-process' would cause >> maintenance burden on ob-R. It would require some knowledge about the >> ESS internals to maintain properly. > > Not really. I only meant writing an advice iff our request is accepted > by ESS devs. Then, all we need is to advice the earlier versions of ESS > and remove the advice after the new ESS release (we only support the > latest release of the optional third-party packages: > https://orgmode.org/worg/org-maintenance.html#emacs-compatibility). No > changes to advice will be needed in future. > > I plan to propose a patch for ESS soon and see if it is going to be > accepted. In that case, this sounds great. Hope the ESS devs are receptive!
Re: [PATCH] Set Python shell in Org edit buffer
Ihor Radchenko writes: >> It's annoying there's no way to tell ESS to start new session instead of >> evaluating in existing one. Here are a few alternatives we could >> consider to deal with this: >> >> 1. Change the worg/ORG-NEWS, to suggest users make sure the session >> exists (either by evaluating a source block or call "M-x R" in edit >> block) before running ess-eval-line. >> >> 2. Add ob-R and ob-julia customization options (as previously suggested) >> to explicitly control the startup behavior (either to auto-start, or not). >> >> 3. Submit PR to ESS to add a variable we could let-bind, to force it to >> start an associated session rather than evaluate in an existing >> non-associated sessions. >> >> Currently I lean towards a combination of #1 and #3, but am not sure, >> and happy to go with whatever you think is best. > > We can also advice `ess-request-a-process' as a temporary workaround. My concern is that advising `ess-request-a-process' would cause maintenance burden on ob-R. It would require some knowledge about the ESS internals to maintain properly. Reading through `ess-request-a-process' is rather daunting, and it doesn't look straightforward to patch it to behave as we want. I think the reason is because ESS allows you to call `rename-buffer' on the inferior R session, and still have it remain associated with its editing buffers. Which is quite a different model than the way python.el works. After some more thought, I'm now leaning towards #2 as the best and simplest option. In particular, I think ob-R should have an option `org-babel-R-start-session-on-edit', which would have 2 options, nil (default) or t. No need to add an option for the current "earmuffs" behavior unless we find someone who actually wants it. Then, update Worg documentation to suggest manually calling "M-x R" if session hasn't already started yet, or to set `org-babel-R-start-session-on-edit' if the user would like that behavior. The alternative (trying to make `ess-eval-line' smart enough to start new session without user intervention) seems overcomplicated from Org development/maintenance POV, for too little gain. Clear documentation and configuration options on Org side are good enough. This is a bit of an edge case anyways, and unless there is interest from the ESS developers, I think it is too much struggle to try and change how ESS works for this. (But thank you for emailing ESS to ask).
Re: [PATCH] Set Python shell in Org edit buffer
Ihor Radchenko writes: > See the attached tentative patch. Thanks! > --- > etc/ORG-NEWS | 11 +++ > lisp/ob-R.el | 20 ++-- > lisp/ob-julia.el | 16 +--- > 3 files changed, 26 insertions(+), 21 deletions(-) Not sure if you are doing this in a separate commit, but you also need to make the change to org-src.el to remove (org-babel-comint-buffer-livep session) in `org-src-associate-babel-session'. Else, `org-babel-R-associate-session' won't run if there's no live session, and hence ESS won't create new session with the right name. > * Version 9.7 (not released yet) > ** Important announcements and breaking changes > +*** ~org-edit-special~ no longer force-starts session in R and Julia source > blocks > + > +Previously, when R/Julia source block had =:session= header argument > +set to a session name with "earmuffs" (like =*session-name*=), > +~org-edit-special~ always started a session, if it does not exist. > + > +Now, ~org-edit-special~ arranges that a new session with correct name > +is initiated only when user explicitly executes R/Julia-mode commands > +that trigger session interactions. The same session will remain > +available in the context of Org babel. I tested the patch (plus the additional change to org-src.el), with an Org file with following 2 blocks: #+begin_src R :session foo :results output print('foo') #+end_src #+begin_src R :session *bar* :results output print('bar') #+end_src On block "foo", I did C-', and then ess-eval-line. It creates a session named "foo", as expected. On block "*bar*", I did the same. It does not create session named "*bar*", instead evaluating in session "foo". It seems ESS will always assume you want to evaluate in existing session if one exists, rather than start a new associated session, and it seems there is no way to tell it to behave otherwise. However, calling "M-x R" while editing block "*bar*" does create session "*bar*" with correct name. After sessions "foo" and "*bar*" have both been created, doing C-' and then ess-eval-line will evaluate in the correct session. It's annoying there's no way to tell ESS to start new session instead of evaluating in existing one. Here are a few alternatives we could consider to deal with this: 1. Change the worg/ORG-NEWS, to suggest users make sure the session exists (either by evaluating a source block or call "M-x R" in edit block) before running ess-eval-line. 2. Add ob-R and ob-julia customization options (as previously suggested) to explicitly control the startup behavior (either to auto-start, or not). 3. Submit PR to ESS to add a variable we could let-bind, to force it to start an associated session rather than evaluate in an existing non-associated sessions. Currently I lean towards a combination of #1 and #3, but am not sure, and happy to go with whatever you think is best.
Re: [PATCH] Set Python shell in Org edit buffer
Jack Kamm writes: > Also it seems unnecessary to call `ess-make-buffer-current', as it's > already called by `ess-force-buffer-current' (which is called by > `ess-eval-region'). Though it doesn't hurt to call it, either. On reflection, maybe it's better to keep `ess-make-buffer-current'. Maybe it's useful for other functionality besides evaluation (e.g. completion), or on older versions of ESS. And even if it isn't necessary anymore, it doesn't hurt either.
Re: [PATCH] Set Python shell in Org edit buffer
Ihor Radchenko writes: > Note that I proposed to remove auto-starting session completely, which > is in odds to what you propose below. Sure, I'm fine with that -- it seems like a reasonable change. > IMHO, it might be enough to adjust org-babel-R-associate-session as the > following > > (defun org-babel-R-associate-session (session) > "Associate R code buffer with an R session. > Make SESSION be the inferior ESS process associated with the > current code buffer." > (setq ess-local-process-name > (process-name (get-buffer-process session))) > (when ess-local-process-name (ess-make-buffer-current)) > (setq-local ess-gen-proc-buffer-name-function (lambda (_) session))) I think you need to check that (get-buffer-process session) is non-nil before calling process-name, otherwise you'll get (wrong-type-argument processp nil). Also it seems unnecessary to call `ess-make-buffer-current', as it's already called by `ess-force-buffer-current' (which is called by `ess-eval-region'). Though it doesn't hurt to call it, either. Otherwise, this looks good to me.
Re: [PATCH] Set Python shell in Org edit buffer
Ihor Radchenko writes: > So, a good option could be > (1) removing (org-babel-comint-buffer-livep session) from > `org-src-associate-babel-session' > (2) Removing `org-babel-edit-prep:R' > > With the above, we can use `org-babel-python-associate-session' Sounds good to me. > I imagine that both #1 and #2 should happen in > org-babel--associate-session. #1 should probably be discouraged, > and it looks like even for ob-R creating new session is not really > necessary. It looks like ob-R and ob-julia are the only languages that start sessions on edit (based on grepping for "edit-prep" and "associate-session"). I think their behavior is peculiar enough to have an ob-R/julia-specific option on whether to initiate session on edit, with options nil, t, and earmuffs. Earmuffs is the current behavior, but it's surprising enough (IMO) that it might be worth changing the default to nil or t. But still worth keeping the earmuffs option since this behavior seems to go back to the original implementation (30931bfe1). If it helps, I can prepare a patch for this after you've made the changes for org-babel--associate-session. In my notebooks I generally define my ob-R sessions to have earmuffs (like ":session *R:project-name*") so that they can easily work with "M-x R" (which names sessions as such by default). Until now I did not realize this was the culprit for the annoying (and undocumented) startup behavior I was experiencing.
Re: [PATCH] Set Python shell in Org edit buffer
Ihor Radchenko writes: > Now, the question is what to do with the existing implementation of > `org-src-associate-babel-session'. It only runs > org-babel--associate-session when > > (and session (not (string= session "none")) >(org-babel-comint-buffer-livep session) >(let ((f (intern (format "org-babel-%s-associate-session" > (nth 0 info) >(and (fboundp f) (funcall f session > > The questionable check here is (org-babel-comint-buffer-livep session) - > it only triggers when session is already initiated, while ob-python and > some other backends do not necessarily need to start a new session to > "associate" it with Org Src buffer. > > I am tentatively inclined to change this check to > > (or (org-babel-comint-buffer-livep session) > (eq org-src-auto-initiate-session t) > (alist-get (nth 0 info) org-src-auto-initiate-session) > (alist-get 'default org-src-auto-initiate-session)) > > With `org-src-auto-initiate-session' being a customization that controls > whether to associate session for a given babel backend. > > We may set the default value to something like > > ((default . t) ("R" . nil)) I think there are 2 aspects of the session+editing behavior that I'd like to disentangle: 1. Whether to create session when editing (if it doesn't exist yet) 2. If session exists, whether to "associate" it so that evaluation commands (e.g. python-shell-send-region, ess-eval-region) and autocompletion use it. Personally, I prefer to disable #1 while enabling #2. For ob-R, it seems like #1 happens in `org-babel-edit-prep:R' while #2 happens in `org-babel-R-associate-session', so adding an option to disable the latter isn't useful for me, and it's not clear to me whether it'd be useful generally for others either. (I realize #2 hasn't worked properly for some time now until you fixed it in this thread. I wasn't too badly affected because I usually only use one session at a time, and ess-eval-region is able to figure out the session in this case. But I did sometimes have to manually call `ess-force-buffer-current' to get completion working, which it seems I no longer have to do now that you've fixed this). As an aside, I just noticed some inconsistent behavior in ob-R. It seems it only auto-creates the session when ":session *foo*" (i.e. the session name has "earmuffs"). But when ":session foo" (no earmuffs), ob-R doesn't autostart the session. While this is probably accidental, it doesn't seem to cause any problems, which makes me question whether it's really needful for ob-R to initiate a session on edit. In particular, if ":session foo" already exists, then ess-eval-region still works fine in the src block. Which is exactly my desired behavior -- don't create the session if it doesn't exist yet, but if it already exists then play nicely with it. Another thing to note is that ob-R works fine with sessions externally created with "M-x R". (similar to how ob-python works fine with "M-x run-python"). In fact, I sometimes use ob-R with manual "M-x R" sessions when I need to use a different R binary/environment than my usual one. So, it is not necessary for the R session to be started through ob-R. As for ob-python; I think it's best to set `python-shell-buffer-name' in `org-babel-edit-prep:python' rather than `org-babel-python-associate-session'. While both work for `python-shell-send-region' if the session already exists, `org-babel-edit-prep:python' has the advantage that it will run even if the session doesn't exist yet, so then "M-x run-python" in the src block will create a session with the correct name.
Re: [PATCH] Set Python shell in Org edit buffer
Ihor Radchenko writes: >> I agree that `python-shell-buffer-name' should be set according to the >> :session header, and that Liu's patch fixes a problem in ob-python. >> >> Is there any objection if I go ahead and apply it? > > Because I am still thinking about the idea with global customization and > `org-babel--associate-session'. It's great that you're thinking about this -- it would be nice to have better consistency between ob-R, ob-python, etc, and to have better configurability on this. In particular, ob-R's behavior to automatically start a session is annoying for me, especially when editing on my HPC's login node which forbids starting R, python, etc outside of slurm. However, the most recent version of Liu's patch is very small, and the changes should be easy to modify in future, whatever your conclusion on `org-babel--associate-session'. So I would suggest not to let it be blocked by this for too long.
Re: Month-week and quarter-week datetrees (RFC and package announcement)
Ihor Radchenko writes: >> https://gitlab.com/jackkamm/org-grouped-weektree >> >> I'd appreciate feedback on 2 points: >> >> 1. Are any of these datetree formats worth upstreaming into org-mode >>proper? > > That would make sense. > >> 2. Can we add a public interface for `org-datetree--find-create', and >>are there any suggestions on how to do it? >> >> Regarding #2, an ugly aspect of my current implementation is the abuse >> of the private function `org-datetree--find-create'. I also pass in >> the week or quarter for the DAY and MONTH arguments of this function, >> though I note that `org-datetree-find-iso-week-create' does something >> similar in its implementation. > > The API of `org-datetree--find-create' is generally very limiting. > It would be nice to come up with something less limiting. Thanks for the feedback -- I'll start working on something along these lines. Though this might take me a little while since the holiday is ending soon :''-(
Re: [PATCH] Set Python shell in Org edit buffer
Ihor Radchenko writes: > python.el is convenient as it allows setting the session buffer name in > advance via buffer-local variable. Then, all the normal python-mode > commands, including `run-python' or `python-shell-send-region' will work > as expected. However, other babel backends like ob-R do not have such a > luxury (see the dance with renaming R process buffer in > `org-babel-R-initiate-session'). For R specifically, using `ess-gen-proc-buffer-name-function' might simplify things. Though of course this doesn't solve the problem more generally for other languages.
Re: [PATCH] Change default ob-python session command to match run-python
Ihor Radchenko writes: > Thanks! > I have several comments. Thanks for the feedback! I attach a followup patch in response. If all looks good, I will squash before applying. >> -(defcustom org-babel-python-command "python" >> - "Name of the command for executing Python code." >> - :version "24.4" >> - :package-version '(Org . "8.0") >> +(defcustom org-babel-python-command nil >> + "Name of the command for interactive and non-interactive Python code. >> +If set, it overrides `org-babel-python-command-session' and >> +`org-babel-python-command-nonsession'." > > What about default value being 'auto? That would be more explicit. Sure, I have updated `org-babel-python-command(-session)' to use auto as default. > :version tags should not be used when we use :package-version - we drop > them as we change the defcustom that still has them. I've dropped the :version tags for the updated variables. I can also make a separate commit later to drop :version from other ob-python variables like `org-babel-python-hline-to' and `org-babel-python-None-to'. > Unless I miss something, it looks like you also allow command arguments, > not just command name as the value. You may need to adjust the docstring > to reflect this fact. I've updated the docstrings to say they are for the command including arguments. >> (defun org-babel-execute:python (body params) >>"Execute Python BODY according to PARAMS. >> This function is called by `org-babel-execute-src-block'." >> - (let* ((org-babel-python-command >> + (let* ((org-babel-python-command-nonsession >> + (or (cdr (assq :python params)) >> + org-babel-python-command >> + org-babel-python-command-nonsession)) >> + (org-babel-python-command-session >>(or (cdr (assq :python params)) >> - org-babel-python-command)) >> + org-babel-python-command >> + org-babel-python-command-session)) > >> @@ -374,7 +407,7 @@ (defun org-babel-python-evaluate-external-process >> non-nil, then save graphical results to that file instead." >>(let ((raw >> (pcase result-type >> - (`output (org-babel-eval org-babel-python-command >> + (`output (org-babel-eval org-babel-python-command-nonsession >> > > I am slightly concerned about > `org-babel-python-evaluate-external-process' relying upon being called > from `org-babel-execute:python' that let-binds > `org-babel-python-nonsession'. > > Since `org-babel-python-evaluate-external-process' is a public function, > it may also be called independently. And it will not obey > `org-babel-python-command' then. That's a good catch. I've fixed this by reverting `org-babel-execute:python' to set `org-babel-python-command' (not `org-babel-python-command-nonsession'). Then, `org-babel-python-evaluate-external-process' checks both variables to decide what the command is. >From 91352d03c897dc546f57f48a14847cc78e74ec85 Mon Sep 17 00:00:00 2001 From: Jack Kamm Date: Sat, 30 Dec 2023 20:44:46 -0800 Subject: [PATCH 2/2] Incorporate feedback from Ihor; to be squashed before applying --- lisp/ob-python.el | 67 ++- 1 file changed, 37 insertions(+), 30 deletions(-) diff --git a/lisp/ob-python.el b/lisp/ob-python.el index 2b17f41fe..5251c3b33 100644 --- a/lisp/ob-python.el +++ b/lisp/ob-python.el @@ -46,30 +46,27 @@ (defconst org-babel-header-args:python (python . :any)) "Python-specific header arguments.") -(defcustom org-babel-python-command nil - "Name of the command for interactive and non-interactive Python code. -If set, it overrides `org-babel-python-command-session' and -`org-babel-python-command-nonsession'." - :version "29.2" +(defcustom org-babel-python-command 'auto + "Command (including arguments) for interactive and non-interactive Python code. +When not `auto', it overrides `org-babel-python-command-session' +and `org-babel-python-command-nonsession'." :package-version '(Org . "9.7") :group 'org-babel - :type '(choice string (const nil))) + :type '(choice string (const auto))) -(defcustom org-babel-python-command-session nil - "Name of the command for starting interactive Python sessions. -If `nil' (the default), uses the values from +(defcustom org-babel-python-command-session 'auto + "Command (including arguments) for starting interactive Python sessions. +If `auto' (the default), uses the values from `python-shell-interpreter' and `p
Month-week and quarter-week datetrees (RFC and package announcement)
I have started a package that defines org-capture datetrees following year-quarter-week and year-month-week formats: https://gitlab.com/jackkamm/org-grouped-weektree I'd appreciate feedback on 2 points: 1. Are any of these datetree formats worth upstreaming into org-mode proper? 2. Can we add a public interface for `org-datetree--find-create', and are there any suggestions on how to do it? Regarding #2, an ugly aspect of my current implementation is the abuse of the private function `org-datetree--find-create'. I also pass in the week or quarter for the DAY and MONTH arguments of this function, though I note that `org-datetree-find-iso-week-create' does something similar in its implementation.
Re: [PATCH] Set Python shell in Org edit buffer
Liu Hui writes: > But it is indeed possible that two sessions are inconsistent, if users > intend to have different org-babel-python-command and > python-shell-interpreter, which are used by > `org-babel-python-initiate-session' and `run-python', respectively. I have just proposed this patch, which will reduce this discrepancy by having ob-python sessions use the same interpreter as `run-python' by default: https://list.orgmode.org/87edf41yeb@gmail.com/T/#u Additionally, we could set `python-shell-interpreter(-args)' in `org-babel-edit-prep:python' to make `run-python' in Org Src buffer use the same interpreter as :python header or customized `org-babel-python-command'. See the changes to `org-babel-python-initiate-session-by-key' in the linked patch for an example -- specifically the part using `split-string-and-unquote' to get the 2 parts for `python-shell-interpreter' and `python-shell-interpreter-args', respectively.
[PATCH] Change default ob-python session command to match run-python
The attached patch changes the default behavior of ob-python sessions, to respect python-shell-interpreter(-args) when starting an interactive session. It also allows separate customization of the default Python command for nonsessions and sessions. This mainly benefits IPython users. IPython isn't suitable as the default command for nonsessions, but until now there isn't a way to set it as the default interpreter for sessions only. Another benefit is to promote greater consistency between ob-python.el and upstream python.el, in particular for shells started with run-python. If a user configures python-shell-interpreter(-args), then ob-python will respect those settings now. As explained in the NEWS entry, this change should have no effect on users who previously configured `org-babel-python-command', or on users who stick to the default `python-shell-interpreter'. But I submit the patch for review before applying it, because it involves changing the default values of some custom variables. >From a49ddcb6ef72cfefab400e36e6d4a19e869c47a1 Mon Sep 17 00:00:00 2001 From: Jack Kamm Date: Fri, 29 Dec 2023 13:22:18 -0800 Subject: [PATCH] ob-python: Changed options for default Python command ob-python will now use the same settings as `run-python' when starting interactive sessions, by default. * lisp/ob-python.el (org-babel-python-command): Changed to have default value of nil. (org-babel-python-command-session): New option to control default session Python command. (org-babel-python-command-nonsession): New option to control default nonsession Python command. (org-babel-execute:python): Set `org-babel-python-command-session' and `org-babel-python-command-nonsession' using :python header arg. (org-babel-python-initiate-session-by-key): Call `run-python' without CMD arg. Instead, set `python-shell-interpreter' and `python-shell-interpreter-args' when needed. (org-babel-python-evaluate-external-process): Use `org-babel-python-command-nonsession' to start nonsession Python. --- etc/ORG-NEWS | 30 lisp/ob-python.el | 58 --- 2 files changed, 75 insertions(+), 13 deletions(-) diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS index c54473f55..688735a5b 100644 --- a/etc/ORG-NEWS +++ b/etc/ORG-NEWS @@ -551,6 +551,36 @@ Currently implemented options are: The capture template expansion element =%K= creates links using ~org-store-link~, which respects the values of ~org-id-link-to-use-id~. +*** Changes to ~org-babel-python-command~, and new session/nonsession specific options + +The default Python command used by interactive sessions has been +changed to match ~python-shell-interpreter~ and +~python-shell-interpreter-args~ by default. The default Python +command for nonsessions has not changed. + +New options ~org-babel-python-command-nonsession~ and +~org-babel-python-command-session~ control the default Python command +for nonsessions and sessions, respectively. By default, +~org-babel-python-command-session~ is nil, which means to use the +configuration for ~python-shell-interpreter(-args)~ as default. + +The old option ~org-babel-python-command~ has been changed to have +default value of nil. When non-nil, it overrides both +~org-babel-python-command-nonsession~ and +~org-babel-python-command-session~. Therefore, users who had +previously set ~org-babel-python-command~ will not experience any +changes. + +Likewise, users who had neither set ~org-babel-python-command~ nor +~python-shell-interpreter(-args)~ will not see any changes -- ~python~ +remains the default command. + +The main change will be for users who did not configure +~org-babel-python-command~, but did configure +~python-shell-interpreter~, e.g. to use IPython. In this case, +~ob-python~ will now start interactive sessions in a more consistent +manner with ~run-python~. + ** New features *** =ob-plantuml.el=: Support tikz file format output diff --git a/lisp/ob-python.el b/lisp/ob-python.el index 6b3a608c8..2b17f41fe 100644 --- a/lisp/ob-python.el +++ b/lisp/ob-python.el @@ -46,10 +46,31 @@ (defconst org-babel-header-args:python (python . :any)) "Python-specific header arguments.") -(defcustom org-babel-python-command "python" - "Name of the command for executing Python code." - :version "24.4" - :package-version '(Org . "8.0") +(defcustom org-babel-python-command nil + "Name of the command for interactive and non-interactive Python code. +If set, it overrides `org-babel-python-command-session' and +`org-babel-python-command-nonsession'." + :version "29.2" + :package-version '(Org . "9.7") + :group 'org-babel + :type '(choice string (const nil))) + +(defcustom org-babel-python-command-session nil + "Name of the command for starting interactive Python sessions. +If `ni
Re: [PATCH] Set Python shell in Org edit buffer
Ihor Radchenko writes: > As long as it remains undocumented, we can break this in future (maybe > years from now, but still...). Fair enough, I've had to fix this feature from time to time due to breakage in the past. I just pushed d0d838b02 which should hopefully prevent future breakage: https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=d0d838b02e44a40adca14d6eae39fd4c364730da It adds a unit test for this feature, and makes the docstrings for org-babel-python-initiate-session(-by-key) more thorough. It also improves robustness if the source block is executed before run-python finishes initializing. > What might be more robust is to provide an explicit "start session > from Org Src buffer" command for ob-python and re-bind `run-python' to > this command in Org Src buffers. We could refactor `org-babel-python-initiate-session-by-key' to call a separate `org-babel-python-run-python' interactive command that wraps `run-python', and then rebind `run-python' to it in a local minor mode for ob-python Src buffers, which could be started in `org-babel-edit-prep:python'. But I'm not sure if it's worth the hassle, or if d0d838b02 already addresses the concern sufficiently? Note that I'd like ob-python to keep working with run-python, even if it's invoked outside of an Org Src buffer.
Re: [PATCH] Set Python shell in Org edit buffer
Ihor Radchenko writes: > I think we have a misunderstanding here. > > Didn't we just discuss that C-c C-p in python is not equivalent to > `org-babel-python-initiate-session'? ob-python works fine with sessions started externally by `run-python'. And I have preserved this functionality, as I enjoy the flexibility of working this way. In particular, ob-python will detect when it needs to run `org-babel-python--setup-session', even if the session was started externally by `run-python'. `org-babel-python-initiate-session' might be misnamed, which may be causing some confusion here. It is run whenever a Python block is executed, even if the session already exists.
Re: [PATCH] Set Python shell in Org edit buffer
Liu Hui writes: > I just want to set 'python-shell-buffer-name' in the edit buffer > according to the :session header and don't need to start the session > even if the session doesn't exist. Sorry that I missed this thread. I agree that `python-shell-buffer-name' should be set according to the :session header, and that Liu's patch fixes a problem in ob-python. Is there any objection if I go ahead and apply it? I think the question of what to do when a session doesn't exist is a separate issue. Liu's patch doesn't change the status quo on this matter (C-c C-c still tells you to call run-python either way). And I'm not convinced there is a problem with the status quo anyways, though additional customization could be nice.
Re: [BUG] FAILED test-ob-python/session-multiline
Ihor Radchenko writes: >> 1. In addition to printing `org-babel-python-eoe-indicator' after >>execution, we could also print out a "beginning of execution" >>indicator before execution, and then capture the output between the >>beginning and end indicators. This is how the async session >>execution works, and should avoid any possibility of capturing >>prompts. > > This idea looks interesting. Although I would not be so sure that it > will fix things - I have learned that comint has many edge cases we may > not easily anticipate. > > For example, see the discussion in > https://yhetil.org/emacs-devel/87y1tgqhmc.fsf@localhost/ I think this strategy could work better in ob-python than ob-shell because ob-python sends code to a temp file and executes the whole file at once, which should prevent prompts arising between commands. I will probably try this approach next, if the fix I just sent here doesn't work out: https://list.orgmode.org/87h6mrihfg@gmail.com/ >>Alternatively, we could add an argument to >>`python-shell-send-string-no-output' to avoid suppressing output, >>submit it upstream to python.el, and then backport to Org to >>support older emacs versions. > > If we can (eventually) remove some custom code from Org and move it to > Emacs, it will be the best for working towards RMS request > https://orgmode.org/list/e1kiph1-0001lu...@fencepost.gnu.org I started down this path here: https://lists.gnu.org/archive/html/emacs-devel/2023-10/msg4.html But I haven't followed up because I started to have some doubts. In particular, `python-shell-send-string-no-output' will terminate once it detects a prompt, so if some output looks like it ends in a prompt then it will terminate prematurely. Whereas in our current indicator-based approach, the user accidentally emitting `org-babel-python-eoe-indicator' is unlikely. Another approach I have considered is to redirect sys.stdout from within Python. In particular, set it to a custom class inheriting from IOBase during the block's execution, that both prints and saves the output. I think this approach could ultimately be more robust, and without needing to print an ugly indicator token, but it could be complicated to do it right.
Re: [BUG] FAILED test-ob-python/session-multiline
Ihor Radchenko writes: > We have fairly regular CI test failures for one of the ob-python tests. > The test does not fail _every_ time, but I keep seeing the problem in > various Emacs versions, including Emacs 29. > > Example log: https://builds.sr.ht/~bzg/job/1047678#task-build > > In the test the result somehow includes prompt: > > Test test-ob-python/session-multiline condition: > (ert-test-failed > ((should >(equal "20" > (org-test-with-temp-text "#+begin_src python :session :results > output\n foo = 0\n for _ in range(10):\n foo += 1\n\n foo += > 1\n\n print(foo)\n#+end_src" ...))) > :form > (equal "20" ">>> 20") > :value nil :explanation > > > --> (arrays-of-different-length 2 6 "20" ">>> 20" first-mismatch-at 0))) Hello, sorry for the long time to address this. I've just pushed a commit [1] that might address this, based on a new hypothesis I have for the root cause: https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=1eb598758980d5fa4d7bb21c98dfc56f42cae59a Please let me know whether the problem continues, or whether it seems to improve. As an aside -- I am having a hard time figuring out how to monitor our CI for this. When I search in https://lists.sr.ht/~bzg/org-build-failures I can only find an example from 11 months ago. The example you sent (https://builds.sr.ht/~bzg/job/1047678#task-build) is more recent, but is "Unlisted" and doesn't show up when I search for it.
Re: ICalendar export
Ihor Radchenko writes: > Henrik Frisk writes: > >> Recently (not sure when) the ics output came out malformed and a newline is >> omitted between the end of one event and the beginning of another: >> >> END:VEVENT >> BEGIN:VEVENT >> >> is now >> >> END:VEVENTBEGIN:VEVENT >> >> I can't figure out wha the pattern is, for some events the output is >> correct. >> >> This is on Org mode version 9.7-pre release_9.6.8-719-gf299fb and emacs 29.1 > > We recently did some major changes to comply better to the icalendar > specs and introduced new feature. As it usually goes, major changes can > easily introduce new bugs. > > Does the attached patch fix the problem for you? I can reproduce the bug by calling `org-icalendar-export-current-agenda' in my agenda buffer. It seems to mainly happen when subsequent agenda entries originate from different files. Ihor's patch fixes the problem on my end. Bisecting, the bug was introduced in f4446ce79.
Re: [BUG] ob-python hangs on second start [9.5.4 (release_9.5.4-763-g06373a @ ~/emacs/org-mode/lisp/)]
Ihor Radchenko writes: > LGTM! Feel free to push. Pushed to bugfix (c81dba2fb) and merged to main (b49275acb). Pushed the unit test as a separate commit to main only (8000b1120).
Re: [BUG] ob-python hangs on second start [9.5.4 (release_9.5.4-763-g06373a @ ~/emacs/org-mode/lisp/)]
Ihor Radchenko writes: > Peter Mao writes: > >> Expectation: When running ob-python code blocks, I should be able to >> kill the python session in the *Python* buffer and run another code >> block (or the same one). >> >> Problem: ob-python works fine on the first execution, but after >> `exit()`ing the python session, it hangs without executing the code. >> After a `C-g`, the prompt in the *Python* session shows up, but one then >> has to re-execute the code block, as none of it has run. > > Confirmed. > > It looks like `python-shell-first-prompt-hook' does not get triggered > in the described scenario with exit() and we enter infinite loop in the > code below. I think `python-shell-first-prompt-hook' is actually still triggered, but rather it is `org-babel-comint-wait-for-output' that is hanging while waiting for startup. However, we can just replace it with a `sleep-for' instead, because we now wait for `org-babel-python--initialized' anyways. See the attached patch. Please let me know if it fixes the problem for you. >From d5721aa37a399afcd527906e5d9f1b6bce37fdb9 Mon Sep 17 00:00:00 2001 From: Jack Kamm Date: Sun, 27 Aug 2023 14:13:15 -0700 Subject: [PATCH] ob-python: Fix hanging on second start See: https://list.orgmode.org/87ttsnh5bx.fsf@localhost/T/#t * lisp/ob-python.el (org-babel-python-initiate-session-by-key): Switch from `org-babel-comint-wait-for-output' to `sleep-for' while waiting for `org-babel-python--initialized', to prevent hanging on restarted Python process. * testing/lisp/test-ob-python.el (test-ob-python/session-restart): New test for restarting ob-python session process. --- lisp/ob-python.el | 2 +- testing/lisp/test-ob-python.el | 19 +++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/lisp/ob-python.el b/lisp/ob-python.el index b9dab91b4..6b216ce89 100644 --- a/lisp/ob-python.el +++ b/lisp/ob-python.el @@ -272,7 +272,7 @@ (defun org-babel-python-initiate-session-by-key (&optional session) ;; multiple prompts during initialization. (with-current-buffer py-buffer (while (not org-babel-python--initialized) - (org-babel-comint-wait-for-output py-buffer))) + (sleep-for 0 10))) (setq org-babel-python-buffers (cons (cons session py-buffer) (assq-delete-all session org-babel-python-buffers))) diff --git a/testing/lisp/test-ob-python.el b/testing/lisp/test-ob-python.el index 82fbca36e..c11e1d0c2 100644 --- a/testing/lisp/test-ob-python.el +++ b/testing/lisp/test-ob-python.el @@ -310,6 +310,25 @@ (ert-deftest test-ob-python/async-local-python-shell () #+end_src" (should (org-babel-execute-src-block +(ert-deftest test-ob-python/session-restart () + ;; Disable the test on older Emacs as built-in python.el sometimes + ;; fail to initialize session. + (skip-unless (version<= "28" emacs-version)) + (should + (equal "success" + (progn +(org-test-with-temp-text "#+begin_src python :session :results output +print('start') +#+end_src" + (org-babel-execute-src-block)) +(let ((proc (python-shell-get-process))) + (python-shell-send-string "exit()") + (while (accept-process-output proc))) +(org-test-with-temp-text "#+begin_src python :session :results output +print('success') +#+end_src" + (org-babel-execute-src-block)) + (provide 'test-ob-python) ;;; test-ob-python.el ends here -- 2.41.0
Re: [BUG] FAILED test-ob-python/session-multiline
Ihor Radchenko writes: > Jack Kamm writes: > >>>FAILED 376/1256 test-ob-python/session-multiline (0.011955 sec) at >>> ../lisp/test-ob-python.el:105 >> >> Hmmm. Do you have an idea of how long this has been happening, and how >> frequently it breaks? > > For months. > >> My first suspicion is the large ob-python commit I pushed on Tuesday: >> >> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=579e8c572345c42ad581d3ddf0f484567d55a787 > > So, should not be the recent commit. This one might take some time to fix, since it's hard to reproduce and I'm not sure the cause of it. But here are 3 different solutions I am considering now: 1. In addition to printing `org-babel-python-eoe-indicator' after execution, we could also print out a "beginning of execution" indicator before execution, and then capture the output between the beginning and end indicators. This is how the async session execution works, and should avoid any possibility of capturing prompts. 2. Instead of relying on our own custom `org-babel-python-send-string', we could try switching to python.el's `python-shell-send-string-no-output', which is probably more robust. This would also allow removing the ugly `org-babel-python-eoe-indicator' we currently print. Downside is that the output would not be echoed into the session anymore. To fix that, we could manually insert the captured output into the comint session buffer after execution. Alternatively, we could add an argument to `python-shell-send-string-no-output' to avoid suppressing output, submit it upstream to python.el, and then backport to Org to support older emacs versions. 3. Revisit a series of commits I made in 2020, which was supposed to make session evaluation more robust, and was inspired by `python-shell-send-string-no-output': https://git.sr.ht/~bzg/org-mode/commit/4df12ea39 However, I had to partially revert that work, due to compatibility issue with emacs 26.3: https://list.orgmode.org/871rjcan53@kyleam.com/ I think we no longer support emacs 26.3, so I could potentially revisit this now -- but it's been a few years and will take some time to refresh my memory about this. Of these options, Option 1 is the easiest, and the most certain to solve this bug. Options 2 and 3 are more difficult and riskier, but would have other benefits if they work: we can remove the ugly `org-babel-python-eoe-indicator' that is currently printed to the session, and/or reduce long-term maintenance burden by relying on python.el's implementation for capturing output.
Re: [BUG] FAILED test-ob-python/session-multiline
Ihor Radchenko writes: > Hi, > > We have fairly regular CI test failures for one of the ob-python tests. > The test does not fail _every_ time, but I keep seeing the problem in > various Emacs versions, including Emacs 29. > > Example log: https://builds.sr.ht/~bzg/job/1047678#task-build > > In the test the result somehow includes prompt: > > Test test-ob-python/session-multiline condition: > (ert-test-failed > ((should >(equal "20" > (org-test-with-temp-text "#+begin_src python :session :results > output\n foo = 0\n for _ in range(10):\n foo += 1\n\n foo += > 1\n\n print(foo)\n#+end_src" ...))) > :form > (equal "20" ">>> 20") > :value nil :explanation > > > --> (arrays-of-different-length 2 6 "20" ">>> 20" first-mismatch-at 0))) > > >FAILED 376/1256 test-ob-python/session-multiline (0.011955 sec) at > ../lisp/test-ob-python.el:105 Hmmm. Do you have an idea of how long this has been happening, and how frequently it breaks? My first suspicion is the large ob-python commit I pushed on Tuesday: https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=579e8c572345c42ad581d3ddf0f484567d55a787 But that commit shouldn't affect ":results output", at least not intentionally. Also, do you have any tips for searching or navigating the failing CI builds? I tried going to https://builds.sr.ht/~bzg, but it didn't include the failed build you linked to (which I guess is an "unlisted" build).
Re: [PATCH] ob-python results handling for dicts, dataframes, arrays, and plots
Jack Kamm writes: > Liu Hui writes: > >> I think these objects need to be shown in a single column rather than >> two. Besides, if the python code becomes too complex finally, I think >> maintaining the python code outside the ob-python.el, as suggested by >> Ihor, is a good idea. > > Thanks for reporting these misbehaving examples. I think the root of the > problem is `org-babel-script-escape', which is too aggressive in > recursively converting strings to lists. We may need to rewrite our own > implementation for ob-python. > > Also, I agree that moving the python code to an external file will be > helpful in handling these more complex cases. > > I may leave these tasks for future patches. In the meantime, we may have > to recommend ":results verbatim" for these more complex cases that > ":results table" doesn't fully handle yet. Pushed the patch now, with one final change: I decided to leave dict as string by default, converting to table only when ":results table" is explicitly set. I think it's better this way for now, because of the misbehaving examples you pointed out -- table conversion is not yet fully robust for complex dict's containing complicated objects or structures.
Re: [PATCH] ob-python results handling for dicts, dataframes, arrays, and plots
Ihor Radchenko writes: > +1 > Don't forget to update > https://orgmode.org/worg/org-contrib/babel/languages/ob-doc-python.html > (note how the docs already have an example of org formatting from python) Thanks! Done now: https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=579e8c572345c42ad581d3ddf0f484567d55a787 And updated Worg as well: https://git.sr.ht/~bzg/worg/commit/7c7d352be72271ae73f31ddffa0f48d225b34259
Re: [PATCH] ob-python results handling for dicts, dataframes, arrays, and plots
Liu Hui writes: > I think these objects need to be shown in a single column rather than > two. Besides, if the python code becomes too complex finally, I think > maintaining the python code outside the ob-python.el, as suggested by > Ihor, is a good idea. Thanks for reporting these misbehaving examples. I think the root of the problem is `org-babel-script-escape', which is too aggressive in recursively converting strings to lists. We may need to rewrite our own implementation for ob-python. Also, I agree that moving the python code to an external file will be helpful in handling these more complex cases. I may leave these tasks for future patches. In the meantime, we may have to recommend ":results verbatim" for these more complex cases that ":results table" doesn't fully handle yet.
Re: [PATCH] ob-python results handling for dicts, dataframes, arrays, and plots
Ihor Radchenko writes: > We might add the code into a separate proper python file. Then, we can > use the contents of that file to retrieve the variable value. > > We already do the same thing for CSL style files and odt schema/style. Thanks, I think this is a good idea, and will make the python code easier to maintain. And thanks also for the pointer to oc-csl and ox-odt -- I think I should be able to implement this by following their example. It seems like there will be an extra logistical step, to make sure the extra python file is added to emacs as well. I'm not familiar with the details of how we sync Org into Emacs, but will start to look into it. In the meantime, I'm thinking to squash and apply my patch as is. Then afterwards, I can start working on a followup patch to move some Python code into a separate file (and coordinate with emacs-devel if necessary).
Re: [PATCH] ob-python results handling for dicts, dataframes, arrays, and plots
Ihor Radchenko writes: > Similar to the existing LaTeX formatters, one may write a Python package > that will pretty-print Org markup as text. This sounds interesting -- are these LaTeX formatters external to Org? Could you provide a link/reference?
Re: [PATCH] ob-python results handling for dicts, dataframes, arrays, and plots
gerard.vermeu...@posteo.net writes: > I do not know how much this "abuse" of defconst is frowned > upon (elisp manual says defconst is advisory), but maybe it > can be advertised as a feature. org-babel-python--def-format-value is a "private" variable (it has double dash "--" in its name). Therefore it's not generally recommended to modify it. Of course, elisp doesn't have true private variables or functions, and you are free to change things as you wish -- this is one of the perks of Emacs :) But you've been warned, since this is a private variable, we make no guarantees, and may break things in backward-incompatible ways in the future. As to the broader point, I agree there are many more features that would be nice to add ob-python results handling. But making ob-python too complex will be difficult to maintain, especially since the Python code is all in quoted strings without proper linting. So I am thinking now about how we could make this more extensible in future. One idea is to create a Python package for interfacing with Org Babel, and release it on PyPi. If we detect the package is installed, then we can delegate to it for results formatting. And the community could contribute results handling for all sorts of Python objects to that package. That is just one idea for improving extensibility -- I'm not sure it's the best, and am open to other suggestions as well.
Re: [PATCH] ob-python results handling for dicts, dataframes, arrays, and plots
Ihor Radchenko writes: > This is an ORG-NEWS entry for Version 9.4. Is it an intentional change? Sorry, that was an accident. I've reverted it now: https://github.com/jackkamm/org-mode/commit/f12a695d67bc5c06013d9fbe0af844c9739e347a >> @@ -142,7 +144,9 @@ (defun org-babel-python-table-or-string (results) >>"Convert RESULTS into an appropriate elisp value. >> If the results look like a list or tuple, then convert them into an >> Emacs-lisp table, otherwise return the results as a string." >> - (let ((res (org-babel-script-escape results))) >> + (let ((res (if (string-equal "{" (substring results 0 1)) >> + results ;don't covert dicts to elisp >> + (org-babel-script-escape results > > You may also need to update the docstring for > `org-babel-python-table-or-string' after this change. That change got reverted in subsequent update when I changed dict to return as table by default instead of string. So there's no need to update the docstring anymore. >> -body))) >> - (`value (let ((tmp-file (org-babel-temp-file "python-"))) >> +(if graphics-file >> +(format >> org-babel-python--output-graphics-wrapper >> +body graphics-file) >> + body >> + (`value (let ((results-file (or graphics-file >> + (org-babel-temp-file "python-" > > What about :results graphics file ? Not entirely sure what you mean here. When ":results graphics file", then graphics-file will be non-nil -- org-babel-execute:python passes graphics-file onto org-babel-python-evaluate and then org-babel-python-evaluate-external-process. In case of ":results graphics file output", org-babel-python--output-graphics-wrapper is used to save pyplot.gcf(). Or if ":results graphics file value", then org-babel-python--def-format-value saves the result with Figure.savefig().
Re: [PATCH] ob-python results handling for dicts, dataframes, arrays, and plots
Liu Hui writes: > Hi, > > Thank you for the patch! Thanks for your feedback, I've incorporated it into https://github.com/jackkamm/org-mode/tree/python-results-revisited-2023 More specifically, here: https://github.com/jackkamm/org-mode/commit/af1d18314073446045395ff7a3d1de0303e92586 > Do we need to limit the table/list size by default, or handle them > only with relevant result type (e.g. `table/list')? Dataframe/array > are often large. I've updated the patch so that Dataframe/Array are converted to table only when ":results table" is explicitly set now. If ":results table" is not set, they will be returned as string by default. So code blocks that return large dataframes/arrays can continue to be safely run. Note I did make an additional change to Numpy array default behavior: Previously, numpy arrays would be returned as table, but get mangled when they were very large, e.g.: #+begin_src python import numpy as np return np.zeros((30,40)) #+end_src #+RESULTS: | (0 0 0 ... 0 0 0) | (0 0 0 ... 0 0 0) | (0 0 0 ... 0 0 0) | ... | (0 0 0 ... 0 0 0) | (0 0 0 ... 0 0 0) | (0 0 0 ... 0 0 0) | But now, Numpy array is returned in string form by default, in the same format as in Jupyter: #+begin_src python import numpy as np return np.zeros((30,40)) #+end_src #+RESULTS: : array([[0., 0., 0., ..., 0., 0., 0.], :[0., 0., 0., ..., 0., 0., 0.], :[0., 0., 0., ..., 0., 0., 0.], :..., :[0., 0., 0., ..., 0., 0., 0.], :[0., 0., 0., ..., 0., 0., 0.], :[0., 0., 0., ..., 0., 0., 0.]]) >> +if isinstance(result, pandas.DataFrame): >> +result = [[''] + list(result.columns), None] + \ > > Here we can use '{}'.format(df.index.name) to show the name of index Patch has been updated to print the index name when it is non-None. > Maybe `org-babel-python--def-format-value' can be evaluated only once > in the session mode? It would shorten the string sent to the python > shell, where temp files are used for long strings. Patch has been updated to evaluate `org-babel-python--def-format-value' once per session.
Re: [PATCH] ob-python results handling for dicts, dataframes, arrays, and plots
Ihor Radchenko writes: >>> #+begin_src python :results list >>>return {"a": 1, "b": 2} >>> #+end_src >>> >>> #+RESULTS: >>> - a :: 1 >>> - b :: 2 >> >> This seems harder, and may require more widespread changes beyond >> ob-python. In particular, I think we'd need to change >> `org-babel-insert-result' so that it can call `org-list-to-org' with a >> list of type "descriptive" instead of "unordered" here: > > Actually, (org-list-to-org '(unordered ("a :: b") ("c :: d"))) > will just work. > > We do not support nested lists when transforming output anyway. So, > unordered/descriptive does not matter in practice. You're right, thanks for the suggestion. I've added it now to https://github.com/jackkamm/org-mode/tree/python-results-revisited-2023 More specifically, here: https://github.com/jackkamm/org-mode/commit/0440caa3326b867a3a15d5f92a6f99cbf94c14d5
Re: [PATCH] ob-python results handling for dicts, dataframes, arrays, and plots
Ihor Radchenko writes: > What about > > #+begin_src python :results table >return {"a": 1, "b": 2} > #+end_src > > #+RESULTS: > | a | 1 | > | b | 2 | I attach a 2nd patch implementing this. It also makes ":results table" the default return type for dict. (Use ":results verbatim" to get the dict as a string instead). I am also putting a branch with these changes here: https://github.com/jackkamm/org-mode/tree/python-results-revisited-2023 > > or > > #+begin_src python :results list >return {"a": 1, "b": 2} > #+end_src > > #+RESULTS: > - a :: 1 > - b :: 2 This seems harder, and may require more widespread changes beyond ob-python. In particular, I think we'd need to change `org-babel-insert-result' so that it can call `org-list-to-org' with a list of type "descriptive" instead of "unordered" here: https://git.sr.ht/~bzg/org-mode/tree/cc435cba71a99ee7b12676be3b6e1211a9cb7285/item/lisp/ob-core.el#L2535 >From c24d2eeb3b8613df9b9c23583a4b26a6c0934931 Mon Sep 17 00:00:00 2001 From: Jack Kamm Date: Wed, 16 Aug 2023 20:27:10 -0700 Subject: [PATCH 2/2] ob-python: Convert dicts to tables This commit to be squashed with its parent before applying --- etc/ORG-NEWS | 8 +++- lisp/ob-python.el | 12 +--- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS index 2630554ae..509011737 100644 --- a/etc/ORG-NEWS +++ b/etc/ORG-NEWS @@ -578,11 +578,9 @@ tested property is actually present. *** =ob-python.el=: Support for more result types and plotting -=ob-python= now recognizes numpy arrays, and pandas dataframes/series, -and will convert them to org-mode tables when appropriate. - -In addition, dict results are now returned in appropriate string form, -instead of being mangled as they were previously. +=ob-python= now recognizes dictionaries, numpy arrays, and pandas +dataframes/series, and will convert them to org-mode tables when +appropriate. When the header argument =:results graphics= is set, =ob-python= will use matplotlib to save graphics. The behavior depends on whether value diff --git a/lisp/ob-python.el b/lisp/ob-python.el index 35a82afc0..3d987da2f 100644 --- a/lisp/ob-python.el +++ b/lisp/ob-python.el @@ -144,9 +144,7 @@ (defun org-babel-python-table-or-string (results) "Convert RESULTS into an appropriate elisp value. If the results look like a list or tuple, then convert them into an Emacs-lisp table, otherwise return the results as a string." - (let ((res (if (string-equal "{" (substring results 0 1)) - results ;don't covert dicts to elisp - (org-babel-script-escape results + (let ((res (org-babel-script-escape results))) (if (listp res) (mapcar (lambda (el) (if (eq el 'None) org-babel-python-None-to el)) @@ -242,6 +240,14 @@ (defconst org-babel-python--def-format-value "\ else: if not set(result_params).intersection(\ ['scalar', 'verbatim', 'raw']): +def dict2table(res): +if isinstance(res, dict): +return [(k, dict2table(v)) for k, v in res.items()] +elif isinstance(res, list) or isinstance(res, tuple): +return [dict2table(x) for x in res] +else: +return res +result = dict2table(result) try: import pandas except ImportError: -- 2.41.0
[PATCH] ob-python results handling for dicts, dataframes, arrays, and plots
Following up on a discussion from last month [1], I am reviving my proposal from a couple years ago [2] to improve ob-python results handling. Since it's a relatively large change, I am sending it to the list for review before applying the patch. The patch changes how ob-python handles the following types of results: - Dictionaries - Numpy arrays - Pandas dataframes and series - Matplotlib figures Starting with dicts: these are no longer mangled. The current behavior (before patch) is like so: #+begin_src python return {"a": 1, "b": 2} #+end_src #+RESULTS: | a | : | 1 | b | : | 2 | But after the patch they appear like so: #+begin_src python return {"a": 1, "b": 2} #+end_src #+RESULTS: : {'a': 1, 'b': 2} Next, for numpy arrays and pandas dataframes/series: these are converted to tables, for example: #+begin_src python import pandas as pd import numpy as np return pd.DataFrame(np.array([[1,2,3],[4,5,6]]), columns=['a','b','c']) #+end_src #+RESULTS: | | a | b | c | |---+---+---+---| | 0 | 1 | 2 | 3 | | 1 | 4 | 5 | 6 | To avoid conversion, you can specify "raw", "verbatim", "scalar", or "output" in the ":results" header argument. Finally, for plots: ob-python now supports ":results graphics" header arg. The behavior depends on whether using output or value results. For output results, the current figure (pyplot.gcf) is cleared before evaluating, then the result saved. For value results, the block is expected to return a matplotlib Figure, which is saved. To set the figure size, do it from within Python. Here is an example of how to plot: #+begin_src python :results output graphics file :file boxplot.svg import matplotlib.pyplot as plt import seaborn as sns plt.figure(figsize=(5, 5)) tips = sns.load_dataset("tips") sns.boxplot(x="day", y="tip", data=tips) #+end_src Compared to the original version of this patch [2], I tried to simplify and streamline things as much as possible, since this is a relatively large and complex change. For example, the handling for dict objects is much more simplistic now. And there are other miscellaneous changes to the code structure which I hope improve the clarity a bit. [1] https://list.orgmode.org/caoqtw-n9re7fdrm1apmo8x5lrzmjfn_zjht3rvaf4x+s5m_...@mail.gmail.com/ [2] https://list.orgmode.org/87eenpfe77@gmail.com/ >From 468eeaa69660a18d8b0503e5a68c275301d6e6ae Mon Sep 17 00:00:00 2001 From: Jack Kamm Date: Mon, 7 Sep 2020 09:58:30 -0700 Subject: [PATCH] ob-python: Results handling for dicts, dataframes, arrays, plots * lisp/ob-python.el (org-babel-execute:python): Parse graphics-file from params, and pass it to `org-babel-python-evaluate'. (org-babel-python-table-or-string): Prevent `org-babel-script-escape' from mangling dict results. (org-babel-python--def-format-value): Python code for formatting value results before returning. (org-babel-python-wrapper-method): Removed. Instead use part of the string directly in `org-babel-python-evaluate-external-process'. (org-babel-python-pp-wrapper-method): Removed. Pretty printing is now handled by `org-babel-python--def-format-value'. (org-babel-python--output-graphics-wrapper): New constant. Python code to save graphical output. (org-babel-python--exec-tmpfile): Removed. Instead use the raw string directly in `org-babel-python-evaluate-session'. (org-babel-python--def-format-value): New constant. Python function to format and save value results to file. Includes handling for graphics, dataframes, and arrays. (org-babel-python-format-session-value): Updated to use `org-babel-python--def-format-value' for formatting value result. (org-babel-python-evaluate): New parameter graphics-file. Pass graphics-file onto downstream helper functions. (org-babel-python-evaluate-external-process): New parameter graphics-file. Use `org-babel-python--output-graphics-wrapper' for graphical output. For value result, use `org-babel-python--def-format-value'. (org-babel-python-evaluate-session): New parameter graphics-file. Use `org-babel-python--output-graphics-wrapper' for graphical output. Replace the removed constant `org-babel-python--exec-tmpfile' with the string directly. Rename local variable tmp-results-file to results-file, which may take the value of graphics-file when provided. (org-babel-python-async-evaluate-session): New parameter graphics-file. Use `org-babel-python--output-graphics-wrapper' for graphical output. Rename local variable tmp-results-file to results-file, which may take the value of graphics-file when provided. --- etc/ORG-NEWS | 19 +- lisp/ob-python.el | 164 -- 2 files changed, 119 insertions(+), 64 deletions(-) diff --git a/etc/ORG-NEWS b/etc/OR
Re: [PATCH] ob-python: support header argument `:results file graphics'
Ihor Radchenko writes: > We may instead arrange org-lint and possibly ob-core to throw a > warning when an src block uses confusing setting combinations. > Without changing the underlying behaviour. > Basically, discourage using confusing staff. > ... > We should update the docs to avoid such examples. > ... > We should generally rewrite that part of the manual, I think. > My previous message was a tentative outline on how the things should be > presented in the manual. > ... >> IMO the more technically correct approach is in the ob-python patch >> that I proposed a couple years ago [2], and plan to revisit soon. In >> that patch, ob-python ":results graphics output" will plot from >> pyplot.gcf(), while ":results graphics value" will expect a >> matplotlib.Figure object to be returned for plotting. > > Sounds reasonable. Let me know if you need any help along the way. Thank you. And likewise, I agree with your suggestions to update org-lint, org-manual, and Worg, and will try to help where I can. (I might be a little slow to start due to other deadlines the next couple weeks).
Re: [PATCH] ob-python: Fix async evaluation
Liu Hui writes: > Thanks for pointing out the problem! I find the problem disappears > after removing the `run-python` line, and I have updated the patch > accordingly. Thank you. I applied your patch to main branch. I am curious why the previous version of your test was causing hanging -- whether it indicates a bug in ob-python, or just an issue with the testing. Would be good to look into it more...
Re: [PATCH] ob-python: Fix async evaluation
Liu Hui writes: > OK, I have added a test to the patch. While your test works on its own, it seems to break subsequent tests (the next test hangs). My guess is that it has something to do with the fact that most of the Python session tests share the same session, and ob-python is getting confused about which session to use on the next test. One possible fix might be to kill the Python session started by your new test, after it's finished running. (It is probably not the best design that the Python session tests all share the same session...)
Re: [PATCH] ob-python: support header argument `:results file graphics'
Ihor Radchenko writes: > ":results file" imply that results of the code block are written to a file > (the file is specified using header args). > > ":results file link" imply that results of the code block are interpreted > as file link. The fact that presence of :file header arg overrides this > behaviour is something we may want to reconsider - it is confusing. I think this is a lot clearer and more intuitive than the current behavior, and worth doing. But it is a breaking change, so it would be good to provide a variable to re-enable the previous behavior, for back-compatibility of older Org documents. In particular, the Worg matplotlib example of ":results file" without ":file" header arg is fairly old, and I have a few Org docs using ":results file" this way as well. So I would appreciate a backwards-compatibility variable if we change this. > ":results graphics file" imply that graphics generated during code > block execution is saved to file specified in the :file header args. > This feature is only available for some backends that can derive > graphics data from the source block. When :file is not specified, > using the actual code block output is confusing, and we may want to > reconsider this behaviour. I agree. On a related note, I think we should revert most of b088389c6 on bugfix branch. That documentation causes more harm than good, and is based on some confusion in [1] ("graphics" and "link" are _not_ equivalent in general). > Sorry, but I do not fully understand. > Generated graphics is not what Org sees as "results of evaluation". > I think it is well illustrated by > > #+begin_src R :file img.png > hist(rnorm(100)) > "img.png is going to contain this string." > #+end_src > > #+begin_src R :file img.png :results graphics > hist(rnorm(100)) > "But now img.png is going to contain graphics." > #+end_src > > The latter has nothing to do with block output, which is a string. IMO block _value_ is string, while block _output_ is string AND graphics. So by my interpretation, ob-R is slightly incorrect in how it handles ":results output graphics" vs ":results value graphics". IMO the more technically correct approach is in the ob-python patch that I proposed a couple years ago [2], and plan to revisit soon. In that patch, ob-python ":results graphics output" will plot from pyplot.gcf(), while ":results graphics value" will expect a matplotlib.Figure object to be returned for plotting. (Note I do _not_ suggest changing ob-R -- even if my interpretation is correct, I think that common usage and backwards-compatibility outweighs strict technical correctness in this case). [1] https://list.orgmode.org/87fu41zcn2@nicolasgoaziou.fr/ [2] https://list.orgmode.org/87eenpfe77@gmail.com/
Re: [PATCH] ob-python: Fix async evaluation
Ihor Radchenko writes: > Liu Hui writes: > >> To reproduce the bug: >> >> 1. create test.org: >> ──✀── >> #+begin_src python :session "*Python 3*" :async t >> 1 >> #+end_src >> >> # Local Variables: >> # python-shell-buffer-name: "Python 3" >> # End: >> ──✀── >> >> 2. emacs -Q -L --eval "(require 'ob-python)" >> >> 3. Open test.org, then start a python shell with M-x run-python, which >> should create a buffer named "*Python 3*" >> >> 4. Press C-c C-c on the src block. Then an error "No inferior Python >> process running" is shown. > > Confirmed. > CCing Jack Kamm, the maintainer. And I can confirm now as well. Thanks for reporting, and for the patch. The patch looks good, but it would be nice to include a unit test as well -- could you update the patch to include one, Liu Hui? Thanks, Jack
Re: [PATCH] ob-python: support header argument `:results file graphics'
Liu Hui writes: > I think your proposal about ":results graphics" is more flexible and > complies the documentation. Since the patch has no real problem and > the feature is useful indeed, I hope it can be merged instead of mine > after the problem of documentation is resolved. Thanks for your gracious words. I will aim to revisit and finish this work in a month or so -- I'm swamped the next couple weeks, and anyways we first need to sort out the issues around documentation, back-compatibility, and general semantics of ":results graphics". > As for other features in the patch, maybe it is better to convert > dict/dataframe/array to table/list only when the result type is > explicitly set to table or list? Thanks, that is an interesting idea. Especially for dict, which can be interpreted in many ways, it is a good idea to make use of the table/list/etc type specifications.
Re: [PATCH] ob-python: support header argument `:results file graphics'
Ihor Radchenko writes: > Your patch appear to only add more confusion, IMHO. > > I feel that the description about :results file is confusing from the > very beginning: Well, I guess ":results file" has confusing behavior. So it's difficult to write accurate, comprehensive, non-confusing documentation for it ;) > :results file may currently imply three things: > > 1. Results of evaluation are the _contents_ of a file > 2. Results of evaluation are the path to a file > 3. Results of evaluation are discarded and Org just inserts a constant >link, derived from header arguments. > > (3) is used for :results file graphics/:results file link > (2) is used when Org is unable to deduce the file name from > :file/:file-ext+#+name > (1) is used when the file name can be deduced from src block params Laying out the 3 behaviors this way seems clearer. But I disagree that ":results graphics" means (3). It can behave as (1) or (3), depending on the language. In practice (1) is the more common usage by far [*], and is also the original intended use case [**]. (3) is just what happens when a Babel language doesn't implement graphics (because then Org doesn't know how to save the graphical results [***]). If a Babel language doesn't implement graphics handling, the user can workaround it by manually saving the plot. But this is just a workaround, and we should discourage doing this with ":results graphics", because it causes problems later if the language wants to implement graphics support. Instead, ":results link" is the correct usage for this case -- it always behaves as (3). It is a mistake for the Org manual to define ":results link" and ":results graphics" as equivalent, because it is counter to the common usage and understanding of ":results graphics", and because functions like `org-babel-graphical-output-file' behave differently with ":results graphics". And what is the benefit of having 2 header arguments with the same meaning? It is much more useful to define ":results graphics" for returning graphical output -- and indeed, that is how ":results graphics" is generally used in practice. [*] For example, nearly every code block in Worg with ":results graphics" behaves as (1). Checking "grep results.\*graphics", I found that 15 out of 16 code blocks behaved this way, inserting the graphical results to file. The lone exception was in ob-doc-clojure.org. [**] ":results graphics" was originally created to implement (1) for ob-R graphical results. See org commit 6bcbdce94, worg commit 8c49402c, and Version 7.5 Release Notes in worg/org-release-notes.org. Also, see the Org Babel paper, which uses ":results graphics" with behavior (1), not (3): https://www.jstatsoft.org/article/view/v046i03 [***] Specifically, `org-babel-execute-src-block' doesn't know anything about graphics, and needs to delegate to language-specific implementation for that.
Re: [PATCH] ob-python: support header argument `:results file graphics'
Liu Hui writes: > I don't think so. Some users may want to keep the figure between > blocks, and they can always clear the figure themselves when > necessary. I'd rather not have to call pyplot.gcf().clear() every time, it doesn't seem nice. ob-R doesn't require manually clearing the plot, and neither do Jupyter notebooks. I would propose the following instead: for ":results output graphics", ob-python should plot the gcf, and clear it beforehand. But for ":results value graphics", the ob-python block should return a matplotlib Figure object to plot, which would allow keeping and modifying a Figure between blocks. I actually proposed that behavior before in this patch: https://list.orgmode.org/87eenpfe77@gmail.com/ But never wound up applying it -- the patch was rather large, with a lot of extra features, and I wasn't sure they were all worth the extra complexity. Then life got in the way, and I never got around to revisiting ob-python plotting, until now. > BTW, I have updated the patch to turn off the feature by default, > since it may break existing src blocks using `graphics'. WDYT? Thanks. I don't think the defcustom is necessary at this point. The feature makes ob-python consistent with similar Babel languages like ob-R, and I can't think of any existing use case of ob-python with ":results graphics" that this would break. In case I missed some obscure edge case, we can ask forgiveness and redirect the user to ":results link" instead. The defcustom might be useful in the future in case we want to support a non-matplotlib plotting system (such as ete3), but that is relatively rare and I think it's better to keep it simple for now.
Re: [PATCH] ob-python: support header argument `:results file graphics'
Ihor Radchenko writes: > Jack Kamm writes: > >> I think the Worg documentation is accurately describing the behavior of >> ob-python -- it's just that ob-python uses ":results file" in a >> nonstandard way. > > May you please point me to the place in ob-python where :results file is > specially treated? ob-python doesn't treat :results file specially. So "nonstandard" may not be the right term. In fact, :results file also works this way for other Babel languages. And I used this behavior before for plotting with ob-reticulate blocks. I attach a patch to fix the documentation in the manual about this. As an aside: I would like ":results graphics" to partially revert its old behavior before Org 9.3. Prior to then, ob-R could generate a plot with :results graphics :file filename.png but since commit 26ed66b23, we require the more verbose :results graphics file :file filename.png which seems unnecessarily verbose (since ":results graphics" doesn't do anything without ":results file"), and also annoyingly broke many of my Org documents before 2020. I think it would be better if ":results graphics" was equivalent to ":results graphics file", and may propose a patch for this in future. >From 538c464ca88c8a1646a1b80352f0c8fb9f114f08 Mon Sep 17 00:00:00 2001 From: Jack Kamm Date: Wed, 5 Jul 2023 19:02:25 -0700 Subject: [PATCH] doc/org-manual: Clarify undocumented uses of :results file (Type): (Format): Document that :results file is using the source block result as file path when :file header argument is not present. Document that some Babel languages require :results graphics for plotting. --- doc/org-manual.org | 37 ++--- 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/doc/org-manual.org b/doc/org-manual.org index 71ad4d9e8..3bf1c7d7f 100644 --- a/doc/org-manual.org +++ b/doc/org-manual.org @@ -18558,11 +18558,7 @@ described in the documentation for individual languages. See #+cindex: @samp{file-ext}, header argument If =file= header argument is missing, Org generates the base name of the output file from the name of the code block, and its extension - from the =file-ext= header argument. In that case, both the name - and the extension are mandatory. - - Result can also be interpreted as path to file. See =:results - link=. + from the =file-ext= header argument. #+begin_example ,#+name: circle @@ -18572,6 +18568,30 @@ described in the documentation for individual languages. See ,#+END_SRC #+end_example + If both =file= and =file-ext= header arguments are missing, then + result is interpreted as path to file. + + #+begin_example + ,#+BEGIN_SRC python :results file + import matplotlib.pyplot as plt + import numpy as np + + fname = "path/to/file.png" + + plt.plot(np.arange(5)) + plt.savefig(fname) + + return fname # return filename to org-mode + ,#+END_SRC + #+end_example + + When =file= or =file-ext= header arguments are present, you can + prevent Org from directly writing to that file by using =:results + link= or =:results graphics=. This might be desirable if you write + to the file within the code block itself. Some Babel languages also + require these extra header arguments for plotting. See =:results + link= for more details. + #+cindex: @samp{file-desc}, header argument The =file-desc= header argument defines the description (see [[*Link Format]]) for the link. If =file-desc= is present but has no value, @@ -18644,8 +18664,11 @@ follows from the type specified above. [[file:org-mode-unicorn.svg]] #+end_example - If =:file= header argument is omitted, interpret source block result - as the file path. + =:results file graphics= is also needed by some languages for + plotting, such as ob-R, ob-julia, and ob-octave, because they save + plots to file via wrapper code in their respective languages rather + than via elisp. Consult the documentation for the respective Babel + languages for more details. - =org= :: -- 2.41.0
Re: [PATCH] ob-python: support header argument `:results file graphics'
Ihor Radchenko writes: > WORG documentation is not accurate here. > > If the block has :file parameter _or_ :file-exp parameter and #+name, > the results of evaluation are inserted into a file. Otherwise, results > of evaluation are interpreted as file name--*undocumented*. > > That said, your patch should still work fine even with these > considerations. But we may need to sort out this undocumented behaviour. > Probably, an error like in `org-babel-graphical-output-file' should be > thrown by `org-babel-generate-file-param'. I think the Worg documentation is accurately describing the behavior of ob-python -- it's just that ob-python uses ":results file" in a nonstandard way. But I'm hesitant to make a breaking change to ob-python on this. Until now it has been the only way for ob-python to return a plot. And the matplotlib example in Worg has been there for 10 years, long before I started maintaining ob-python.
Re: [PATCH] ob-python: support header argument `:results file graphics'
Liu Hui writes: > Hi, > > This patch adds graphics output support for ob-python via matplotlib. > Specifically, it allows to use header argument `:results file > graphics' as follows: > > #+begin_src python :file "test.png" :results graphics file > import matplotlib.pyplot as plt > plt.plot([1,2,3,4,5]) > #+end_src > > The feature is described in the documentation as follows and has been > supported by ob-R, ob-julia, etc. Thanks -- I haven't had a chance to test drive this yet, but I do have a couple questions: 1. Do you need to add a call to pyplot.gcf().clear(), in case of multiple blocks in a session? 2. Would it make sense to wrap in pyplot.rc_context, so that we can use the :width and :height arguments like ob-R? E.g., with pyplot.rc_context({'figure.figsize': (8,5)}): pyplot.plot([1,2,3,4,5]) pyplot.gcf().savefig('filename.png') Will create a png file with width 8 and height 5.
Re: [PATCH] ob-python: support header argument `:results file graphics'
Ihor Radchenko writes: > Liu Hui writes: > >> This patch adds graphics output support for ob-python via matplotlib. >> Specifically, it allows to use header argument `:results file >> graphics' as follows: >> >> #+begin_src python :file "test.png" :results graphics file >> import matplotlib.pyplot as plt >> plt.plot([1,2,3,4,5]) >> #+end_src > > This might be a useful feature, but it will break the existing > conventions, if used as in the patch. Currently, the above combination > of :file and :results is treated as the following: ":results graphics file" is used this way in ob-R and ob-julia, and also in the testing files test-ob-octave.el and ob-maxima-test.org. ":results graphics" just means that the result from org-babel-execute:lang isn't written to the file by org-babel-execute-src-block. This is needed for plotting because org-babel-execute:lang usually writes directly to the file, rather than returning a byte stream for the PNG or SVG. So the Org manual's wording about "side-effects" and "output not written to disk" is correct in a sense, but also confusing (readers should not have to know about these internal implementation details).
Re: [PATCH] ox-icalendar: Unscheduled tasks & repeating tasks
Ihor Radchenko writes: > Would it make sense to throw a warning instead of silently skipping ~++~ > and ~.+~ repeaters? > > I think it would make sense to link to this variable in the > `org-icalendar-use-scheduled' docstring and possibly in the manual. Thanks, I agree. I've updated the patch to add a warning for the nonstandard repeaters (plus a unit test), and also added links to `org-icalendar-todo-unscheduled-start' in the manual and `org-icalendar-use-scheduled' docstring. For convenience I attach these as separate patches here. If it looks OK I will squash with the prior patch before applying to main. >From 80c05e00335062cc96bdcd85ec507066af4a1d3b Mon Sep 17 00:00:00 2001 From: Jack Kamm Date: Sat, 17 Jun 2023 07:55:17 -0700 Subject: [PATCH 2/3] ox-icalendar: Display warning for unsupported repeaters This commit to be squashed with the previous one * lisp/ox-icalendar.el (org-icalendar--repeater-type): Helper function to get the repeater type, and display warning if not supported. * testing/lisp/test-ox-icalendar.el (test-ox-icalendar/warn-unsupported-repeater): Unit test to warn for unsupported repeater types. --- lisp/ox-icalendar.el | 30 +- testing/lisp/test-ox-icalendar.el | 14 ++ 2 files changed, 35 insertions(+), 9 deletions(-) diff --git a/lisp/ox-icalendar.el b/lisp/ox-icalendar.el index 8c569752b..0dbc623b4 100644 --- a/lisp/ox-icalendar.el +++ b/lisp/ox-icalendar.el @@ -810,6 +810,23 @@ (\"PUBLIC\", \"CONFIDENTIAL\", and \"PRIVATE\") are predefined, others (org-icalendar--valarm entry timestamp summary) "END:VEVENT"))) +(defun org-icalendar--repeater-type (elem) + "Return ELEM's repeater-type if supported, else warn and return nil." + (let ((repeater-value (org-element-property :repeater-value elem)) +(repeater-type (org-element-property :repeater-type elem))) +(cond + ((not (and repeater-type +repeater-value +(> repeater-value 0))) + nil) + ;; TODO Add catch-up to supported repeaters (use EXDATE to implement) + ((not (memq repeater-type '(cumulate))) + (org-display-warning + (format "Repeater-type %s not currently supported by iCalendar export" + (symbol-name repeater-type))) + nil) + (repeater-type + (defun org-icalendar--vtodo (entry uid summary location description categories timezone class) "Create a VTODO component. @@ -826,13 +843,8 @@ (defun org-icalendar--vtodo (org-element-property :scheduled entry))) (dl (and (memq 'todo-due org-icalendar-use-deadline) (org-element-property :deadline entry))) - ;; TODO Implement catch-up repeaters using EXDATE - (sc-repeat-p (and (eq (org-element-property :repeater-type sc) - 'cumulate) - (> (org-element-property :repeater-value sc) 0))) - (dl-repeat-p (and (eq (org-element-property :repeater-type dl) - 'cumulate) - (> (org-element-property :repeater-value dl) 0))) + (sc-repeat-p (org-icalendar--repeater-type sc)) + (dl-repeat-p (org-icalendar--repeater-type dl)) (repeat-value (or (org-element-property :repeater-value sc) (org-element-property :repeater-value dl))) (repeat-unit (or (org-element-property :repeater-unit sc) @@ -881,14 +893,14 @@ (defun org-icalendar--vtodo (eq repeat-unit (org-element-property :repeater-unit dl) ;; TODO Implement via RDATE with changing DURATION - (warn "Not yet implemented: \ + (org-display-warning "Not yet implemented: \ different repeaters on SCHEDULED and DEADLINE. Skipping.") nil) ;; DEADLINE has repeater but SCHEDULED doesn't ((and dl-repeat-p (and sc (not sc-repeat-p))) ;; TODO SCHEDULED should only apply to first instance; ;; use RDATE with custom DURATION to implement that - (warn "Not yet implemented: \ + (org-display-warning "Not yet implemented: \ repeater on DEADLINE but not SCHEDULED. Skipping.") nil) ((or sc-repeat-p dl-repeat-p) diff --git a/testing/lisp/test-ox-icalendar.el b/testing/lisp/test-ox-icalendar.el index 6a0c961d7..e631b2119 100644 --- a/testing/lisp/test-ox-icalendar.el +++ b/testing/lisp/test-ox-icalendar.el @@ -114,5 +114,19 @@ (ert-deftest test-ox-icalendar/todo-repeater-until-utc () (should (re-search-forward "RRULE:FREQ=DAILY;INTERVAL=3;UNTIL=2023050.T..Z" (when (file-exists-p tmp-ics) (delete-file tmp-ics)))
[PATCH] ox-icalendar: Unscheduled tasks & repeating tasks
Hello, I am attaching an updated patch for ox-icalendar unscheduled and repeating TODOs, incorporating some of Ihor's feedback to my RFC some months ago. Compared to my original RFC, here are the main changes: - For unscheduled TODOs with repeating deadline, the deadline warning days is used as the start time by default, in order to comply with the iCalendar spec which demands a start time in this case. - Previously I had separate patches for unscheduled and repeating TODOs, but now I combine them into a single patch because of the way repeats and start times are intertwined for repeating deadlines. - New customization `org-icalendar-todo-unscheduled-start' controls the exported start time for unscheduled TODOs. It replaces `org-icalendar-todo-force-scheduling' from my previous version of the patch. - In case of a SCHEDULED repeater, and a DEADLINE with no repeater, the task repeats until the deadline, using the RRULE UNTIL keyword. - Added linting for the case where SCHEDULED and DEADLINE have mismatching repeaters. - Added several tests for ox-icalendar, and a test for the new lint as well. There are still a few cases that are not yet handled, but they are less common and will take some more work to implement, so I would prefer to leave them to future patches: - Case where SCHEDULED and DEADLINE have mismatched repeaters. We can use RDATE with differing DURATION for this. - Case where DEADLINE has repeater but SCHEDULED does not. We can use RDATE for the first instance, and RRULE for the subsequent repeats. - Case of catch-up "++" repeaters. We can use EXDATE to exclude repeats before today. - Case of restart ".+" repeaters. I don't think iCalendar can handle this case, and we should ignore it. >From 1135e3e7cb08353892c439b085d3bf0bf1072ecb Mon Sep 17 00:00:00 2001 From: Jack Kamm Date: Sun, 11 Jun 2023 07:50:20 -0700 Subject: [PATCH] ox-icalendar: Add support for unscheduled and repeating TODOs * lisp/ox-icalendar.el (org-icalendar-todo-unscheduled-start): New customization to control the exported start time of unscheduled tasks. (org-icalendar--rrule): Helper function for RRULE export. (org-icalendar--vevent): Use the new helper function for RRULE. (org-icalendar--vtodo): Change how unscheduled TODOs are handled using the new customization option. Export SCHEDULED and DEADLINE repeaters. In case of SCHEDULED repeater and a DEADLINE without repeater, treat DEADLINE as RRULE UNTIL. Emit a warning for tricky edge cases that are not yet implemented. * testing/lisp/test-ox-icalendar.el (test-ox-icalendar/todo-repeater-shared): Test for exporting shared SCHEDULED/DEADLINE repeater. (test-ox-icalendar/todo-repeating-deadline-warndays): Test using warning days as DTSTART of repeating deadline. (test-ox-icalendar/todo-repeater-until): Test using DEADLINE as RRULE UNTIL. (test-ox-icalendar/todo-repeater-until-utc): Test RRULE UNTIL is in UTC format when DTSTART is not in local time format. * lisp/org-lint.el (org-lint-mismatched-planning-repeaters): Add lint for mismatched SCHEDULED and DEADLINE repeaters. * testing/lisp/test-org-lint.el (test-org-lint/mismatched-planning-repeaters): Add test for linting of mismatched SCHEDULED and DEADLINE repeaters. --- etc/ORG-NEWS | 64 lisp/org-lint.el | 34 ++ lisp/ox-icalendar.el | 165 +- testing/lisp/test-org-lint.el | 7 ++ testing/lisp/test-ox-icalendar.el | 74 ++ 5 files changed, 320 insertions(+), 24 deletions(-) diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS index 7e7015064..a24caddfe 100644 --- a/etc/ORG-NEWS +++ b/etc/ORG-NEWS @@ -50,6 +50,21 @@ ox-icalendar. In particular, older versions of org-caldav may encounter issues, and users are advised to update to the most recent version of org-caldav. See [[https://github.com/dengste/org-caldav/commit/618bf4cdc9be140ca1993901d017b7f18297f1b8][this org-caldav commit]] for more information. +*** Icalendar export of unscheduled TODOs no longer have start time of today + +For TODOs without a scheduled start time, ox-icalendar no longer +forces them to have a scheduled start time of today when exporting. + +Instead, the new customization ~org-icalendar-todo-unscheduled-start~ +controls the exported start date for unscheduled tasks. Its default +is ~recurring-deadline-warning~ which will export unscheduled tasks +with no start date, unless it has a recurring deadline (in which case +the iCalendar spec demands a start date, and +~org-deadline-warning-days~ is used for that). + +To revert to the old behavior, set +~org-icalendar-todo-unscheduled-start~ to ~current-datetime~. + ** New and changed options *** Commands affected by ~org-fold-catch-invisible-edits~ can now be customized @@ -188,6 +203,28 @@ default settings of "Body only", "Visible only", and "Force publishing" in
Re: [RFC] ox-icalendar: Unscheduled tasks & repeating tasks
Ihor Radchenko writes: >> So technically, a standalone DEADLINE + repeater isn't allowed -- a >> repeating task must always have a start date. > > May we then use org-deadline-warning-days/timestamp warntime spec as DTSTART? > VALARM component is not fitting for warning days anyway. > >> But still, maybe we should stick to the requirement, and only export >> repeater on SCHEDULED. That would simplify the implementation. The >> downside is that repeating deadlines won't show up in iCalendar, which >> seems undesirable. > > Agree. We should better stick to the spec. I took a closer look into how other programs handle RRULE, DTSTART, DUE. I tried the following CalDav servers: Nextcloud, radicale And the following clients: Tasks.org, Thunderbird, Evolution. (I did not use Nextcloud client because it doesn't support repeating tasks, even though the Nextcloud server does). Thunderbird and Evolution clients do not allow creating repeating tasks without start date -- if you try to do so, they will force you to specify one. Tasks.org client does allow repeating tasks with only a deadline (no start date). Nextcloud and radicale servers happily accept the repeating deadline from Tasks.org without start date. When I download the ICS file from the server, the VTODO contains RRULE and DUE, but not DTSTART. When I validate the ICS file with icalendar.org [1], it accepts the ICS as valid, even though it seemingly violates the spec by missing DTSTART. So, it seems there is some inconsistency about this in the iCalendar ecosystem. I have not yet reached a firm conclusion on the best solution, but am leaning towards your suggestion to use org-deadline-warning-days for DTSTART in this case. I'll try to have a more concrete, updated patch on this ready in a couple weeks or so. [1] https://icalendar.org/validator.html
Re: Bug report for ox-icalendar: newlines should be CRLF
Jack Kamm writes: > writes: > >>> > There is a related issue about EOLs, not just \r\n -- each line should >>> > be a maximum of 75 characters; this is handled by >>> > org-icalendar-fold-string >>> >>> May you please provide a link to the iCalendar spec document section >>> describing this requirement? >> >> It's in rfc5545 [1], referenced to from rfc7986 [2]. > > Since VEVENT and VTODO are wrapped in `org-icalendar-fold-string', I > think the only place this error may occur is in the preamble created by > `org-icalendar--vcalendar', e.g. in PRODID, X-WR-CALDESC, etc. > > The fix is to move the call to `org-icalendar-fold-string' out of > org-icalendar--vevent/vtodo, and instead put it in > `org-icalendar--vcalendar'. Then the line folding will apply to the > whole VCALENDAR instead of just the VTODO and VEVENT. An earlier version > of my CRLF patch actually did something like this. > > I can push such a fix over the weekend. Let me know if you'd rather I > send the patch for review here first. I've pushed this change now: https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=f4446ce795c924a1e115e360d3674f6ad89be845
Re: [RFC] ox-icalendar: Unscheduled tasks & repeating tasks
Ihor Radchenko writes: > The question is: does iCalendar allow something like > > DTSTART;TZID=America/New_York:19970105T083000 > RRULE:FREQ=YEARLY > DUE;TZID=America/New_York:20070105T083000 > > and repeats past DUE? > > If not, we have to choose when exporting from Org source - either to > keep DUE or not. >From the defintion of RRULE [1]: If the duration of the recurring component is specified with the "DTEND" or "DUE" property, then the same exact duration will apply to all the members of the generated recurrence set. So the RRULE applies to both DTSTART and DUE, and the repeats continue past DUE. But, another thing to note from the definition of DTSTART [2]: This property [DTSTART] is REQUIRED in all types of recurring calendar components that specify the "RRULE" property. So technically, a standalone DEADLINE + repeater isn't allowed -- a repeating task must always have a start date. But my impression is that not all iCalendar programs respect this. In particular, Tasks.org app with Nextcloud server seemed to allow a standalone repeating deadline. But I will check this more carefully, and also in a couple more programs (radicale, Evolution). But still, maybe we should stick to the requirement, and only export repeater on SCHEDULED. That would simplify the implementation. The downside is that repeating deadlines won't show up in iCalendar, which seems undesirable. [1] https://www.rfc-editor.org/rfc/rfc5545#section-3.8.5.3: [2] https://www.rfc-editor.org/rfc/rfc5545#section-3.8.2.4 > If we want to leave as many options as possible to the users, we can (1) > Implement ICALENAR_DUE property that will set DUE explicitly on export; > (2) ICALENDAR_DUE may allow special values that will indicate how to > treat Org DEADLINEs - make them into DUE, use Org DEADLINE as a > bound for SCHEDULED repeater, or ignore DEADLINE completely. A couple of these behaviors can already be achieved by customizing `org-icalendar-use-deadline' (making DUE or ignoring). For using DEADLINE as a bound, we could potentially add another option for that. > Is there any reason for this? May we instead export to a single VEVENT > with appropriate RDATE list? I guess if there are multiple timestamps with repeaters, it's easier to export these as separate VEVENT, because it's not possible to have multiple RRULE in one VEVENT. But, your suggestion earlier in thread could also solve this: in case of different repeaters, we can use RDATE to generate occurrences manually sufficiently far into future (with defcustom for "how far").
Re: [RFC] ox-icalendar: Unscheduled tasks & repeating tasks
Ihor Radchenko writes: > Another scenario we may need to consider is when schedule has a repeater > while deadline does not, and vice versa. The former scenario is probably > valid - a VTODO with limited number of occurrences. That is an interesting idea; and we can use the UNTIL or COUNT keywords in RRULE to implement it. However, it doesn't seem completely faithful to the way the TODO ends up in the Org Agenda (or does Org have some option to use DEADLINE to bound a repeating SCHEDULED in this way?) I think the most faithful way to represent different SCHEDULED and DEADLINE repeaters is to export 2 separate VTODOs, each with different RRULE. Then the exported iCalendar will look just like the Org Agenda. It is also in line with how ox-icalendar exports multiple timestamps to separate VEVENTs. That said, I am not really happy with this solution either. The fact that ox-icalendar can create multiple VEVENT per entry already creates headaches for any setup doing bidirectional sync between Org and iCalendar, such as with org-caldav, ical2org.awk, or ical2orgpy. And I am hesitant to make this problem worse, by making it happen for VTODO as well.
Re: Bug report for ox-icalendar: newlines should be CRLF
writes: >> > There is a related issue about EOLs, not just \r\n -- each line should >> > be a maximum of 75 characters; this is handled by >> > org-icalendar-fold-string >> >> May you please provide a link to the iCalendar spec document section >> describing this requirement? > > It's in rfc5545 [1], referenced to from rfc7986 [2]. Since VEVENT and VTODO are wrapped in `org-icalendar-fold-string', I think the only place this error may occur is in the preamble created by `org-icalendar--vcalendar', e.g. in PRODID, X-WR-CALDESC, etc. The fix is to move the call to `org-icalendar-fold-string' out of org-icalendar--vevent/vtodo, and instead put it in `org-icalendar--vcalendar'. Then the line folding will apply to the whole VCALENDAR instead of just the VTODO and VEVENT. An earlier version of my CRLF patch actually did something like this. I can push such a fix over the weekend. Let me know if you'd rather I send the patch for review here first.
Re: Bug report for ox-icalendar: newlines should be CRLF
Ihor Radchenko writes: > Sorry for the late reply. > > Do I understand correctly that all the ical lines must use \r\n? [1] Following up here that I have pushed a fix for the EOL issue in ox-icalendar: https://list.orgmode.org/87355ikzwk.fsf@localhost/T/#m180c100587d3d88ab5787942271a546b51891996
Re: [RFC] ox-icalendar: Unscheduled tasks & repeating tasks
Ihor Radchenko writes: > Looks reasonable, but I have one comment on the code. > We should not use user-defined hooks for things that must be executed. > Instead, we should better explicitly call the necessary functions. Thanks, I updated the patch to explicitly call the function. Also, I tweaked the coding-system-for-write to be a bit safer, in case of edge cases where utf-8 doesn't work -- I think RFC 5545 just says it's the default charset. Attached is the (I think) final version of the patch. I'll install it soon, unless I hear otherwise. PS I haven't forgotten your feedback on the original VTODO-related patches (thanks for that review). I'll work on that next, but it might take me a bit longer. >From aa59625cd08dcee767f42ad8d45d8902aa8d38bd Mon Sep 17 00:00:00 2001 From: Jack Kamm Date: Sat, 1 Apr 2023 16:53:35 -0700 Subject: [PATCH] ox-icalendar: Use consistent CRLF line endings Fixes issue where the ox-icalendar export uses an inconsistent mix of dos and unix style line endings. * lisp/ox-icalendar.el (org-icalendar-fold-string): No longer converts to CRLF, instead delegating that to `org-icalendar--post-process-file'. (org-icalendar--post-process-file): New function to handle exported file post-processing. Converts EOL to CRLF, and then runs `org-icalendar-after-save-hook'. (org-icalendar-export-to-ics, org-icalendar-export-current-agenda, org-icalendar--combine-files): Call `org-icalendar--post-process-file' instead of running `org-icalendar-after-save-hook' directly. * testing/lisp/test-ox-icalendar.el: New file for unit tests of ox-icalendar. Add an initial test for CRLF line endings. See also: https://list.orgmode.org/87o7oetneo.fsf@localhost/T/#m3e3eb80f9fc51ba75854b33ebfe9ecdefa2ded24 https://list.orgmode.org/orgmode/87ilgljv6i.fsf@localhost/ --- etc/ORG-NEWS | 12 + lisp/ox-icalendar.el | 27 --- testing/lisp/test-ox-icalendar.el | 44 +++ 3 files changed, 74 insertions(+), 9 deletions(-) create mode 100644 testing/lisp/test-ox-icalendar.el diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS index ac233a986..9f7d01707 100644 --- a/etc/ORG-NEWS +++ b/etc/ORG-NEWS @@ -23,6 +23,18 @@ If you still want to use python-mode with ob-python, you might consider [[https://gitlab.com/jackkamm/ob-python-mode-mode][ob-python-mode-mode]], where the code to support python-mode has been ported to. +*** =ox-icalendar.el= line ending fix may affect downstream packages + +iCalendar export now uses dos-style CRLF ("\r\n") line endings +throughout, as required by the iCalendar specification (RFC 5545). +Previously, the export used an inconsistent mix of dos and unix line +endings. + +This might cause errors in external packages that parse output from +ox-icalendar. In particular, older versions of org-caldav may +encounter issues, and users are advised to update to the most recent +version of org-caldav. See [[https://github.com/dengste/org-caldav/commit/618bf4cdc9be140ca1993901d017b7f18297f1b8][this org-caldav commit]] for more information. + ** New and changed options *** New ~org-cite-natbib-export-bibliography~ option defining fallback bibliography style diff --git a/lisp/ox-icalendar.el b/lisp/ox-icalendar.el index 81a77a770..ccc237721 100644 --- a/lisp/ox-icalendar.el +++ b/lisp/ox-icalendar.el @@ -540,12 +540,23 @@ (defun org-icalendar-fold-string (s) ;; line, real contents must be split at 74 chars. (while (< (setq chunk-end (+ chunk-start 74)) len) (setq folded-line - (concat folded-line "\r\n " + (concat folded-line "\n " (substring line chunk-start chunk-end)) chunk-start chunk-end)) - (concat folded-line "\r\n " (substring line chunk-start)) -(org-split-string s "\n") "\r\n"))) - + (concat folded-line "\n " (substring line chunk-start)) +(org-split-string s "\n") "\n"))) + +(defun org-icalendar--post-process-file (file) + "Post-process the exported iCalendar FILE. +Converts line endings to dos-style CRLF as per RFC 5545, then +runs `org-icalendar-after-save-hook'." + (with-temp-buffer +(insert-file-contents file) +(let ((coding-system-for-write (coding-system-change-eol-conversion +last-coding-system-used 'dos))) + (write-region nil nil file))) + (run-hook-with-args 'org-icalendar-after-save-hook file) + nil) ;;; Filters @@ -932,8 +943,7 @@ (defun org-icalendar-export-to-ics (org-export-to-file 'icalendar outfile async subtreep visible-only body-only '(:ascii-charset utf-8 :ascii-links-to-notes nil) - '(lambda (file) - (run-hook-with-args 'org-icalendar-after-save-hook file) nil + #'org-icalendar--post-process-file))) ;;;###autoload (
Re: [RFC] ox-icalendar: Unscheduled tasks & repeating tasks
Ihor Radchenko writes: > So, we should probably override `org-export-coding-system', even when it > is set. iCalendar demands UTF8 anyway. Also, ox-icalendar already sets ":ascii-charset utf-8" in the ext-plist during export. > We likely want (according to 34.10.1 Basic Concepts of Coding Systems): I attach a new patch, which takes the approach of converting to utf-8-dos in `org-icalendar-after-save-hook', instead of converting newlines in `org-icalendar-fold-string'. I think this way is simpler, and should be more robust across locales. Note, this means the string returned by `org-export-as' won't contain CRLF. Instead, the newlines are converted during post-process. >From 04761429f82bfd2aee63f4978afec3449abaa37d Mon Sep 17 00:00:00 2001 From: Jack Kamm Date: Sat, 1 Apr 2023 16:53:35 -0700 Subject: [PATCH] ox-icalendar: Use consistent CRLF line endings Fixes issue where the ox-icalendar export uses an inconsistent mix of dos and unix style line endings. * lisp/ox-icalendar.el (org-icalendar-fold-string): No longer converts to CRLF, instead delegating that to `org-icalendar--convert-eol'. (org-icalendar--convert-eol): New function to convert EOL to CRLF. It runs early in `org-icalendar-after-save-hook'. * testing/lisp/test-ox-icalendar.el: New file for unit tests of ox-icalendar. Add an initial test for CRLF line endings. See also: https://list.orgmode.org/87o7oetneo.fsf@localhost/T/#m3e3eb80f9fc51ba75854b33ebfe9ecdefa2ded24 https://list.orgmode.org/orgmode/87ilgljv6i.fsf@localhost/ --- etc/ORG-NEWS | 12 + lisp/ox-icalendar.el | 14 +++--- testing/lisp/test-ox-icalendar.el | 44 +++ 3 files changed, 67 insertions(+), 3 deletions(-) create mode 100644 testing/lisp/test-ox-icalendar.el diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS index ac233a986..9f7d01707 100644 --- a/etc/ORG-NEWS +++ b/etc/ORG-NEWS @@ -23,6 +23,18 @@ If you still want to use python-mode with ob-python, you might consider [[https://gitlab.com/jackkamm/ob-python-mode-mode][ob-python-mode-mode]], where the code to support python-mode has been ported to. +*** =ox-icalendar.el= line ending fix may affect downstream packages + +iCalendar export now uses dos-style CRLF ("\r\n") line endings +throughout, as required by the iCalendar specification (RFC 5545). +Previously, the export used an inconsistent mix of dos and unix line +endings. + +This might cause errors in external packages that parse output from +ox-icalendar. In particular, older versions of org-caldav may +encounter issues, and users are advised to update to the most recent +version of org-caldav. See [[https://github.com/dengste/org-caldav/commit/618bf4cdc9be140ca1993901d017b7f18297f1b8][this org-caldav commit]] for more information. + ** New and changed options *** New ~org-cite-natbib-export-bibliography~ option defining fallback bibliography style diff --git a/lisp/ox-icalendar.el b/lisp/ox-icalendar.el index 81a77a770..7f675b5d0 100644 --- a/lisp/ox-icalendar.el +++ b/lisp/ox-icalendar.el @@ -540,12 +540,20 @@ (defun org-icalendar-fold-string (s) ;; line, real contents must be split at 74 chars. (while (< (setq chunk-end (+ chunk-start 74)) len) (setq folded-line - (concat folded-line "\r\n " + (concat folded-line "\n " (substring line chunk-start chunk-end)) chunk-start chunk-end)) - (concat folded-line "\r\n " (substring line chunk-start)) -(org-split-string s "\n") "\r\n"))) + (concat folded-line "\n " (substring line chunk-start)) +(org-split-string s "\n") "\n"))) +(defun org-icalendar--convert-eol (f) + "Convert line endings to CRLF as per RFC 5545." + (with-temp-buffer +(insert-file-contents f) +(let ((coding-system-for-write 'utf-8-dos)) + (write-region nil nil f + +(add-hook 'org-icalendar-after-save-hook #'org-icalendar--convert-eol -90) ;;; Filters diff --git a/testing/lisp/test-ox-icalendar.el b/testing/lisp/test-ox-icalendar.el new file mode 100644 index 0..bfc756d51 --- /dev/null +++ b/testing/lisp/test-ox-icalendar.el @@ -0,0 +1,44 @@ +;;; test-ox-icalendar.el --- tests for ox-icalendar.el -*- lexical-binding: t; -*- + +;; Copyright (C) 2023 Jack Kamm + +;; Author: Jack Kamm + +;; This program 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. + +;; This program 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 s
Re: [RFC] ox-icalendar: Unscheduled tasks & repeating tasks
Ihor Radchenko writes: > I now only have one minor concern about `org-icalendar-fold-string' when > the original buffer contains DOS line endings. May they mess things up > producing \r\r\n? There are 2 issues here: what does `org-icalendar-fold-string' do when string already contains \r, and what does `org-export-to-file' do when `org-export-coding-system' or `buffer-file-coding-system' is dos-like. In both cases, the patch doesn't change the existing behavior -- which is to produce \r\r\n. For issue 1, what `org-icalendar-fold-string' does when string already contains \r\n, you can see that it produces \r\r\n as follows: emacs -Q -l ox-icalendar M-: (org-icalendar-fold-string (org-icalendar-fold-string "Line1\nLine2")) This is why the patch removes the calls to `org-icalendar-fold-string' in `org-icalendar--vevent' and `org-icalendar--vtodo' -- otherwise we would add \r multiple times to the same string. To change this behavior of `org-icalendar-fold-string', we could modify the patch to do: (defun org-icalendar-fold-string (s) "Fold string S according to RFC 5545." (replace-regexp-in-string - "\n" "\r\n" + "\r*\n" "\r\n" which would strip out any extra \r at end of line. Another alternative would be to use "\r?\n" instead of "\r*\n". For the second issue -- when `org-export-coding-system' is dos (or similar), the file created by `org-export-to-file' will contain \r\r\n. This was already the pre-existing behavior, but note the patch does cause a minor change here: before the patch just the main body will have \r\r\n, but after the patch, the preamble will also have it.
Re: [RFC] ox-icalendar: Unscheduled tasks & repeating tasks
Ihor Radchenko writes: > Thanks! > Note that I did not implement my suggestion because I am concerned if > putting CRLF is safe as every single line ending. > > I am looking at > https://icalendar.org/iCalendar-RFC-5545/3-6-2-to-do-component.html, and > I note that only BEGIN:VTODO and END:VTODO lines must actually have > CRLF. For example, > https://icalendar.org/iCalendar-RFC-5545/3-3-11-text.html has no > mentions of CRLF, but does talk about escaping staff. My reading of [1] is that all lines must end with CRLF: > The iCalendar object is organized into individual lines of text, > called content lines. Content lines are delimited by a line break, > which is a CRLF sequence And in particular, for the component properties after BEGIN:VTODO, [1] gives the general notation as: > contentline = name *(";" param ) ":" value CRLF For example, the DTSTART notation [2] is: > dtstart= "DTSTART" dtstparam ":" dtstval CRLF And the same is true for all the other properties. [1] https://icalendar.org/iCalendar-RFC-5545/3-1-content-lines.html [2] https://icalendar.org/iCalendar-RFC-5545/3-8-2-4-date-time-start.html
Re: [RFC] ox-icalendar: Unscheduled tasks & repeating tasks
Ihor Radchenko writes: > Side note: here, and in other places, we use "\n" as end of line. Yet, > for example > https://icalendar.org/iCalendar-RFC-5545/3-8-2-4-date-time-start.html > prescribes CRLF (\r\n). Also, see > https://orgmode.org/list/87ilgljv6i.fsf@localhost > If you are familiar with iCalendar spec, may you look through the > ox-icalendar code and check other places where we do not conform to the > newline spec? org-icalendar--vtodo is wrapped in org-icalendar-fold-string, so this "\n" gets converted to CRLF later on. However you are right that other parts of the iCalendar export have inconsistent line endings. Currently, VEVENT and VTODO components have the correct CRLF endings, but the other parts of the VCALENDAR do not (such as the preamble). I like your suggestion in the above thread to just wrap the whole export in `org-icalendar-fold-string'. Though I think it's slightly nicer to do it in `org-icalendar--vcalendar' instead of `org-icalendar-template'. So, I've attached a standalone patch to do this. It also fixes an issue with `org-icalendar-fold-string' where the last newline was missing "\r", and adds a unit test. Note that fixing the line endings causes a surprising compatibility issue with org-caldav. I fixed this problem on the org-caldav side, and made a note in ORG-NEWS. >From 712a4ef09b63b2f6bdec2a3967712be912dce0d2 Mon Sep 17 00:00:00 2001 From: Jack Kamm Date: Thu, 30 Mar 2023 22:19:09 -0700 Subject: [PATCH] ox-icalendar: Use consistent CRLF line endings Fixes issue where the ox-icalendar export uses an inconsistent mix of dos and unix style line endings. * lisp/ox-icalendar.el (org-icalendar-fold-string): Don't use "\r" during the string construction, instead replace "\n" with "\r\n" after string has been created. This fixes an issue where the final "\n" added by `org-element-normalize-string' was missing "\r". (org-icalendar--vevent): Remove call to `org-icalendar-fold-string'. (org-icalendar--vtodo): Remove call to `org-icalendar-fold-string'. (org-icalendar--vcalendar): Wrap in `org-icalendar-fold-string'. * testing/lisp/test-ox-icalendar.el: New file for unit tests of ox-icalendar. Add an initial test for CRLF line endings. See also: https://list.orgmode.org/87o7oetneo.fsf@localhost/T/#m3e3eb80f9fc51ba75854b33ebfe9ecdefa2ded24 https://list.orgmode.org/orgmode/87ilgljv6i.fsf@localhost/ --- etc/ORG-NEWS | 12 +++ lisp/ox-icalendar.el | 159 +++--- testing/lisp/test-ox-icalendar.el | 46 + 3 files changed, 138 insertions(+), 79 deletions(-) create mode 100644 testing/lisp/test-ox-icalendar.el diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS index ac233a986..9f7d01707 100644 --- a/etc/ORG-NEWS +++ b/etc/ORG-NEWS @@ -23,6 +23,18 @@ If you still want to use python-mode with ob-python, you might consider [[https://gitlab.com/jackkamm/ob-python-mode-mode][ob-python-mode-mode]], where the code to support python-mode has been ported to. +*** =ox-icalendar.el= line ending fix may affect downstream packages + +iCalendar export now uses dos-style CRLF ("\r\n") line endings +throughout, as required by the iCalendar specification (RFC 5545). +Previously, the export used an inconsistent mix of dos and unix line +endings. + +This might cause errors in external packages that parse output from +ox-icalendar. In particular, older versions of org-caldav may +encounter issues, and users are advised to update to the most recent +version of org-caldav. See [[https://github.com/dengste/org-caldav/commit/618bf4cdc9be140ca1993901d017b7f18297f1b8][this org-caldav commit]] for more information. + ** New and changed options *** New ~org-cite-natbib-export-bibliography~ option defining fallback bibliography style diff --git a/lisp/ox-icalendar.el b/lisp/ox-icalendar.el index 81a77a770..06e90d032 100644 --- a/lisp/ox-icalendar.el +++ b/lisp/ox-icalendar.el @@ -526,25 +526,27 @@ (defun org-icalendar-cleanup-string (s) (defun org-icalendar-fold-string (s) "Fold string S according to RFC 5545." - (org-element-normalize-string - (mapconcat -(lambda (line) - ;; Limit each line to a maximum of 75 characters. If it is - ;; longer, fold it by using "\r\n " as a continuation marker. - (let ((len (length line))) - (if (<= len 75) line - (let ((folded-line (substring line 0 75)) - (chunk-start 75) - chunk-end) - ;; Since continuation marker takes up one character on the - ;; line, real contents must be split at 74 chars. - (while (< (setq chunk-end (+ chunk-start 74)) len) - (setq folded-line - (concat folded-line "\r\n " - (substring line chunk-start chunk-end)) - chunk-start chunk-end)) - (concat folded-line "\r\n " (substring line ch
[RFC] ox-icalendar: Unscheduled tasks & repeating tasks
Hello, The attached 2 patches add support for exporting unscheduled tasks and repeating tasks to iCalendar, respectively. For patch 1 (unscheduled tasks): Currently, ox-icalendar does not allow creating an iCalendar task without a scheduled start date. If an Org TODO is missing a SCHEDULED timestamp, then ox-icalendar sets today as the scheduled start date for the exported task. Patch 1 changes this by adding a new customization org-icalendar-todo-force-scheduling. When non-nil, the start date is set to today (same as the current behavior). When nil, unscheduled Org TODOs are instead exported without a start date. I also propose the default value to be nil. Note, this is backwards-incompatible with the previous behavior! But I think it should be the default anyways, because IMO it is the more correct and useful behavior. An iCalendar VTODO without a DTSTART property is valid, and has the same meaning as an Org TODO without a SCHEDULED timestamp. Also, all the iCalendar programs I have tried support unscheduled tasks, including Thunderbird, Evolution, Nextcloud, and Tasks.org. For patch 2 (repeating timestamps): I add recurrence rule (RRULE) export for repeating SCHEDULED and DEADLINE timestamps in TODOs, similar to how repeating non-TODO events are currently handled. The main complication here is that iCalendar's RRULE applies to both DTSTART and DUE properties; by contrast, Org's SCHEDULED and DEADLINE timestamps may have different repeaters. I am not sure the best way to handle the case where SCHEDULED and DEADLINE have different repeaters, so in that case I issue a warning and skip the repeater. >From 1bd268ab260d5077d7456c0d64fea36128772f86 Mon Sep 17 00:00:00 2001 From: Jack Kamm Date: Sun, 26 Mar 2023 07:43:53 -0700 Subject: [PATCH 1/2] ox-icalendar: Allow exporting unscheduled VTODOs * lisp/ox-icalendar.el (org-icalendar-todo-force-scheduling): New option to revert to previous export behavior of unscheduled TODOs. (org-icalendar--vtodo): Don't force unscheduled TODOs to have a scheduled start time of today, unless `org-icalendar-todo-force-scheduling' is set. --- etc/ORG-NEWS | 15 +++ lisp/ox-icalendar.el | 32 +--- 2 files changed, 36 insertions(+), 11 deletions(-) diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS index ac233a986..fb4f82b29 100644 --- a/etc/ORG-NEWS +++ b/etc/ORG-NEWS @@ -23,6 +23,15 @@ If you still want to use python-mode with ob-python, you might consider [[https://gitlab.com/jackkamm/ob-python-mode-mode][ob-python-mode-mode]], where the code to support python-mode has been ported to. +*** Icalendar export of TODOs no longer forces a start time + +For TODOs without a scheduled start time, ox-icalendar no longer +forces them to have a scheduled start time of today when exporting. +This makes it possible to create icalendar TODOs without a start time. + +To revert to the old behavior, set the new custom option +~org-icalendar-todo-force-scheduling~ to non-nil. + ** New and changed options *** New ~org-cite-natbib-export-bibliography~ option defining fallback bibliography style @@ -111,6 +120,12 @@ backend used for evaluation of ClojureScript. official [[https://clojure.org/guides/deps_and_cli][Clojure CLI tools]]. The command can be customized with ~ob-clojure-cli-command~. +*** New ~org-icalendar-todo-force-scheduling~ option for old ox-icalendar TODO scheduling behavior + +Set ~org-icalendar-todo-force-scheduling~ to non-nil to revert to the +old ox-icalendar TODO export behavior, that forced all exported TODOs +to have a scheduled start time. + ** New features *** Add support for ~logind~ idle time in ~org-user-idle-seconds~ diff --git a/lisp/ox-icalendar.el b/lisp/ox-icalendar.el index 81a77a770..63aefcc84 100644 --- a/lisp/ox-icalendar.el +++ b/lisp/ox-icalendar.el @@ -231,6 +231,12 @@ (defcustom org-icalendar-include-todo nil (repeat :tag "Specific TODO keywords" (string :tag "Keyword" +(defcustom org-icalendar-todo-force-scheduling nil + "Non-nil means unscheduled tasks are exported as scheduled. +The current date is used as the scheduled time for such tasks." + :group 'org-export-icalendar + :type 'boolean) + (defcustom org-icalendar-include-bbdb-anniversaries nil "Non-nil means a combined iCalendar file should include anniversaries. The anniversaries are defined in the BBDB database." @@ -776,21 +782,25 @@ (defun org-icalendar--vtodo Return VTODO component as a string." (let ((start (or (and (memq 'todo-start org-icalendar-use-scheduled) (org-element-property :scheduled entry)) - ;; If we can't use a scheduled time for some - ;; reason, start task now. - (let ((now (decode-time))) - (list 'timestamp - (list :type 'active - :minute-start (nth 1 now) - :hour-start (nth 2 now) - :day-start (nth 3 now) - :month-start (nth 4 now) - :year-s
Re: [PATCH] Async evaluation in ob-shell
Matt writes: > Hi Jack and Jeremie! I'm curious your thoughts about what Ihor and I are > discussing at the end of this message about `md5'. Thanks for checking in. I don't see any problems with switching to org-id-uuid or similar. Feel free to update ob-python to do this, or I can also do it later following ob-shell's example.
Re: [BUG] org-id-get-create creating property drawers after SCHEDULE and DEADLINE keywords [9.6 (release_9.6-3-ga4d38e @ /nix/store/zr2g5z2hbqxa93ndfkx6n0v489al6lfq-emacs-git-20221206.0/share/emacs/30
Ihor Radchenko writes: > Which is org-caldav's problem not recognizing Org syntax. I don't think this is a bug in org-mode or org-caldav, nor has to do with property drawers. org-caldav just uses ox-icalendar.el to export to ICS, and by default SCHEDULED timestamp is not exported to calendar VEVENTs. However, the user can set org-icalendar-use-scheduled if they want to change this behavior.
Re: [DISCUSSION] Should we deprecate python-mode.el (alternative to built-in python.el) support in ob-python?
Ihor Radchenko writes: > Let's follow the usual approach and first deprecate it. It should become > a constant with 'python value in org-compat.el. Deprecating is not make > sure that other packages that might be testing the variable value do not > get broken. > > Note that we usually `quote' Elisp symbols in the commit messages. Thank you for the review. I've incorporated your feedback and pushed the patch as commit aa48c80fe17eaaaf83c11c9ac2f2fd864f2f3ad9 > I find the need to use advise awkward. Is it necessary? I agree it is awkward, and also fragile to future changes in ob-python. But unless I've missed something, I think some advice is needed to get the old behavior. Still, it would be more robust to just advise the main function org-babel-execute:python, instead of advising the 2 helper functions. But, this would require a little extra work, and while I might do it later, it's a lower priority for me right now (it's unclear whether there are currently active python-mode users of ob-python). For the moment, I just updated the Readme to say the current implementation is a temporary workaround, and it will work for Org 9.7, but may not work in future versions as ob-python evolves. Arguably, it would be best to avoid overriding python src blocks altogether, and instead have completely separate python-mode src blocks. But I would leave such a design decision to whoever decides to maintain babel+python-mode integration in future.
Re: [DISCUSSION] Should we deprecate python-mode.el (alternative to built-in python.el) support in ob-python?
Ihor Radchenko writes: > Marked for future removal in ob-python.el. > https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=21741a469 Thanks Ihor. Not sure if it's too early for this, but I've prepared a patch to remove the support for python-mode.el, attached. Also, to help any python-mode users with the transition, I ported the code to support python-mode to an external package here: https://gitlab.com/jackkamm/ob-python-mode-mode As always, feedback is welcome, either on this patch or the external package. Best, Jack >From 44bdfbbd9858f4190e4404467fa61b8a3f445347 Mon Sep 17 00:00:00 2001 From: Jack Kamm Date: Sat, 21 Jan 2023 08:24:41 -0800 Subject: [PATCH] ob-python: Remove python-mode.el support * lisp/ob-python.el (org-babel-python-mode): Remove customization variable. (org-babel-python-initiate-session-by-key): Remove code to support python-mode.el. (org-babel-python-send-string): Renamed from org-babel-python--send-string, turning it into a public function to accommodate ob-python-mode-mode which advises this function. Also, remove some code for python-mode.el (org-babel-python-evaluate-session): Update calls to renamed function org-babel-python-send-string. --- etc/ORG-NEWS | 10 ++ lisp/ob-python.el | 88 +-- 2 files changed, 34 insertions(+), 64 deletions(-) diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS index 3ef76ec1a..ad7bd6604 100644 --- a/etc/ORG-NEWS +++ b/etc/ORG-NEWS @@ -12,6 +12,16 @@ See the end of the file for license conditions. Please send Org bug reports to mailto:emacs-orgmode@gnu.org. * Version 9.7 (not released yet) +** Important announcements and breaking changes +*** =python-mode.el (MELPA)= support in =ob-python.el= is removed + +=python-mode.el= support has been removed from =ob-python.el=. The +related customization =org-babel-python-mode= has also been removed. + +If you still want to use python-mode with ob-python, you might +consider [[https://gitlab.com/jackkamm/ob-python-mode-mode][ob-python-mode-mode]], where the code to support python-mode +has been ported to. + ** New options *** New options for the "csl" citation export processor's LaTeX output diff --git a/lisp/ob-python.el b/lisp/ob-python.el index 39fe5c28d..f19440e00 100644 --- a/lisp/ob-python.el +++ b/lisp/ob-python.el @@ -36,10 +36,6 @@ (require 'ob) (require 'org-macs) (require 'python) -(declare-function py-shell "ext:python-mode" (&rest args)) -(declare-function py-choose-shell "ext:python-mode" (&optional shell)) -(declare-function py-shell-send-string "ext:python-mode" (strg &optional process)) - (defvar org-babel-tangle-lang-exts) (add-to-list 'org-babel-tangle-lang-exts '("python" . "py")) @@ -52,16 +48,6 @@ (defcustom org-babel-python-command "python" :group 'org-babel :type 'string) -;; FIXME: Remove third-party `python-mode' package support in the next release. -(defcustom org-babel-python-mode - (if (featurep 'python-mode) 'python-mode 'python) - "Preferred python mode for use in running python interactively. -This will typically be either `python' or `python-mode'." - :group 'org-babel - :version "24.4" - :package-version '(Org . "8.0") - :type 'symbol) - (defcustom org-babel-python-hline-to "None" "Replace hlines in incoming tables with this when translating to python." :group 'org-babel @@ -183,7 +169,6 @@ (defun org-babel-python-without-earmuffs (session) (substring name 1 (- (length name) 1)) name))) -(defvar py-which-bufname) (defvar python-shell-buffer-name) (defvar-local org-babel-python--initialized nil "Flag used to mark that python session has been initialized.") @@ -197,47 +182,25 @@ (defun org-babel-python-initiate-session-by-key (&optional session) (cmd (if (member system-type '(cygwin windows-nt ms-dos)) (concat org-babel-python-command " -i") org-babel-python-command))) - (cond - ((eq 'python org-babel-python-mode) ; python.el - (unless py-buffer - (setq py-buffer (org-babel-python-with-earmuffs session))) - (let ((python-shell-buffer-name - (org-babel-python-without-earmuffs py-buffer))) - (run-python cmd) - (with-current-buffer py-buffer -(add-hook - 'python-shell-first-prompt-hook - (lambda () - (setq-local org-babel-python--initialized t) - (message "I am running!!!")) - nil 'local - ((and (eq 'python-mode org-babel-python-mode) - (fboundp 'py-shell)) ; python-mode.el - (require 'python-mode) - ;; Make sure that py-which-bufname is initialized, as otherwise - ;; it will be overwritten the first time a Python buffer
Re: [DISCUSSION] Should we deprecate python-mode.el (alternative to built-in python.el) support in ob-python? (was: [BUG] ob-python: async session evaluation does not support python-mode.el [9.6-pre (
Ihor Radchenko writes: > However, supporting python-mode.el integration is difficult for Org > team: neither the current ob-python maintainer nor the rest of Org team > are familiar with python-mode.el. > > The bug discussed above already shows that parts of ob-python do not > work properly with third-party python-mode.el. > > Should we deprecate the support altogether and not bother with the extra > maintenance? Or maybe someone want to volunteer maintaining > python-mode.el integration? Thanks for this summary Ihor. In my experience, supporting both python.el and python-mode.el adds a high maintenance burden to ob-python. I think it would be better if ob-python could focus on python.el only, and a separate ob-python-mode created if there is demand for it. One cause of the higher maintenance burden, is that a lot of functionality needs to be implemented twice to support both modes. Or alternatively, functions need to be written in a way that's agnostic to the python mode -- but this also adds complexity, as working with comint directly can be tricky and bug-prone. Testing is another problem, as there isn't a way to use ob-python with both python modes in the same emacs config. So a separate emacs setup has to be maintained to test that python-mode works, which adds more maintenance headache. If python-mode support is removed, I'd be willing to help create a new ob-python-mode package, so that python-mode users can keep using Babel. However I don't normally use python-mode, so it would be best if a python-mode user could volunteer to help with this as well.
Re: [PATCH] Ignore flaky ob-python tests
Christian Köstlin writes: > I think we know of the async problem, and you reported that as well, > so I would expect > if someone is able to fix it, he will also move the tests to "sharp" again. Could you provide some references about the async problem, either how to reproduce it or the underlying cause? I tried searching emacs-orgmode and emacs-devel, but couldn't find a report of the problem.
Re: [BUG] ob-python: async session evaluation does not support python-mode.el [9.6-pre (release_9.5.5-2661-g2d040b.dirty @ /home/yantar92/.emacs.d/straight/build/org/)]
Hi Ihor, Ihor Radchenko writes: > However, async evaluation fails when org-babel-python-mode is set to > 'python-mode. As a stopgap, perhaps Org could issue an error message that async is not implemented for python-mode.el I was actually planning to deprecate ob-python support for python-mode.el, because I don't think it's worth the extra complexity & burden to support 2 python modes. For users who prefer python-mode, I think it would be better to have a separate ob-python-mode, perhaps in org-contrib. There was a related discussion about this [1] a couple years ago. However, I never got around to deprecating python-mode support, as I have been unable to work on ob-python for the last 1.5+ years due to employment changes and subsequent copyright problems (my employer's lawyers have some issues with the FSF's form, and I am waiting for the FSF to review their proposed changes). I was hoping it would be resolved by now, but it's still pending, and I should be removed as ob-python maintainer since my hands are tied. [1] https://list.orgmode.org/87d0a95eyz@bzg.fr/
Re: Bug? org-id-find not finding some IDs
Ihor Radchenko writes: > I suggest to bind org-id-track-globally to nil file-locally in that > backup file. Thanks for the suggestion. I tried it out, and it initially seemed to work, but eventually the backup file ended up in org-id-files. It seems like there is more than one way that files may be added to org-id-files, and not all will respect a locally-set org-id-track-globally. However, I found another solution: we can use write-region to append the entry to org-caldav-backup.org, without opening it. Then this avoids the problem in normal usage of org-caldav: org-caldav-backup.org won't end up in org-id-files, since it won't be open in Emacs. I've updated my PR to org-caldav accordingly. > If necessary, we may add an extra customization like > org-id-exclude-files to explicitly exclude files from ID tracking. I think this would be nice, but it isn't necessary for the time being, since I found another solution for org-caldav, as described above.
Re: Bug? org-id-find not finding some IDs
Ihor Radchenko writes: > Applied onto main via 8f5bf1725. > https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=8f5bf172556564df89fb16ce8ecec68c5b7f0221 My sincere apologies, but after a bit of testing, I found that my requested change had some unforeseen consequences, and I don't think it's a good idea anymore. So, I'd like to request reverting it. Doing some research, I found the behavior of org-id-update-id-locations, to not search all the open Org files, was made in this 2019 commit: https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=9865e6bd8be65229be4eac4f459f62e47fab2be7 The commit message suggests this change in behavior was intentional. Only, the docstring of org-id-extra-files wasn't updated to reflect the new behavior, which was the cause of confusion on my end. In terms of unforeseen consequences of the new commit: I found it caused some problems for org-caldav. When making changes, org-caldav copies entries into a backup file at ~/.emacs.d/org-caldav-backup.org. With the new commit, it is sometimes finding entries in this backup file instead of the correct file. To fix the original problem I was experiencing with org-caldav, I think it may be better to modify org-caldav, instead of upstream Org-mode. To that end, I submitted this PR to org-caldav: https://github.com/dengste/org-caldav/pull/250
Bug: Folding problem with markdown source block
Hello, I found that Org entries containing markdown source blocks don't get properly folded on the main development branch, when markdown-mode is also loaded. To reproduce: 1. Download markdown-mode from MELPA or Github. [1] 2. Fix the paths in the attached init.el. 3. emacs -Q -l init.el test.org 4. Shift-tab to collapse the visibility Output should look like this: * Headline 1... But instead I observe this: * Headline 1... ``` ... ``` ... If markdown-mode isn't loaded, then the problem goes away. I think the problem might have to do with the fontification that markdown-mode applies to the back-quoted code block. Versions: GNU Emacs 28.1 (build 2, x86_64-pc-linux-gnu, X toolkit, cairo version 1.17.6, Xaw3d scroll bars) of 2022-07-10 Org mode version 9.5.4 (release_9.5.4-717-g9cc60d @ /home/jack/dev/org-mode/lisp/) [1] https://github.com/jrblevin/markdown-mode (add-to-list 'load-path "~/.emacs.d/elpa/markdown-mode-20220708.6") (require 'markdown-mode) (add-to-list 'load-path "~/dev/org-mode/lisp") * Headline 1 ** Headline 1a #+begin_src markdown Some source code: ``` echo hello world ``` A list: ,* Item 1 ,* Item 2 #+end_src ** Headline 1b Lorem ipsum.