Subject: [PATCH] Fix DISPLAY error on exporting org with plantuml to html

Subject: [PATCH] Fix DISPLAY error on exporting org with plantuml to html

[Sun Lin]

[-- Attachment #1: Type: text/plain, Size: 908 bytes --]

Hi,

When the environment variable "DISPLAY" is invalid, exporting plantuml block in org will got error message:
Exception in thread "main" java.awt.AWTError: Can't connect to X11 window server using 'localhost:11.0' as the value of the DISPLAY variable.
    at sun.awt.X11GraphicsEnvironment.initDisplay(Native Method)
...
To reproduce this issue, install the plantuml.jar and make sure it work inside org-mode.
Then open `/tmp/testing.org` and  input follow plantuml code and eval-expression `(progn (setenv "DISPLAY" "localhost:90")(org-html-export-to-html))`.
The root cause is org-mord execute the plantuml.jar in command line without headless parameter, then plantuml will try read the DISPLAY, it's unnecessary.
(For standalone plantuml, `org-plantuml-executable-args` has default value '-headless' to avoid reading DISPLAY).

Please help review the patch. Thanks.

Best Regards
Lin

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-DISPLAY-error-on-exporting-org-with-plantuml-to-.patch --]
[-- Type: text/x-patch, Size: 927 bytes --]

From 5e04a800691b04edd940dd124987336f6f30dd62 Mon Sep 17 00:00:00 2001
From: Lin Sun <lin.sun@zoom.us>
Date: Sat, 16 Oct 2021 03:01:18 +0000
Subject: [PATCH] Fix DISPLAY error on exporting org with plantuml to html

* org-babel-execute:plantuml: using headless mode for org calling plantuml for exporting to html
---
 lisp/ob-plantuml.el | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lisp/ob-plantuml.el b/lisp/ob-plantuml.el
index fc62160..7d92227 100644
--- a/lisp/ob-plantuml.el
+++ b/lisp/ob-plantuml.el
@@ -122,6 +122,7 @@ This function is called by `org-babel-execute-src-block'."
 				((not (file-exists-p org-plantuml-jar-path))
 				 (error "Could not find plantuml.jar at %s" org-plantuml-jar-path))
 				(t (list java
+					 "-Djava.awt.headless=true"
 					 "-jar"
 					 (shell-quote-argument (expand-file-name org-plantuml-jar-path))))))
 	 (full-body (org-babel-plantuml-make-body body params))
-- 
2.7.0

Subject: [PATCH] Fix DISPLAY error on exporting org with plantuml to html

[Sun Lin]

[-- Attachment #1: Type: text/plain, Size: 1257 bytes --]


Hi Bastien,

Please help to review this patch. Thanks

Best Regards

Lin Sun

----- Forwarded Message -----

From: Sun Lin <sunlin7@yahoo.com>
To: emacs-orgmode@gnu.org <emacs-orgmode@gnu.org>
Sent: Saturday, October 16, 2021, 03:25:40 AM UTC
Subject: Subject: [PATCH] Fix DISPLAY error on exporting org with plantuml to html


Hi,

When the environment variable "DISPLAY" is invalid, exporting plantuml block in org will got error message:
Exception in thread "main" java.awt.AWTError: Can't connect to X11 window server using 'localhost:11.0' as the value of the DISPLAY variable.
    at sun.awt.X11GraphicsEnvironment.initDisplay(Native Method)
...
To reproduce this issue, install the plantuml.jar and make sure it work inside org-mode.
Then open `/tmp/testing.org` and  input follow plantuml code and eval-expression `(progn (setenv "DISPLAY" "localhost:90")(org-html-export-to-html))`.
The root cause is org-mord execute the plantuml.jar in command line without headless parameter, then plantuml will try read the DISPLAY, it's unnecessary.
(For standalone plantuml, `org-plantuml-executable-args` has default value '-headless' to avoid reading DISPLAY).

Please help review the patch. Thanks.

Best Regards
Lin

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-DISPLAY-error-on-exporting-org-with-plantuml-to-.patch --]
[-- Type: text/x-patch, Size: 927 bytes --]

From 5e04a800691b04edd940dd124987336f6f30dd62 Mon Sep 17 00:00:00 2001
From: Lin Sun <lin.sun@zoom.us>
Date: Sat, 16 Oct 2021 03:01:18 +0000
Subject: [PATCH] Fix DISPLAY error on exporting org with plantuml to html

* org-babel-execute:plantuml: using headless mode for org calling plantuml for exporting to html
---
 lisp/ob-plantuml.el | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lisp/ob-plantuml.el b/lisp/ob-plantuml.el
index fc62160..7d92227 100644
--- a/lisp/ob-plantuml.el
+++ b/lisp/ob-plantuml.el
@@ -122,6 +122,7 @@ This function is called by `org-babel-execute-src-block'."
 				((not (file-exists-p org-plantuml-jar-path))
 				 (error "Could not find plantuml.jar at %s" org-plantuml-jar-path))
 				(t (list java
+					 "-Djava.awt.headless=true"
 					 "-jar"
 					 (shell-quote-argument (expand-file-name org-plantuml-jar-path))))))
 	 (full-body (org-babel-plantuml-make-body body params))
-- 
2.7.0

Re: Subject: [PATCH] Fix DISPLAY error on exporting org with plantuml to html

[Bastien Guerry]

Hi,

Sun Lin <sunlin7@yahoo.com> writes:

> Please help to review this patch.

I will - but please be aware that we have a policy of waiting at least
one month before bumping a thread:

https://orgmode.org/worg/org-mailing-list.html#i-didnt-receive-an-answer

Thanks,

-- 
 Bastien

Re: Subject: [PATCH] Fix DISPLAY error on exporting org with plantuml to html

[Sun Lin]

Hi Bastien,

Okay, waiting a month is no problem for me, I'll give org version info in future patches.

The patch is for org version:
Org mode version 9.5 (release_9.5-46-gb71474 @ ~/.root/share/emacs/28.0.60/lisp/org/)

And the reproduce steps are edit /tmp/testing.org and eval-expression 
(progn (setenv "DISPLAY" "localhost:90")(org-html-export-to-html))

# testing.org
#+BEGIN_SRC plantuml :cmdline -charset UTF-8 :file testing.png
  @startuml
  a -> b: testing
  @enduml
#+END_SRC


Thanks.

--
Lin

Re: Subject: [PATCH] Fix DISPLAY error on exporting org with plantuml to html

[Sun Lin]

[-- Attachment #1: Type: text/plain, Size: 279 bytes --]

Hi Bastien,

> Hi,
> I will - but please be aware that we have a policy of waiting at least
> one month before bumping a thread:
> https://orgmode.org/worg/org-mailing-list.html#i-didnt-receive-an-answer
> Thanks,
> Bastien

Please help the patch, thanks.

Lin    

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-DISPLAY-error-on-exporting-org-with-plantuml-to-.patch --]
[-- Type: text/x-patch, Size: 927 bytes --]

From 5e04a800691b04edd940dd124987336f6f30dd62 Mon Sep 17 00:00:00 2001
From: Lin Sun <lin.sun@zoom.us>
Date: Sat, 16 Oct 2021 03:01:18 +0000
Subject: [PATCH] Fix DISPLAY error on exporting org with plantuml to html

* org-babel-execute:plantuml: using headless mode for org calling plantuml for exporting to html
---
 lisp/ob-plantuml.el | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lisp/ob-plantuml.el b/lisp/ob-plantuml.el
index fc62160..7d92227 100644
--- a/lisp/ob-plantuml.el
+++ b/lisp/ob-plantuml.el
@@ -122,6 +122,7 @@ This function is called by `org-babel-execute-src-block'."
 				((not (file-exists-p org-plantuml-jar-path))
 				 (error "Could not find plantuml.jar at %s" org-plantuml-jar-path))
 				(t (list java
+					 "-Djava.awt.headless=true"
 					 "-jar"
 					 (shell-quote-argument (expand-file-name org-plantuml-jar-path))))))
 	 (full-body (org-babel-plantuml-make-body body params))
-- 
2.7.0

Re: Subject: [PATCH] Fix DISPLAY error on exporting org with plantuml to html

[Timothy]

Hi Sun,

> [snipped patch description]
> Please help review the patch. Thanks.
> [2. text/x-patch; 0001-Fix-DISPLAY-error-on-exporting-org-with-plantuml-to-.patch]...

Thanks for your patience. This looks quite straight-forward. I don't use
or have plantuml so I'm in a poor position to test.

However, I can use google, and it seems that -Djava.awt.headless=true
just uses Java for some graphics operations which are normally delegated
to the system graphics stack.

This seems rather safe to me, and so I'd be inclined to take your word
that this is a beneficial change and merge the patch.

All the best,
Timothy

Re: Subject: [PATCH] Fix DISPLAY error on exporting org with plantuml to html

[Sun Lin]

Hi Timothy,

Thank you so much!

B.R.
Lin

Subject: [PATCH] Fix DISPLAY error on exporting org with plantuml to html

[Sun Lin]

Hi Timothy,

Can any help merge the patch? Or should I request Bastien to merge the patch? 
Thanks.

B.R.
Lin

>On Thursday, November 18, 2021, 07:42:24 AM UTC, Timothy <tecosaur@gmail.com> wrote:

>Hi Sun,

>> [snipped patch description]

>> Please help review the patch. Thanks.

>> [2. text/x-patch; 0001-Fix-DISPLAY-error-on-exporting-org-with-plantuml-to-.patch]...

>Thanks for your patience. This looks quite straight-forward. I don't use
>or have plantuml so I'm in a poor position to test.

>However, I can use google, and it seems that -Djava.awt.headless=true
>just uses Java for some graphics operations which are normally delegated
>to the system graphics stack.

>This seems rather safe to me, and so I'd be inclined to take your word
>that this is a beneficial change and merge the patch.

>All the best,

>Timothy

Re: Subject: [PATCH] Fix DISPLAY error on exporting org with plantuml to html

[Timothy]

[-- Attachment #1: Type: text/plain, Size: 628 bytes --]

Hi Sun,

> Can any help merge the patch? Or should I request Bastien to merge the patch? 

Thanks for not forgetting about this. As I commented before, I this looks
sensible enough to apply, and so I’ve just pushed it as f9dcc3d with a tweaked
commit message.

You’ve also been added to the list of TINYCHANGE contributors on
<https://orgmode.org/worg/contributors.html>. If you submit more than 20
cumulative lines of non-trivial code, you’ll need to get FSF assignment.

Thanks for sharing the issue and making a patch for it 🙂 hopefully we’ll see
you around in the future.

All the best,
Timothy

Re: Subject: [PATCH] Fix DISPLAY error on exporting org with plantuml to html

[Sun Lin]

Hi Timothy,

Thank you! : ) 

I had signed the FSF, RT: 1718442. 

B.R.
 Lin

On Tuesday, November 30, 2021, 04:22:03 AM UTC, Timothy <tecosaur@gmail.com> wrote: 
> Hi Sun,

> Thanks for not forgetting about this. As I commented before, I this looks
 sensible enough to apply, and so I’ve just pushed it as f9dcc3d with a tweaked
 commit message.

> You’ve also been added to the list of TINYCHANGE contributors on
 <https://orgmode.org/worg/contributors.html>. If you submit more than 20
 cumulative lines of non-trivial code, you’ll need to get FSF assignment.

> Thanks for sharing the issue and making a patch for it 🙂 hopefully we’ll see
 you around in the future.

Subject: [PATCH] Fix DISPLAY error on exporting org with plantuml to html

[lin Sun via General discussions about Org-mode.]

[-- Attachment #1.1: Type: text/plain, Size: 191 bytes --]

Hi Timothy,

The commit f9dcc3d was overridden by the commit b5f0efc.

This patch will re-submit the solution based on the last rev.

Please help review and merge the patch. Thanks

B.R.
Lin

[-- Attachment #1.2: Type: text/html, Size: 362 bytes --]

[-- Attachment #2: 0001-ob-plantuml-fix-DISPLAY-error-with-html-export.patch --]
[-- Type: text/x-patch, Size: 1059 bytes --]

From 6512e94806a4c08f40e6796e239ad2b318f7fbc5 Mon Sep 17 00:00:00 2001
From: Lin Sun <lin.sun@zoom.us>
Date: Tue, 7 Jun 2022 04:06:06 +0000
Subject: [PATCH] ob-plantuml: fix DISPLAY error with html export

* lisp/ob-plantuml.el (org-babel-execute:plantuml): Use headless mode
for Org calling plantuml for exporting to html to avoid issues with
operations that normally try to access the system graphics stack.
---
 lisp/ob-plantuml.el | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lisp/ob-plantuml.el b/lisp/ob-plantuml.el
index 6e1b1b1..37a631b 100644
--- a/lisp/ob-plantuml.el
+++ b/lisp/ob-plantuml.el
@@ -122,6 +122,7 @@ This function is called by `org-babel-execute-src-block'."
 				((not (file-exists-p org-plantuml-jar-path))
 				 (error "Could not find plantuml.jar at %s" org-plantuml-jar-path))
 				(t `(,java
+                                     "-Djava.awt.headless=true"
 				     "-jar"
 				     ,(shell-quote-argument (expand-file-name org-plantuml-jar-path))
                                      ,@org-plantuml-args))))
-- 
2.7.0

Re: Subject: [PATCH] Fix DISPLAY error on exporting org with plantuml to html

[Dr. Arne Babenhauserheide]

[-- Attachment #1: Type: text/plain, Size: 1581 bytes --]

Hi Lin Sun,

lin Sun via "General discussions about Org-mode." <emacs-orgmode@gnu.org> writes:
> This patch will re-submit the solution based on the last rev.
>
> Please help review and merge the patch. Thanks 
>
> [4. text/x-patch; 0001-ob-plantuml-fix-DISPLAY-error-with-html-export.patch]...
> From 6512e94806a4c08f40e6796e239ad2b318f7fbc5 Mon Sep 17 00:00:00 2001
> From: Lin Sun <lin.sun@zoom.us>
> Date: Tue, 7 Jun 2022 04:06:06 +0000
> Subject: [PATCH] ob-plantuml: fix DISPLAY error with html export
> 
> * lisp/ob-plantuml.el (org-babel-execute:plantuml): Use headless mode
> for Org calling plantuml for exporting to html to avoid issues with
> operations that normally try to access the system graphics stack.
> ---
>  lisp/ob-plantuml.el | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/lisp/ob-plantuml.el b/lisp/ob-plantuml.el
> index 6e1b1b1..37a631b 100644
> --- a/lisp/ob-plantuml.el
> +++ b/lisp/ob-plantuml.el
> @@ -122,6 +122,7 @@ This function is called by `org-babel-execute-src-block'."
>  				((not (file-exists-p org-plantuml-jar-path))
>  				 (error "Could not find plantuml.jar at %s" org-plantuml-jar-path))
>  				(t `(,java
> +                                     "-Djava.awt.headless=true"

This looks good and low-risk to me.

>  				     "-jar"
>  				     ,(shell-quote-argument (expand-file-name org-plantuml-jar-path))
>                                       ,@org-plantuml-args))))
> -- 
> 2.7.0

Best wishes,
Arne
-- 
Unpolitisch sein
heißt politisch sein,
ohne es zu merken.
draketo.de

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 1125 bytes --]

Re: Subject: [PATCH] Fix DISPLAY error on exporting org with plantuml to html

[Max Nikulin]

On 07/06/2022 13:34, Dr. Arne Babenhauserheide wrote:
> 
> lin Sun writes:
>> This patch will re-submit the solution based on the last rev.

>> diff --git a/lisp/ob-plantuml.el b/lisp/ob-plantuml.el
>> index 6e1b1b1..37a631b 100644
>> --- a/lisp/ob-plantuml.el
>> +++ b/lisp/ob-plantuml.el
>> @@ -122,6 +122,7 @@ This function is called by `org-babel-execute-src-block'."
>>   				((not (file-exists-p org-plantuml-jar-path))
>>   				 (error "Could not find plantuml.jar at %s" org-plantuml-jar-path))
>>   				(t `(,java
>> +                                     "-Djava.awt.headless=true"
> 
> This looks good and low-risk to me.

Could you, please, clarify why -headless in `org-plantuml-args' is not 
enough and -Djava.awt.headless=true should be added despite it looks 
redundant. Another question is why this option should be applied to jar 
but not to plantuml launcher script. The commit dropping this option has 
a link to discussion whether this argument is necessary and hard coded 
in such way.

> 
>>   				     "-jar"
>>   				     ,(shell-quote-argument (expand-file-name org-plantuml-jar-path))
>>                                        ,@org-plantuml-args))))

Re: Subject: [PATCH] Fix DISPLAY error on exporting org with plantuml to html

[lin Sun]

[-- Attachment #1: Type: text/plain, Size: 88 bytes --]

Hi Arne,

Thanks a lot!

Look forward to seeing who can merge this patch. : )

B.R.
Lin

[-- Attachment #2: Type: text/html, Size: 191 bytes --]

Re: Subject: [PATCH] Fix DISPLAY error on exporting org with plantuml to html

[Ihor Radchenko]

lin Sun <sunlin7@yahoo.com> writes:

> The commit f9dcc3d was overridden by the commit b5f0efc.
>
> This patch will re-submit the solution based on the last rev.
>
> Please help review and merge the patch. Thanks

I am unable to reproduce the original error on latest main.
Could you please provide a reproducer when the proposed patch is
actually useful?

Best,
Ihor

Re: Subject: [PATCH] Fix DISPLAY error on exporting org with plantuml to html

[lin Sun]

Hi lhor,

You're totally right, the commit b5f0efc had appended the `-headless`
to the jar parameter list. Here is the comment:
> *** =org-plantump-executable-args= is renamed and applies to jar as well
The original issue does not exist now.  Please ignore the previous patch.

Thank you so much.

B.R.
Lin

[PATCH] before emit an error message, try to load the babel language

[lin Sun]

[-- Attachment #1: Type: text/plain, Size: 688 bytes --]

Hi,
The function `org-babel-execute-src-block' will emit an error message
if the language is not loaded.

Before the error message, the patch will give a try to load the
associated babel language.

Why does the language exist in the `lang org-babel-load-languages' but
not be loaded?
Because the `org-babel-load-languages' are defined as a customer
variable with  :set 'org-babel-do-load-languages,  the ":set" function
can only be triggered with function `custom-set-variables`.
While setq/cl-pushnew `org-babel-load-languages' won't trigger the
`:set' function, then the error message will be displayed if I run the
code in the org file.

Please help review that patch. Thanks. Regards

[-- Attachment #2: 0001-lisp-ob-core.el-load-lang-in-org-babel-execute-src-b.patch --]
[-- Type: text/x-patch, Size: 952 bytes --]

From 39a15a34bdb8bd0c397fbd80cdfb0723ce229066 Mon Sep 17 00:00:00 2001
From: Lin Sun <sunlin7@hotmail.com>
Date: Sat, 22 Apr 2023 00:00:39 +0000
Subject: [PATCH] *lisp/ob-core.el: load lang in (org-babel-execute-src-block)

---
 lisp/ob-core.el | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lisp/ob-core.el b/lisp/ob-core.el
index 471887a3a..2ed4f6b5b 100644
--- a/lisp/ob-core.el
+++ b/lisp/ob-core.el
@@ -795,7 +795,9 @@ guess will be made."
 		 (cmd (intern (concat "org-babel-execute:" lang)))
 		 result exec-start-time)
 	    (unless (fboundp cmd)
-	      (error "No org-babel-execute function for %s!" lang))
+              (if (alist-get lang org-babel-load-languages)
+                  (require (intern (concat "ob-" lang)))
+                (error "No org-babel-execute function for %s!" lang)))
 	    (message "Executing %s %s %s..."
 		     (capitalize lang)
                      (pcase executor-type
-- 
2.20.5

Re: [PATCH] before emit an error message, try to load the babel language

[Ruijie Yu via General discussions about Org-mode.]

lin Sun <sunlin7@yahoo.com> writes:

> Hi,
> The function `org-babel-execute-src-block' will emit an error message
> if the language is not loaded.
>
> Before the error message, the patch will give a try to load the
> associated babel language.
>
> Why does the language exist in the `lang org-babel-load-languages' but
> not be loaded?
> Because the `org-babel-load-languages' are defined as a customer
> variable with  :set 'org-babel-do-load-languages,  the ":set" function
> can only be triggered with function `custom-set-variables`.
> While setq/cl-pushnew `org-babel-load-languages' won't trigger the
> `:set' function, then the error message will be displayed if I run the
> code in the org file.
>
> Please help review that patch. Thanks. Regards
>
> [2. text/x-patch; 0001-lisp-ob-core.el-load-lang-in-org-babel-execute-src-b.patch]...

I think this patch slightly modifies the logic of the error.

--8<---------------cut here---------------start------------->8---
 	    (unless (fboundp cmd)
-	      (error "No org-babel-execute function for %s!" lang))
+              (if (alist-get lang org-babel-load-languages)
+                  (require (intern (concat "ob-" lang)))
+                (error "No org-babel-execute function for %s!" lang)))
--8<---------------cut here---------------end--------------->8---

You are saying, that when (fboundp cmd) returns nil, and when (alist-get
...) returns non-nil, then perform the `require', and assume this
function is now available.

Instead of that, I think you should have some sort of conditional in the
`unless' condition.  Something like:

--8<---------------cut here---------------start------------->8---
(unless (or (fboundp cmd)
            (ignore (and (alist-get lang org-babel-load-languages)
                         (require (intern (concat "ob-" lang)))))
            (fboundp cmd))
  (error "... %s" lang))
--8<---------------cut here---------------end--------------->8---

This preserves the logic of the message: when we cannot get
"org-babel-execute" immediately, we then try to require the module and
try again.  Then, we check the function again and conclude that the
org-babel-execute function is not found for this language.

Thoughts?

-- 
Best,


RY

[Please note that this mail might go to spam due to some
misconfiguration in my mail server -- still investigating.]

Re: [PATCH] before emit an error message, try to load the babel language

[Ihor Radchenko]

lin Sun <sunlin7@yahoo.com> writes:

> Before the error message, the patch will give a try to load the
> associated babel language.
>
> Why does the language exist in the `lang org-babel-load-languages' but
> not be loaded?
> Because the `org-babel-load-languages' are defined as a customer
> variable with  :set 'org-babel-do-load-languages,  the ":set" function
> can only be triggered with function `custom-set-variables`.
> While setq/cl-pushnew `org-babel-load-languages' won't trigger the
> `:set' function, then the error message will be displayed if I run the
> code in the org file.

If we want to solve the problem of setting `org-babel-load-languages'
outside `defcustom', we should also consider unloading and other places
that use `fboundp' to check if babel backend is loaded. IMHO, the
cleanest way will be using `add-variable-watcher'.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [PATCH] before emit an error message, try to load the babel language

[lin Sun]

[-- Attachment #1: Type: text/plain, Size: 1048 bytes --]

On Sat, Apr 22, 2023 at 12:42 PM Ihor Radchenko <yantar92@posteo.net> wrote:
> the cleanest way will be using `add-variable-watcher'.
Hi Ihor,
The "add-variable-watcher" may not be able to deal with the
local/global variable `org-babel-load-languages' correctly.

On Sat, Apr 22, 2023 at 8:33 AM Ruijie Yu <ruijie@netyu.xyz> wrote:
> Instead of that, I think you should have some sort of conditional in the
> `unless' condition.  Something like:
> --8<---------------cut here---------------start------------->8---
> (unless (or (fboundp cmd)
>             (ignore (and (alist-get lang org-babel-load-languages)
>                          (require (intern (concat "ob-" lang)))))
>             (fboundp cmd))
>   (error "... %s" lang))
> --8<---------------cut here---------------end--------------->8---
The logic in the patch will follow the function
`org-babel-do-load-languages' to load the ob-LANG without capturing
the error.
I changed the patch to follow your comments.
Please review the new patch I attached. Thanks.

[-- Attachment #2: 0001-lisp-ob-core.el-load-lang-in-org-babel-execute-src-b.patch --]
[-- Type: text/x-patch, Size: 1542 bytes --]

From acc72f9f42120d07cc728eeebb9dd976b4568c1f Mon Sep 17 00:00:00 2001
From: Lin Sun <sunlin7@hotmail.com>
Date: Sat, 22 Apr 2023 00:00:39 +0000
Subject: [PATCH] *lisp/ob-core.el: load lang in (org-babel-execute-src-block)

---
 lisp/ob-core.el | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/lisp/ob-core.el b/lisp/ob-core.el
index 471887a3a..0fc4cbe9f 100644
--- a/lisp/ob-core.el
+++ b/lisp/ob-core.el
@@ -714,6 +714,8 @@ a list with the following pattern:
 (defvar *this*) ; Dynamically bound in `org-babel-execute-src-block'
                 ; and `org-babel-read'
 
+(defvar org-babel-load-languages)
+
 ;;;###autoload
 (defun org-babel-execute-src-block (&optional arg info params executor-type)
   "Execute the current source code block and return the result.
@@ -794,7 +796,9 @@ guess will be made."
 		       d))))
 		 (cmd (intern (concat "org-babel-execute:" lang)))
 		 result exec-start-time)
-	    (unless (fboundp cmd)
+	    (unless (or (fboundp cmd)
+			(and (alist-get (intern lang) org-babel-load-languages)
+			     (require (intern (concat "ob-" lang)) nil t)))
 	      (error "No org-babel-execute function for %s!" lang))
 	    (message "Executing %s %s %s..."
 		     (capitalize lang)
@@ -1952,8 +1956,6 @@ With optional prefix argument ARG, jump backward ARG many source blocks."
   (interactive "p")
   (org-previous-block arg org-babel-src-block-regexp))
 
-(defvar org-babel-load-languages)
-
 ;;;###autoload
 (defun org-babel-mark-block ()
   "Mark current source block."
-- 
2.20.5

Re: [PATCH] before emit an error message, try to load the babel language

[Ihor Radchenko]

lin Sun <sunlin7@yahoo.com> writes:

> On Sat, Apr 22, 2023 at 12:42 PM Ihor Radchenko <yantar92@posteo.net> wrote:
>> the cleanest way will be using `add-variable-watcher'.
> The "add-variable-watcher" may not be able to deal with the
> local/global variable `org-babel-load-languages' correctly.

May you elaborate?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [PATCH] before emit an error message, try to load the babel language

[lin Sun]

Hi Ihor,

Thanks for the comment.
If we switch to the `add-variable-watcher', it seems we'll load the
ob-LANG in the callback immediately.
The previous change can lazyly load the babel languages until the user
tries to evaluate the source block.

Re: [PATCH] before emit an error message, try to load the babel language

[Ihor Radchenko]

lin Sun <sunlin7@yahoo.com> writes:

> Thanks for the comment.
> If we switch to the `add-variable-watcher', it seems we'll load the
> ob-LANG in the callback immediately.
> The previous change can lazyly load the babel languages until the user
> tries to evaluate the source block.

Is there any advantage doing the loading lazily?
I can only see disadvantages:

1. `setopt' already triggers loading when variable is set. It is just
   `setq', often used in place of `setopt', that does not trigger
   loading/unloading.
2. Lazy loading like you proposed will have to be added in a number of
   places, not just when executing src block: `org-edit-special',
   `org-src-associate-babel-session', `org-edit-src-code',
   `pcomplete/org-mode/block-option/src', org-lint,
   `org-babel-tangle-single-block', `org-babel-exp-src-block',
   `org-babel-exp-results', and others.
3. We will also have to monitor newly added babel functions to take care
   about lazy loading. Not to mention third-party code.

Admittedly, (2) can be solved by abstracting babel backend queries to
API. (I have a WIP code doing exactly this). However, I still see no
good reason to make `setq' different from what `setopt' does now.
Most people treat them interchangeably and then get surprised by the
differences.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [PATCH] before emit an error message, try to load the babel language

[Ruijie Yu via General discussions about Org-mode.]

Ihor Radchenko <yantar92@posteo.net> writes:

> lin Sun <sunlin7@yahoo.com> writes:
>
>> Thanks for the comment.
>> If we switch to the `add-variable-watcher', it seems we'll load the
>> ob-LANG in the callback immediately.
>> The previous change can lazyly load the babel languages until the user
>> tries to evaluate the source block.
>
> Is there any advantage doing the loading lazily?
> I can only see disadvantages:
>
> 1. `setopt' already triggers loading when variable is set. It is just
>    `setq', often used in place of `setopt', that does not trigger
>    loading/unloading.

Speaking of which, since we now (or are about to?) depend on compat, we
can just do (require 'compat) and forget that `setopt' is an Emacs 29
addition.

-- 
Best,


RY

[Please note that this mail might go to spam due to some
misconfiguration in my mail server -- still investigating.]

Re: [PATCH] before emit an error message, try to load the babel language

[Ihor Radchenko]

Ruijie Yu <ruijie@netyu.xyz> writes:

>> 1. `setopt' already triggers loading when variable is set. It is just
>>    `setq', often used in place of `setopt', that does not trigger
>>    loading/unloading.
>
> Speaking of which, since we now (or are about to?) depend on compat, we
> can just do (require 'compat) and forget that `setopt' is an Emacs 29
> addition.

No. Compat does not support and does not plan to support `setopt' (see
https://elpa.gnu.org/packages/doc/compat.html#Limitations).
And it is not us who needs to use `setopt', but users.
We just have to deal with de facto situation with users using `setq'
mindlessly.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [PATCH] before emit an error message, try to load the babel language

[Ruijie Yu via General discussions about Org-mode.]

Ihor Radchenko <yantar92@posteo.net> writes:

> Ruijie Yu <ruijie@netyu.xyz> writes:
>
>>> 1. `setopt' already triggers loading when variable is set. It is just
>>>    `setq', often used in place of `setopt', that does not trigger
>>>    loading/unloading.
>>
>> Speaking of which, since we now (or are about to?) depend on compat, we
>> can just do (require 'compat) and forget that `setopt' is an Emacs 29
>> addition.
>
> No. Compat does not support and does not plan to support `setopt' (see
> https://elpa.gnu.org/packages/doc/compat.html#Limitations).
> And it is not us who needs to use `setopt', but users.
> We just have to deal with de facto situation with users using `setq'
> mindlessly.

Makes sense.  So, does that mean nothing should be changed, except for
maybe some additions to documentation mentioning that the users should
avoid using `setq' to set customizable variables, and instead use
`setopt' if >= 29, or [some function in cus-edit.el which I don't
recall] otherwise?

-- 
Best,


RY

[Please note that this mail might go to spam due to some
misconfiguration in my mail server -- still investigating.]

Re: [PATCH] before emit an error message, try to load the babel language

[lin Sun]

Thank you all for the comments, and agree with no code change.
Please feel free to close this ticket. Thanks

Re: [PATCH] before emit an error message, try to load the babel language

[Ihor Radchenko]

lin Sun <sunlin7@yahoo.com> writes:

> Thank you all for the comments, and agree with no code change.
> Please feel free to close this ticket. Thanks

Will you still be interested to improve the patch covering all other
cases in Org code where babel backend is used? Or using
`add-variable-watcher'?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [PATCH] before emit an error message, try to load the babel language

[lin Sun]

Hi Ihor,

I use the `setopt` in my dot-emacs now, and won't make code changes.
Please feel free to close this ticket, and any code change to enhance
the discussed issue is welcome. Thanks

Re: [PATCH] before emit an error message, try to load the babel language

[Ihor Radchenko]

lin Sun <sunlin7@yahoo.com> writes:

> I use the `setopt` in my dot-emacs now, and won't make code changes.
> Please feel free to close this ticket, and any code change to enhance
> the discussed issue is welcome. Thanks

Ok.
Canceled.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [PATCH] before emit an error message, try to load the babel language

[Ihor Radchenko]

Ruijie Yu <ruijie@netyu.xyz> writes:

>> And it is not us who needs to use `setopt', but users.
>> We just have to deal with de facto situation with users using `setq'
>> mindlessly.
>
> Makes sense.  So, does that mean nothing should be changed, except for
> maybe some additions to documentation mentioning that the users should
> avoid using `setq' to set customizable variables, and instead use
> `setopt' if >= 29, or [some function in cus-edit.el which I don't
> recall] otherwise?

Not really. It is a common problem that users keep using `setq' instead
of `setopt', customize interface, or `custom-set-variables'.

Lin Sun's solution is one approach to deal with this problem for
`org-babel-load-languages', but only partial, as I outlined.
I'd prefer to use either `add-variable-watcher' or oc.el-like API with
`org-cite-try-load-processor' being called when babel backend
information is requested.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>