Re: [PATCH] org-babel-demarcate-block: split using element API

[Ihor Radchenko]

gerard.vermeu...@posteo.net writes:

> I resubmit my patch (attached) without any caveats in the commit 
> message.

Thanks!
Applied, onto main.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=c2ea553be

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: [PATCH] org-babel-demarcate-block: split using element API

[gerard . vermeulen]

On 04.03.2024 11:12, Ihor Radchenko wrote:


What about after
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=5f5db3d35
?


This fixes the bug.
I resubmit my patch (attached) without any caveats in the commit 
message.


Regards -- Gerard



0001-org-babel-demarcate-block-split-using-element-API.patch
Description: Binary data

Re: [PATCH] org-babel-demarcate-block: split using element API

[Ihor Radchenko]

gerard.vermeu...@posteo.net writes:

>> Have you tried the latest main?
>
> Yes (just tried again), the minimal function still triggers the 
> "infinite" list of warnings
> "Warning (org-element): ‘org-element-at-point’ cannot be used in non-Org 
> buffer # (python-mode)"
> It is coming from the last org-indent-block call even though the message
> just before tells that the mode is derived from org-mode.

What about after
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=5f5db3d35
?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: [PATCH] org-babel-demarcate-block: split using element API

[gerard . vermeulen]

On 03.03.2024 14:08, Ihor Radchenko wrote:

gerard.vermeu...@posteo.net writes:


I have reduced my version of `org-babel-demarcate-block' to a minimal
function to locate the bug stemming from edit-prep signaling an
user-error or not.  In case edit-prep signals an user-error the call
chain `org-indent-block', `org-indent-region', `org-element-at-point'
triggers an infinite list of warnings (major mode is Python when run
on the test block).


Have you tried the latest main?


Yes (just tried again), the minimal function still triggers the 
"infinite" list of warnings
"Warning (org-element): ‘org-element-at-point’ cannot be used in non-Org 
buffer # (python-mode)"

It is coming from the last org-indent-block call even though the message
just before tells that the mode is derived from org-mode.

Your changes of last thursday helped me to narrow the problem down to 
this call.


Regards -- Gerard

Re: [PATCH] org-babel-demarcate-block: split using element API

[Ihor Radchenko]

gerard.vermeu...@posteo.net writes:

> I have reduced my version of `org-babel-demarcate-block' to a minimal
> function to locate the bug stemming from edit-prep signaling an
> user-error or not.  In case edit-prep signals an user-error the call
> chain `org-indent-block', `org-indent-region', `org-element-at-point'
> triggers an infinite list of warnings (major mode is Python when run
> on the test block).

Have you tried the latest main?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: [PATCH] org-babel-demarcate-block: split using element API

[gerard . vermeulen]

On 29.02.2024 12:56, Ihor Radchenko wrote:

gerard.vermeu...@posteo.net writes:


That includes `org-babel-demarcate-block' splitting with the patch.

I do not understand why it works and why I never see the user-error
re-signalled by `org-babel-edit-prep:sql' (even as demoted message).


[...]

I have reduced my version of `org-babel-demarcate-block' to a minimal
function to locate the bug stemming from edit-prep signaling an
user-error or not.  In case edit-prep signals an user-error the call
chain `org-indent-block', `org-indent-region', `org-element-at-point'
triggers an infinite list of warnings (major mode is Python when run
on the test block).
#+begin_src emacs-lisp -n :results silent
(defun oeap-test ()
  "Test `org-element-at-point': call with point at block."
  (interactive)
  (let* ((info (org-babel-get-src-block-info 'noeval))
 (start (org-babel-where-is-src-block-head))
 (body (and start (match-string 5
(if (and info start)
(let* ((copy (org-element-copy (org-element-at-point)))
   (before (org-element-begin copy))
   (beyond (org-element-end copy)))
  (org-element-put-property copy :value body)
  (delete-region before beyond)
  (insert (org-element-interpret-data copy))
  (org-babel-previous-src-block 1)
  (message "Mode derived from: %S" (derived-mode-p 'org-mode))
  ;; Triggers list of warnings and condition-case is no 
solution:

  (org-indent-block)
#+end_src
Working edit-prep:
#+begin_src emacs-lisp -n :results silent
(defun harm-full-edit-prep (_info)
  (user-error "Harm-FULL edit-prep"))

(defun org-babel-edit-prep:python (info)
  (condition-case nil
  (harm-full-edit-prep info)
(t nil)))

(message "Working edit-prep:python")
#+end_src
Failing edit-prep:
#+begin_src emacs-lisp -n :results silent
(defun harm-full-edit-prep (_info)
  (user-error "Harm-FULL edit-prep"))

(defun org-babel-edit-prep:python (info)
  (harm-full-edit-prep info))

(message "Failing edit-prep:python")
#+end_src
Test block:
#+begin_src python -i -n :results silent
11
22
#+end_src
I do not like to put extra constraints on edit-prep functions. Maybe,
it is better to cancel the patch.

Regards -- Gerard

Re: [PATCH] org-babel-demarcate-block: split using element API

[Ihor Radchenko]

gerard.vermeu...@posteo.net writes:

> That includes `org-babel-demarcate-block' splitting with the patch.
>
> I do not understand why it works and why I never see the user-error
> re-signalled by `org-babel-edit-prep:sql' (even as demoted message).

Because `org-babel-edit-prep:sql' does not signal anything. It simply
returns a string:

In
(condition-case nil
(sql-set-product product)
  (user-error "Cannot set `sql-product' in Org Src edit buffer"))

(user-error ) means "If we encounter user-error, do ".
That code is certainly misleading.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: [PATCH] org-babel-demarcate-block: split using element API

[gerard . vermeulen]

On 28.02.2024 12:54, Ihor Radchenko wrote:

gerard.vermeu...@posteo.net writes:


You may wrap `org-indent-block' into `condition-case' to catch
user-errors.


The caveat is not a real constraint, since Org has limited support for
source block editing in an Org mode buffer when an
`org-babel-edit-prep:' function signals an user-error.

I show that in the attached no-user-errors-in-edit-prep.org.


I studied the existing Org handling of various errors related to src
edit buffers and it seems that we tend to ignore them in a number of
scenarios. In particular, when major mode fails to load for any reason,
Org mode does not even throw an error, but simply displays a warning.

So, I think that we can similarly ignore errors in edit-prep function,
demoting them to messages.

(In addition, it does not look like electric-indent-mode triggered in
your example file by pressing  handles errors gracefully either -
yet another reason not to throw errors in `org-indent-*' functions)

Does it make sense?


Your reply helped me to read the Org Babel code from a different view
point.  I re-discovered that `org-babel-edit-prep:sql' handles all
issues gracefully while trying the block below:
#+begin_src sql :engine wrong
a
b
#+end_src
That includes `org-babel-demarcate-block' splitting with the patch.

I do not understand why it works and why I never see the user-error
re-signalled by `org-babel-edit-prep:sql' (even as demoted message).

After writing `org-babel-edit-prep:python' like:
#+begin_src emacs-lisp -n :results silent
(defun harm-full-edit-prep (_info)
  (user-error "Harm-FULL edit-prep"))

(defun org-babel-edit-prep:python (info)
  "Imitate `org-babel-edit-prep:sql'."
  (condition-case nil
  (harm-full-edit-prep info)
(user-error "Why is this harm-LESS in `org-babel-edit-prep:sql'?")))
#+end_src
and trying it on the block below:
#+begin_src python -i -n :results silent
11
22
#+end_src
I see that this `org-babel-edit-prep:python' handles all issues like
`org-babel-edit-prep:sql' (and it does not matter whether
`electric-indent-mode' is disabled or enabled).

I conclude (contrary to the previous commit message in the patch):

In case functions called by an `org-babel-edit-prep:' function
raise an user-error, this `org-babel-edit-prep:' function should
re-signal the user-error like `org-babel-edit-prep:sql' does.

Attached you'll find the patch with an updated commit message.

Regards -- Gerard


0001-org-babel-demarcate-block-split-using-element-API.patch
Description: Binary data

Re: [PATCH] org-babel-demarcate-block: split using element API

[Ihor Radchenko]

gerard.vermeu...@posteo.net writes:

>> You may wrap `org-indent-block' into `condition-case' to catch
>> user-errors.
>
> The caveat is not a real constraint, since Org has limited support for
> source block editing in an Org mode buffer when an
> `org-babel-edit-prep:' function signals an user-error.
>
> I show that in the attached no-user-errors-in-edit-prep.org.

I studied the existing Org handling of various errors related to src
edit buffers and it seems that we tend to ignore them in a number of
scenarios. In particular, when major mode fails to load for any reason,
Org mode does not even throw an error, but simply displays a warning.

So, I think that we can similarly ignore errors in edit-prep function,
demoting them to messages.

(In addition, it does not look like electric-indent-mode triggered in
your example file by pressing  handles errors gracefully either -
yet another reason not to throw errors in `org-indent-*' functions)

Does it make sense?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: [PATCH] org-babel-demarcate-block: split using element API

[gerard . vermeulen]

On 25.02.2024 13:21, Ihor Radchenko wrote:

gerard.vermeu...@posteo.net writes:


I added the caveat:
This patch is incompatible with `org-babel-edit-prep:' functions
that signal `user-error's.
to the commit message and cleaned it up a bit.


You may wrap `org-indent-block' into `condition-case' to catch
user-errors.


Sorry, I did attach an old version of no-user-errors-in-edit-prep.org.
Corrected.

Regards -- Gerard


no-user-errors-in-edit-prep.org
Description: Binary data

Re: [PATCH] org-babel-demarcate-block: split using element API

[gerard . vermeulen]

On 25.02.2024 13:21, Ihor Radchenko wrote:

gerard.vermeu...@posteo.net writes:


I added the caveat:
This patch is incompatible with `org-babel-edit-prep:' functions
that signal `user-error's.
to the commit message and cleaned it up a bit.


You may wrap `org-indent-block' into `condition-case' to catch
user-errors.


The caveat is not a real constraint, since Org has limited support for
source block editing in an Org mode buffer when an
`org-babel-edit-prep:' function signals an user-error.

I show that in the attached no-user-errors-in-edit-prep.org.

I also attach condition-case.diff that I use to try to argue that
condition-case never gets into its error-handler (I am running the
code with the condition-case now), because I never see the signal of
~(user-error "Error in `org-babel-edit-prep:'?")~ (I needed to
read the condition-case documentation, because this is the first time
I am trying to use it).

#+begin_src emacs-lisp -n :results silent
;; Fails with org-babel-demarcate-block in patch.
(defun edit-prep-user-error (_info)
  (user-error "Signaling user-errors is harmfull"))

(defun org-babel-edit-prep:python (info)
  (edit-prep-user-error info))

(message "Harm-FULL edit-prep:python")
#+end_src

#+begin_src emacs-lisp -n :results silent
;; Works with org-babel-demarcate-block in patch.
(defun edit-prep-message (_info)
  (message "Displaying messages is harmless"))

(defun org-babel-edit-prep:python (info)
  (edit-prep-message info))

(message "Harm-LESS edit-prep:python")
#+end_src

~org-babel-demarcate-blocks~ works with "Harm-LESS edit-prep:python"
but fails with "Harm-FULL edit-prep:python" and it leaves the Org mode
buffer in a state similar to shown in no-user-errors-in-edit-prep.org
(I have to pass through org-edit-src-code to be able to edit the block
in the Org mode buffer).

But I only can do that after interrupting (C-g C-g) an infinite stream
of warnings "Warning (org-element): ‘org-element-at-point’ cannot be
used in non-Org buffer #
(python-mode)" which should come from the `org-element-at-point' call
indicated by ;; <= HERE? in condition-case.diff.

Test block below:

#+begin_src python -i -n :results silent
11
22
#+end_src

Regards -- Gerard


no-user-errors-in-edit-prep.org
Description: Binary data


condition-case.diff
Description: Binary data

Re: [PATCH] org-babel-demarcate-block: split using element API

[Ihor Radchenko]

gerard.vermeu...@posteo.net writes:

> I added the caveat:
> This patch is incompatible with `org-babel-edit-prep:' functions
> that signal `user-error's.
> to the commit message and cleaned it up a bit.

You may wrap `org-indent-block' into `condition-case' to catch
user-errors.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: [PATCH] org-babel-demarcate-block: split using element API

[gerard . vermeulen]

On 23.02.2024 14:43, Ihor Radchenko wrote:

gerard.vermeu...@posteo.net writes:


[...]


I rewrote my `org-babel-edit-prep:python' to get rid of user-errors 
but

then I bumped on the user-error coming from
`org-src--make-source-overlay'.


This is strange. `org-src--make-source-overlay' does not use
`org-element' API. I cannot see how you are getting such warnings from 
there.


I am using since a few days a clone of the mailed 
`org-babel-edit-prep:python'
function (an irrelevant fix for setting point) and it works without 
glitches.


I have no explanation of what happened.

I added the caveat:
This patch is incompatible with `org-babel-edit-prep:' functions
that signal `user-error's.
to the commit message and cleaned it up a bit.

I also removed the scaffolding from the tests that checks where mark and 
point are.

The tests pass with make test.

Patch attached.

Regards -- Gerard

0001-org-babel-demarcate-block-split-using-element-API.patch
Description: Binary data

Re: [PATCH] org-babel-demarcate-block: split using element API

[Ihor Radchenko]

gerard.vermeu...@posteo.net writes:

>>> May you please provide more details?
>> 
> This is different of what I saw before:
>
> When splitting python blocks in an org file I got a long list of 
> warnings:
> Warning (org-element): ‘org-element-at-point’ cannot be used in non-Org 
> buffer # (python-mode)
> until it stopped by itself.
>
> I traced this to user-errors in my own `org-babel-edit-prep:python' 
> which I
> use to let eglot handle python source blocks.
>
> I rewrote my `org-babel-edit-prep:python' to get rid of user-errors but
> then I bumped on the user-error coming from 
> `org-src--make-source-overlay'.

This is strange. `org-src--make-source-overlay' does not use
`org-element' API. I cannot see how you are getting such warnings from there.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: [PATCH] org-babel-demarcate-block: split using element API

[gerard . vermeulen]

On 21.02.2024 19:19, gerard.vermeu...@posteo.net wrote:

On 21.02.2024 10:40, Ihor Radchenko wrote:

gerard.vermeu...@posteo.net writes:

[...]

May you please provide more details?



This is different of what I saw before:

When splitting python blocks in an org file I got a long list of 
warnings:
Warning (org-element): ‘org-element-at-point’ cannot be used in non-Org 
buffer # (python-mode)

until it stopped by itself.

I traced this to user-errors in my own `org-babel-edit-prep:python' 
which I

use to let eglot handle python source blocks.

I rewrote my `org-babel-edit-prep:python' to get rid of user-errors but
then I bumped on the user-error coming from 
`org-src--make-source-overlay'.


I think that this may be the case for other definitions of 
`org-babel-edit-prep:LANG'


I copy my `org-babel-edit-edit-prep:python' without user-errors below:

#+begin_src emacs-lisp -n :results silent
(with-eval-after-load 'emacs
  (defcustom eglot-maybe-ensure-modes '(python-mode)
"Modes where maybe `eglot-ensure' should be or has been called.
This may be in the case of proper directory local variables or in
the case of proper `org-src-mode' buffers.")

  ;; https://www.reddit.com/r/emacs/comments/w4f4u3
  ;; /using_rustic_eglot_and_orgbabel_for_literate/
  (defun eglot-org-babel-edit-prep (info)
"Try to setup an `org-mode-src' buffer to make `eglot-ensure' 
succeed.

INFO has a form similar to the return value of
`org-babel-get-src-block-info'.  Try to load the tangled file
into the `org-src-mode' buffer as well as to narrow the region to
the Org-mode source block code before calling `eglot-ensure'."
(when-let ((ok (bound-and-true-p org-src-mode))
   (mark (point))
   (body (nth 1 info))
   (filename (cdr (assq :tangle (nth 2 info)
  (when (string= filename "no")
(message "Org source block has no tangled file")
(setq ok nil))
  (when ok
(setq filename (expand-file-name filename))
(unless (file-readable-p filename)
  (message "Tangled file %s is not readable" filename)
  (setq ok nil)))
  (when ok
(with-temp-buffer
  (insert-file-contents filename 'visit nil nil 'replace)
  (unless (search-forward body nil 'noerror)
(message "Org source block does not occur in tangled file 
%s"

 filename)
(setq ok nil))
  (when (search-forward body nil 'noerror)
(message "Org source block occurs twice or more in tangled 
file %s"

 filename)
(setq ok nil
  (when ok
(goto-char (point-min))
(insert-file-contents filename 'visit nil nil 'replace)
(search-forward body)
(narrow-to-region (match-beginning 0) (match-end 0))
(goto-char mark)
(eglot-ensure))
  ;; (message "Buffer %s is no `org-src-mode' buffer" (buffer-name))
  ))

  (defun org-babel-edit-prep:python (info)
(eglot-org-babel-edit-prep info)))
#+end_src

Regards -- Gerard

Re: [PATCH] org-babel-demarcate-block: split using element API

[gerard . vermeulen]

On 21.02.2024 10:40, Ihor Radchenko wrote:

gerard.vermeu...@posteo.net writes:


Still failing on my side (when running tests non-interactively from
command line). To fix the problem, please use the approach from
`test-org-list/indent-item':

(transient-mark-mode 1)
(push-mark (point) t t)

Instead of (set-mark-command nil)


Gerard, may I know if you had a chance to look into my comments?


I think that I have addressed this particular comment.


Not really.
In any case, see the attached updated patch with my suggestion
incorporated.

Indeed, I did not address it.  I also tried out your suggestion out this 
morning

and saw that it makes "make test" pass.



[...]



Secondly, I see (saw) sometimes Org emitting warnings with backtraces
starting from my patch.  I think the warning may be due either to a
mistake
on my side or a bug in Org, but I am not sure.


May you please provide more details?


I fail to reproduce the warnings, but I think that I have seen (rx ... )
type warnings in the  Emacs warnings buffer with the patched
org-babel-demarcate-block as backtrace entry point.  I did not capture
those at the time, because I thought I could trigger those warnings 
easily

which is not the case.  This is Emacs-30.0.50.
I am sorry I cannot give more details.

How to proceed? Of course, I agree with your version of the patch
although I had started to remove some of the superfluous scaffolding
to know where point and mark are.

Regards -- Gerard

Re: [PATCH] org-babel-demarcate-block: split using element API

[Ihor Radchenko]

gerard.vermeu...@posteo.net writes:

>>> Still failing on my side (when running tests non-interactively from
>>> command line). To fix the problem, please use the approach from
>>> `test-org-list/indent-item':
>>> 
>>> (transient-mark-mode 1)
>>> (push-mark (point) t t)
>>> 
>>> Instead of (set-mark-command nil)
>> 
>> Gerard, may I know if you had a chance to look into my comments?
>
> I think that I have addressed this particular comment.

Not really.
In any case, see the attached updated patch with my suggestion
incorporated.

> However, I am confused by your comments concerning this example
> #+begin_src emacs-lisp
>'(1 2 3)
>'(1 2 3)
> #+end_src
> since it breaks my patch as well as main in the sense that such examples
> can generate three blocks with invalid code.
> I think there is in general no way to protect a user against bad 
> selections
> for splitting by demarcation.

Fair.

> Secondly, I see (saw) sometimes Org emitting warnings with backtraces
> starting from my patch.  I think the warning may be due either to a 
> mistake
> on my side or a bug in Org, but I am not sure.

May you please provide more details?

>From dc71a916c829e979c0728f95bfefe54b1cfa3887 Mon Sep 17 00:00:00 2001
Message-ID: 
From: Gerard Vermeulen 
Date: Thu, 11 Jan 2024 20:20:01 +0100
Subject: [PATCH] org-babel-demarcate-block: split using element API

* lisp/ob-babel.el (org-babel-demarcate-block): Modify a copy
of (org-element-at-point) to replace the old source block with 2 or 3
new modified copies by means of `org-element-interpret-data'.  The 1st
source block contains the text from the body of the old block before
point or region, the 2nd block contains the body text after point or
body text within region, and in case of region, the 3rd block contains
the text after region.  The caption and the name are deleted from the
1 or 2 blocks below the upper source block.  Indent all blocks
immediately after insertion (pitfall, see link).  Use :post-blank to
control white lines between inserted blocks.  Leave point at the last
inserted block.  Take care to preserve (current-column) of text
after point (and mark) in the 2nd (and 3rd) block.  Trying to split
when point or region is not within the body of the old source block
raises an user-error.
* lisp/ob-babel (org-get-src-block-info): add the "within blank lines
after a source block" condition to the doc-string to match it with the
doc-string of and a comment in `org-babel-demarcate-block'.
* testing/lisp/test-ob.el (test-ob/demarcate-block-split-duplication)
(test-ob/demarcate-block-split-prefix-point)
(test-ob/demarcate-block-split-prefix-region)
(test-ob/demarcate-block-split-user-errors)
(test-ob/demarcate-block-wrap-point)
(test-ob/demarcate-block-wrap-region): New tests to check test cases
that broke earlier versions of this patch.

Link: https://list.orgmode.org/87ply6nyue.fsf@localhost/
---
 lisp/ob-core.el |  94 +++-
 testing/lisp/test-ob.el | 241 
 2 files changed, 307 insertions(+), 28 deletions(-)

diff --git a/lisp/ob-core.el b/lisp/ob-core.el
index bfeac257b..e3110a3f1 100644
--- a/lisp/ob-core.el
+++ b/lisp/ob-core.el
@@ -73,10 +73,12 @@ (declare-function org-element-contents-end "org-element" (node))
 (declare-function org-element-parent "org-element-ast" (node))
 (declare-function org-element-type "org-element-ast" (node  anonymous))
 (declare-function org-element-type-p "org-element-ast" (node  types))
+(declare-function org-element-interpret-data "org-element" (data))
 (declare-function org-entry-get "org" (pom property  inherit literal-nil))
 (declare-function org-escape-code-in-region "org-src" (beg end))
 (declare-function org-forward-heading-same-level "org" (arg  invisible-ok))
 (declare-function org-in-commented-heading-p "org" ( no-inheritance))
+(declare-function org-indent-block "org" ())
 (declare-function org-indent-line "org" ())
 (declare-function org-list-get-list-end "org-list" (item struct prevs))
 (declare-function org-list-prevs-alist "org-list" (struct))
@@ -700,8 +702,9 @@ (defun org-babel-get-src-block-info ( no-eval datum)
 argument DATUM is provided, extract information from that parsed
 object instead.
 
-Return nil if point is not on a source block.  Otherwise, return
-a list with the following pattern:
+Return nil if point is not on a source block (blank lines after a
+source block are considered a part of that source block).
+Otherwise, return a list with the following pattern:
 
   (language body arguments switches name start coderef)"
   (let* ((datum (or datum (org-element-context)))
@@ -2075,7 +2078,7 @@ (defun org-babel-mark-block ()
   (goto-char (match-begin

Re: [PATCH] org-babel-demarcate-block: split using element API

[gerard . vermeulen]

On 19.02.2024 10:46, Ihor Radchenko wrote:

Ihor Radchenko  writes:


gerard.vermeu...@posteo.net writes:

I think that I have improved my region marking code by using 
""

in the temp-text as a start. Then, I only have to find where to set
mark,
and eventually exchange point and mark.


Still failing on my side (when running tests non-interactively from
command line). To fix the problem, please use the approach from
`test-org-list/indent-item':

(transient-mark-mode 1)
(push-mark (point) t t)

Instead of (set-mark-command nil)


Gerard, may I know if you had a chance to look into my comments?


I think that I have addressed this particular comment.

However, I am confused by your comments concerning this example
#+begin_src emacs-lisp
  '(1 2 3)
  '(1 2 3)
#+end_src
since it breaks my patch as well as main in the sense that such examples
can generate three blocks with invalid code.
I think there is in general no way to protect a user against bad 
selections

for splitting by demarcation.

Secondly, I see (saw) sometimes Org emitting warnings with backtraces
starting from my patch.  I think the warning may be due either to a 
mistake

on my side or a bug in Org, but I am not sure.

Regards -- Gerard

Re: [PATCH] org-babel-demarcate-block: split using element API

[Ihor Radchenko]

Ihor Radchenko  writes:

> gerard.vermeu...@posteo.net writes:
>
>>> I think that I have improved my region marking code by using ""
>>> in the temp-text as a start. Then, I only have to find where to set 
>>> mark,
>>> and eventually exchange point and mark.
>
> Still failing on my side (when running tests non-interactively from
> command line). To fix the problem, please use the approach from
> `test-org-list/indent-item':
>
>   (transient-mark-mode 1)
>   (push-mark (point) t t)
>
> Instead of (set-mark-command nil)

Gerard, may I know if you had a chance to look into my comments?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: [PATCH] org-babel-demarcate-block: split using element API

[Ihor Radchenko]

gerard.vermeu...@posteo.net writes:

>>> 1 unexpected results:
>>>FAILED  test-ob/demarcate-block  ((should (string= region-text
>>> (org-trim (nth 1 info :form (string= "mark this line as region"
>>> "") :value nil :explanation (arrays-of-different-length 24 0 "mark
>>> this line as region" "" first-mismatch-at 0))
>> 
>> This is a tough lesson: the tests pass always on my system.
>> 
>> I think the failure you see is related to a problem marking a region
>> in my test code (wish: support in `org-test-with-temp-text' for
>> "" besides "", but maybe that depends on ERT), else
>> the problem would have shown up while testing the patch interactively.
>> 
>> I think that I have improved my region marking code by using ""
>> in the temp-text as a start. Then, I only have to find where to set 
>> mark,
>> and eventually exchange point and mark.

Still failing on my side (when running tests non-interactively from
command line). To fix the problem, please use the approach from
`test-org-list/indent-item':

(transient-mark-mode 1)
(push-mark (point) t t)

Instead of (set-mark-command nil)


> +  ;; Map positions to columns for white-space padding.
> +  (setq pads (mapcar (lambda (p) (save-excursion
> +   (goto-char p)
> +   (current-column)))
> + pads))

This will break when the region does not start near the beginning of
line or does not end there:

#+begin_src emacs-lisp
  '(1 2 3)
  '(1 2 3)
#+end_src

Also, the indentation of source code inside src block should not be used
to indent the whole block. This is because it may be additionally
indented according to `org-edit-src-content-indentation'. If you want to
preserve the original indentation, just use
`org-current-text-indentation' at the beginning of the src block.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: [PATCH] org-babel-demarcate-block: split using element API

[gerard . vermeulen]

On 14.01.2024 20:18, gerard.vermeu...@posteo.net wrote:

On 14.01.2024 13:16, Ihor Radchenko wrote:

gerard.vermeu...@posteo.net writes:

[...]

I tried make test and the tests are still failing with this new patch:

1 unexpected results:
   FAILED  test-ob/demarcate-block  ((should (string= region-text
(org-trim (nth 1 info :form (string= "mark this line as region"
"") :value nil :explanation (arrays-of-different-length 24 0 "mark
this line as region" "" first-mismatch-at 0))


This is a tough lesson: the tests pass always on my system.

I think the failure you see is related to a problem marking a region
in my test code (wish: support in `org-test-with-temp-text' for
"" besides "", but maybe that depends on ERT), else
the problem would have shown up while testing the patch interactively.

I think that I have improved my region marking code by using ""
in the temp-text as a start. Then, I only have to find where to set 
mark,

and eventually exchange point and mark.

The test code now checks (mark) in the 3 places where a region is 
marked.
This looks superfluous if the code is really robust, but at least it 
checks

whether the region marking is (or was) the problem.



To converge faster, I have split the test into 6 tests:

test-ob/demarcate-block-split-duplication
test-ob/demarcate-block-split-prefix-point
test-ob/demarcate-block-split-prefix-region
test-ob/demarcate-block-split-user-errors
test-ob/demarcate-block-wrap-point
test-ob/demarcate-block-wrap-region

The test failure on your system was due to the sub-test that is now
test-ob/demarcate-block-wrap-region

Regards -- Gerard

0001-org-babel-demarcate-block-split-using-element-API.patch
Description: Binary data

Re: [PATCH] org-babel-demarcate-block: split using element API

[gerard . vermeulen]

On 14.01.2024 13:16, Ihor Radchenko wrote:

gerard.vermeu...@posteo.net writes:

[...]

I tried make test and the tests are still failing with this new patch:

1 unexpected results:
   FAILED  test-ob/demarcate-block  ((should (string= region-text
(org-trim (nth 1 info :form (string= "mark this line as region"
"") :value nil :explanation (arrays-of-different-length 24 0 "mark
this line as region" "" first-mismatch-at 0))


This is a tough lesson: the tests pass always on my system.

I think the failure you see is related to a problem marking a region
in my test code (wish: support in `org-test-with-temp-text' for
"" besides "", but maybe that depends on ERT), else
the problem would have shown up while testing the patch interactively.

I think that I have improved my region marking code by using ""
in the temp-text as a start. Then, I only have to find where to set 
mark,

and eventually exchange point and mark.

The test code now checks (mark) in the 3 places where a region is 
marked.
This looks superfluous if the code is really robust, but at least it 
checks

whether the region marking is (or was) the problem.

New patch attached.

Some of the scaffolding (should ...) forms could be removed if
the 5 sub-test in test-ob/demarcate-block would be 5 separated
ERT tests.  I prefer to continue like this and do eventual refactoring
later.

Regards -- Gerard

0001-org-babel-demarcate-block-split-using-element-API.patch
Description: Binary data

Re: [PATCH] org-babel-demarcate-block: split using element API

[Ihor Radchenko]

gerard.vermeu...@posteo.net writes:

> New patch attached.

I tried make test and the tests are still failing with this new patch:

1 unexpected results:
   FAILED  test-ob/demarcate-block  ((should (string= region-text (org-trim 
(nth 1 info :form (string= "mark this line as region" "") :value nil 
:explanation (arrays-of-different-length 24 0 "mark this line as region" "" 
first-mismatch-at 0))


-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: [PATCH] org-babel-demarcate-block: split using element API

[gerard . vermeulen]

On 13.01.2024 21:16, gerard.vermeu...@posteo.net wrote:

On 13.01.2024 16:17, Ihor Radchenko wrote:

gerard.vermeu...@posteo.net writes:


Attached you'll find a new patch addressing all you issues.


Thanks!
I tried to run make test, and I am getting
   FAILED  test-ob/demarcate-block  ((should (string= region-text
(org-trim (nth 1 info :form (string= "mark this line as region"
"") :value nil :explanation (arrays-of-different-length 24 0 "mark
this line as region" "" first-mismatch-at 0))


I have improved a regexp used to mark a region in this sub-test
improving the robustness of the code.
Furthermore, I have replaced all occurrences of (set-mark (point))
with (set-mark-command nil), but I doubt that this is the reason.

Nevertheless, I feel I need to point out the limitation of this 
particular

test case.


[...]

The text after (mark) and (point) is misaligned.
I tend to mark regions in a way that is compatible with the patch,
but some users won't (maybe they are willing to adapt).

Patch attached.


I found a way to preserve the (current-column) of text after point and
mark in the element API code so that point or region splitting behaves
like main where the (current-column)'s remain preserved naturally.

I think this is preferable with respect to the previous patch (at least
it does not break the expectations of current users).

It allowed a minor simplification of the sub-test that failed in your 
case.


New patch attached.

Regards -- Gerard

0001-org-babel-demarcate-block-split-using-element-API.patch
Description: Binary data

Re: [PATCH] org-babel-demarcate-block: split using element API

[gerard . vermeulen]

On 13.01.2024 16:17, Ihor Radchenko wrote:

gerard.vermeu...@posteo.net writes:


Attached you'll find a new patch addressing all you issues.


Thanks!
I tried to run make test, and I am getting
   FAILED  test-ob/demarcate-block  ((should (string= region-text
(org-trim (nth 1 info :form (string= "mark this line as region"
"") :value nil :explanation (arrays-of-different-length 24 0 "mark
this line as region" "" first-mismatch-at 0))


I have improved a regexp used to mark a region in this sub-test
improving the robustness of the code.
Furthermore, I have replaced all occurrences of (set-mark (point))
with (set-mark-command nil), but I doubt that this is the reason.

Nevertheless, I feel I need to point out the limitation of this 
particular

test case.

Prerequisites:
#+begin_src emacs-lisp :results silent
(setopt org-adapt-indentation t
org-edit-src-content-indentation 2
org-src-preserve-indentation nil)
#+end_src

When I mark really the line containing "mark this line as region"
C-u C-C C-v C-d works nicely (done in the sub-test).
** 10 stars with region between two lines
   #+header: :var b="also seen"
   #+begin_src any-language -i -n :var a="seen"
 to upper block
 mark this line as region
 to lower block
   #+end_src

but C-u C-c C-v C-d after marking ' this line as ' produces this:
** 10 stars with region between two lines
   #+header: :var b="also seen"
   #+begin_src any-language -i -n :var a="seen"
 to upper block
 mark
   #+end_src
**
   #+header: :var b="also seen"
   #+begin_src any-language -i -n :var a="seen"
 this line as
   #+end_src
**
   #+header: :var b="also seen"
   #+begin_src any-language -i -n :var a="seen"
region
 to lower block
   #+end_src

The text after (mark) and (point) is misaligned.
I tend to mark regions in a way that is compatible with the patch,
but some users won't (maybe they are willing to adapt).

Patch attached.

Regards -- Gerard


0001-org-babel-demarcate-block-split-using-element-API.patch
Description: Binary data

Re: [PATCH] org-babel-demarcate-block: split using element API

[Ihor Radchenko]

gerard.vermeu...@posteo.net writes:

> Attached you'll find a new patch addressing all you issues.

Thanks!
I tried to run make test, and I am getting
   FAILED  test-ob/demarcate-block  ((should (string= region-text (org-trim 
(nth 1 info :form (string= "mark this line as region" "") :value nil 
:explanation (arrays-of-different-length 24 0 "mark this line as region" "" 
first-mismatch-at 0))

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: [PATCH] org-babel-demarcate-block: split using element API

[gerard . vermeulen]

Attached you'll find a new patch addressing all you issues.

I have integrated our discussion leading to
https://list.orgmode.org/87ply6nyue.fsf@localhost/
Please feel free to add the line

Co-authored-by: Ihor Radchenko 

to the commit message.  I think you should.

On 09.01.2024 15:49, Ihor Radchenko wrote:

gerard.vermeu...@posteo.net writes:


Attached you'll find a new patch fixing the three wrong lines in the
previous
and now the ERT test checks also for `user-error's.


Thanks!

2. Your patch does not create space between the src blocks, unlike 
what

   we have on main.
   I think that you need to (1) create a single blank lines between
   blocks (set :post-blank property to 1); (2) keep the number blank
   lines after the last block the same as in the initial block (copy
the
   :post-blank property and assign it to the last inserted src 
block).


   For C-u argument, do not do anything special - just keep the
original
   :post-blank as is. It is the closest to what we have on main.



The previous version of the patch had
+(insert (if arg (concat stars "\n") ""))
and now it has
+(insert (if arg (concat stars "\n") "\n"))
I prefer this over setting the :post-blank property because it is
simpler.


Yet, it will lead to large spacing between src blocks in the following
scenario:


#+begin_src emacs-lisp
  "This is test"
  "This is test2"
  "This is test3"
#+end_src






Paragraph.




I have used the :post-blank property to fix it.  The result is now:

#+begin_src emacs-lisp
  "This is test"
#+end_src

#+begin_src emacs-lisp
  "This is test2"
  "This is test3"
#+end_src






Paragraph.


Here is also a redone comparison between main and patch:

--- begin comparison main and patch
#+begin_src emacs-lisp :results silent
(setopt org-adapt-indentation t
org-edit-src-content-indentation 2
org-src-preserve-indentation nil))
#+end_src

* main
 C-u C-x C-v C-d
 #+CAPTION: caption.
 #+NAME: name.
 #+BEGIN_SRC emacs-lisp
   above
   ;; region
   below
 #+END_SRC
becomes
 C-u C-x C-v C-d
 #+CAPTION: caption.
 #+NAME: name.
 #+BEGIN_SRC emacs-lisp
   above

   #+END_SRC

   #+BEGIN_SRC emacs-lisp
   ;; region
   #+END_SRC

   #+BEGIN_SRC emacs-lisp
   below
 #+END_SRC
pitfall

* patch
 C-u C-x C-v C-d
 #+CAPTION: caption.
 #+NAME: name.
 #+BEGIN_SRC emacs-lisp
   above
   ;; region
   below
 #+END_SRC
becomes
 C-u C-x C-v C-d
 #+caption: caption.
 #+name: name.
 #+begin_src emacs-lisp
   above
 #+end_src

 #+begin_src emacs-lisp
   ;; region
 #+end_src

 #+begin_src emacs-lisp

   below
 #+end_src
pitfall
--- end comparison main and patch



Also, your new version of the patch will completely misbehave because 
of

setting mark. Please, use `region-beginning' and `region-end' instead.
Setting and checking mark is not to be done in Elisp - it only make
sense when transient-mark-mode is enabled.


Done. See below.


* Adding a user option for properties to set to nil in 
org-element-copy.


This may be overkill, but something like:

#+begin_src emacs-lisp :results nil :tangle no
(defcustom org-babel-demarcate-block-delete '(:caption :name)
   "List of things to delete from blocks below the upper block when
splitting blocks by demarcation.  Possible things are `:caption' to
delete \"#+CAPTION:\" keywords, `:header' to delete \"#+HEADER:\"
keywords, `:name' to delete \"#+NAME:\" keywords, and `switches'
to delete e.g. \"-i +n 10\" from the \"#+BEGIN_SRC\" line."
   :group 'org-babel
   :package-version '(Org . "9.7")
   :type '(set :tag "Things to delete when splitting blocks by
demarcation"
   (const :caption)
   (const :header)
   (const :name)
   (const :switches))
   :initialize #'custom-initialize-default
   :set (lambda (sym val)
  (set-default sym val)))
#+end_src


That would make sense. Although, we do not have to limit the possible
options to just what you listed. Arbitrary affiliated keywords might
also be excluded. For example, #+ATTR_HTML keyword is stored in src
block object as :attr_html.


I prefer to postpone work on this idea.



+  ;; To simplify the (unless ... (user-error ...)).
+  (unless (org-region-active-p) (set-mark (point)))


Setting mark causes issue in my above example.


+  ;; Test mark to be more specific than "Not at a block".
+  (unless (and (>= (point) body-beg) (<= (mark) body-end))
+(user-error "Select within the source block body to split 
it"))


Here, it is better to use `region-beginning', `point', and 
`region-end'.

`region-beginning', unlike mark and point, is guaranteed to be _before_
`region-end'. Mark may be before point, in contrast.

You can write something like

Re: [PATCH] org-babel-demarcate-block: split using element API

[Ihor Radchenko]

gerard.vermeu...@posteo.net writes:

> Attached you'll find a new patch fixing the three wrong lines in the 
> previous
> and now the ERT test checks also for `user-error's.

Thanks!

>> 2. Your patch does not create space between the src blocks, unlike what
>>we have on main.
>>I think that you need to (1) create a single blank lines between
>>blocks (set :post-blank property to 1); (2) keep the number blank
>>lines after the last block the same as in the initial block (copy 
>> the
>>:post-blank property and assign it to the last inserted src block).
>> 
>>For C-u argument, do not do anything special - just keep the 
>> original
>>:post-blank as is. It is the closest to what we have on main.
>> 
>
> The previous version of the patch had
> +(insert (if arg (concat stars "\n") ""))
> and now it has
> +(insert (if arg (concat stars "\n") "\n"))
> I prefer this over setting the :post-blank property because it is 
> simpler.

Yet, it will lead to large spacing between src blocks in the following
scenario:


#+begin_src emacs-lisp
  "This is test"
  "This is test2"
  "This is test3"
#+end_src






Paragraph.


Also, your new version of the patch will completely misbehave because of
setting mark. Please, use `region-beginning' and `region-end' instead.
Setting and checking mark is not to be done in Elisp - it only make
sense when transient-mark-mode is enabled.

> * Adding a user option for properties to set to nil in org-element-copy.
>
> This may be overkill, but something like:
>
> #+begin_src emacs-lisp :results nil :tangle no
> (defcustom org-babel-demarcate-block-delete '(:caption :name)
>"List of things to delete from blocks below the upper block when
> splitting blocks by demarcation.  Possible things are `:caption' to
> delete \"#+CAPTION:\" keywords, `:header' to delete \"#+HEADER:\"
> keywords, `:name' to delete \"#+NAME:\" keywords, and `switches'
> to delete e.g. \"-i +n 10\" from the \"#+BEGIN_SRC\" line."
>:group 'org-babel
>:package-version '(Org . "9.7")
>:type '(set :tag "Things to delete when splitting blocks by 
> demarcation"
>(const :caption)
>(const :header)
>(const :name)
>(const :switches))
>:initialize #'custom-initialize-default
>:set (lambda (sym val)
>   (set-default sym val)))
> #+end_src

That would make sense. Although, we do not have to limit the possible
options to just what you listed. Arbitrary affiliated keywords might
also be excluded. For example, #+ATTR_HTML keyword is stored in src
block object as :attr_html.

> +  ;; To simplify the (unless ... (user-error ...)).
> +  (unless (org-region-active-p) (set-mark (point)))

Setting mark causes issue in my above example.

> +  ;; Test mark to be more specific than "Not at a block".
> +  (unless (and (>= (point) body-beg) (<= (mark) body-end))
> +(user-error "Select within the source block body to split it"))

Here, it is better to use `region-beginning', `point', and `region-end'.
`region-beginning', unlike mark and point, is guaranteed to be _before_
`region-end'. Mark may be before point, in contrast.

You can write something like

(unless
 (if (org-region-active-p)
   (<= body-beg (region-beginning) (region-end) body-end)
  (>= body-beg (point)))
 (user-error ...))

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: [PATCH] org-babel-demarcate-block: split using element API

[gerard . vermeulen]

On 09.01.2024 08:49, gerard.vermeu...@posteo.net wrote:
[...]
Anyhow, I have removed the comment and I have replaced check below it 
with

+  (set-mark (point)) ;; To simplify the next (unless ...):
+  (unless (and (>= (point) body-beg) (<= (mark) body-end))
+(user-error "Select within the source block body to split 
it"))
which also protects against having point in body and mark on or below 
#+end_src


It occurred to me to that I only should set mark to point when the 
region is

not active.  I will add checking for `user-error's to the ERT test.

Attached you'll find a new patch fixing the three wrong lines in the 
previous

and now the ERT test checks also for `user-error's.

Regards -- Gerard

0001-org-babel-demarcate-block-split-using-element-API.patch
Description: Binary data

Re: [PATCH] org-babel-demarcate-block: split using element API

[gerard . vermeulen]

On 08.01.2024 21:25, gerard.vermeu...@posteo.net wrote:

On 08.01.2024 13:08, Ihor Radchenko wrote:

gerard.vermeu...@posteo.net writes:

[...]
Anyhow, I have removed the comment and I have replaced check below it 
with

+  (set-mark (point)) ;; To simplify the next (unless ...):
+  (unless (and (>= (point) body-beg) (<= (mark) body-end))
+(user-error "Select within the source block body to split 
it"))
which also protects against having point in body and mark on or below 
#+end_src


It occurred to me to that I only should set mark to point when the 
region is

not active.  I will add checking for `user-error's to the ERT test.

Regards -- Gerard

Re: [PATCH] org-babel-demarcate-block: split using element API

[gerard . vermeulen]

On 08.01.2024 13:08, Ihor Radchenko wrote:

gerard.vermeu...@posteo.net writes:

Attached you'll find a new version of my patch addressing all your 
issues.

This mail ends with two other ideas in the context of this patch.
[...]

I've tested your patch and found two problems:

1. #+name: lines are duplicated, while they should not


Of course. Sometimes I delete lines by a slip of the fingers. Thanks.


2. Your patch does not create space between the src blocks, unlike what
   we have on main.
   I think that you need to (1) create a single blank lines between
   blocks (set :post-blank property to 1); (2) keep the number blank
   lines after the last block the same as in the initial block (copy 
the

   :post-blank property and assign it to the last inserted src block).

   For C-u argument, do not do anything special - just keep the 
original

   :post-blank as is. It is the closest to what we have on main.



The previous version of the patch had
+(insert (if arg (concat stars "\n") ""))
and now it has
+(insert (if arg (concat stars "\n") "\n"))
I prefer this over setting the :post-blank property because it is 
simpler.

(main concats something like  (if (arg stars "") "\n" ...).

[...]



Agreed, this is wrong. A partial explanation is that I attached too
much value to the doc-string of `org-babel-get-src-block-info'
telling "Return nil if point is not on a source block.  Otherwise,"
which
is for me in contradiction with documentation (string and start
comment) in `org-babel-demarcate-block'.


`org-babel-get-src-block-info' docstring were not wrong. You just 
missed

the Org mode's convention that blank lines after src blocks or other
syntax elements belong to these elements.

That said, we may clarify the `org-babel-get-src-block-info' docstring
to highlight this fact and avoid future confusion.


I changed the docstring as you suggested below.



Now demarcating with point below a source block works again and
checking this is part of the ERT test.


The ERT test does not check removing #+caption from the original block.
Also, as I said above, we should remove #+name.


The ERT test now checks that #+caption and #+name are removed from
the original code.



[...]


Note that indentation of src blocks has been refactored recently on
main. It should be more reliable now. If not, please report any issues.


I will pay attention.



-;; Copyright (C) 2009-2024 Free Software Foundation, Inc.
+;; Copyright (C) 2009-2023 Free Software Foundation, Inc.


This is a spurious change :)

Reverted: it shows that I started editing in 2023 and that I am no good 
at git :)



-Return nil if point is not on a source block.  Otherwise, return
-a list with the following pattern:
+Return nil if point is not on a source block or not within blank
+lines after a source block.  Otherwise, return a list with the
+following pattern:


I'd rather say: Return nil if point is not on a source block (blank
lines after a source block are considered a part of that source block).

It would be more accurate.


Done.



+(let* ((copy (org-element-copy (org-element-at-point)))
+   (before (org-element-begin copy))
+   (beyond (org-element-end copy))
+   (parts (sort (if (org-region-active-p)
+(list body-beg (mark) (point) 
body-end)

+  (list body-beg (point) body-end))
+#'<)))
+  ;; Prevent #+caption:, #+header:, and #+begin_src lines in 
block.


This comment is out of place. Also, we do preserve #+header and
#+begin_src lines, don't we?


Maybe I should have written:
+  ;; Prevent #+caption:, #+header:, and #+begin_src lines in 
*body*.


It prevents that when splitting with point at the # of #+caption a block 
like


#+caption: caption
#+name: name
#+begin_src emacs-lisp
;; elisp code
...
#+end_src

the first block ends up with

#+caption: caption
#+name: name
#+begin_src emacs-lisp
,#+caption: caption
,#+name: name
,#+begin_src emacs-lisp
;; elisp code
...
#+end_src

This is not easy to capture in a 1-2 line comment.

Anyhow, I have removed the comment and I have replaced check below it 
with

+  (set-mark (point)) ;; To simplify the next (unless ...):
+  (unless (and (>= (point) body-beg) (<= (mark) body-end))
+(user-error "Select within the source block body to split 
it"))
which also protects against having point in body and mark on or below 
#+end_src


I think it covers everything that can be checked in the "splitting" 
branch.


I think also that the "wrapping" branch can be better protected against 
similar
region selection "user errors".  I will come back on improving the 
"wrapping"

branch shortly.


And we need to remove #+name lines.


Done.



+  (unless (and (>= (point) body-beg))
+(user-error "move point within the source block body to 
split it"))


Please start error message from capital 

Re: [PATCH] org-babel-demarcate-block: split using element API

[Ihor Radchenko]

gerard.vermeu...@posteo.net writes:

> On 04.01.2024 15:43, Ihor Radchenko wrote:
>> gerard.vermeu...@posteo.net writes:
>> 
> Attached you'll find a new version of the patch that addresses your
> comments.  I have modified the ERT test so that it checks most of
> your examples showing where the older versions of the patch failed.
> The test is now called `test-ob/demarcate-block'

Thanks!
I've tested your patch and found two problems:

1. #+name: lines are duplicated, while they should not
2. Your patch does not create space between the src blocks, unlike what
   we have on main.
   I think that you need to (1) create a single blank lines between
   blocks (set :post-blank property to 1); (2) keep the number blank
   lines after the last block the same as in the initial block (copy the
   :post-blank property and assign it to the last inserted src block).

   For C-u argument, do not do anything special - just keep the original
   :post-blank as is. It is the closest to what we have on main.

> Below, I compare region splitting using main or my patch.  White-space
> differs between main and the patch and one might argue that the result
> produced by the patch is more consistent.

Agree.

> Agreed, this is wrong. A partial explanation is that I attached too
> much value to the doc-string of `org-babel-get-src-block-info'
> telling "Return nil if point is not on a source block.  Otherwise," 
> which
> is for me in contradiction with documentation (string and start
> comment) in `org-babel-demarcate-block'.

`org-babel-get-src-block-info' docstring were not wrong. You just missed
the Org mode's convention that blank lines after src blocks or other
syntax elements belong to these elements.

That said, we may clarify the `org-babel-get-src-block-info' docstring
to highlight this fact and avoid future confusion.

> Now demarcating with point below a source block works again and
> checking this is part of the ERT test.

The ERT test does not check removing #+caption from the original block.
Also, as I said above, we should remove #+name.

>> `org-indent-block' should honor `org-adapt-indentation'. You do not 
>> need
>> to call it conditionally. Re-indenting unconditionally should be better
>> here.
> OK. I have always used `org-adapt-indentation' set to nil and I do not 
> like
> the result of `org-indent-block' when it is non-nil (#+begin_src and 
> #+end_src
> indented and the code pushed to the left), but I will have to get used 
> to it.

Note that indentation of src blocks has been refactored recently on
main. It should be more reliable now. If not, please report any issues.

> -;; Copyright (C) 2009-2024 Free Software Foundation, Inc.
> +;; Copyright (C) 2009-2023 Free Software Foundation, Inc.

This is a spurious change :)
  
> -Return nil if point is not on a source block.  Otherwise, return
> -a list with the following pattern:
> +Return nil if point is not on a source block or not within blank
> +lines after a source block.  Otherwise, return a list with the
> +following pattern:

I'd rather say: Return nil if point is not on a source block (blank
lines after a source block are considered a part of that source block).

It would be more accurate.
  
> +(let* ((copy (org-element-copy (org-element-at-point)))
> +   (before (org-element-begin copy))
> +   (beyond (org-element-end copy))
> +   (parts (sort (if (org-region-active-p)
> +(list body-beg (mark) (point) body-end)
> +  (list body-beg (point) body-end))
> +#'<)))
> +  ;; Prevent #+caption:, #+header:, and #+begin_src lines in block.

This comment is out of place. Also, we do preserve #+header and
#+begin_src lines, don't we?

And we need to remove #+name lines.

> +  (unless (and (>= (point) body-beg))
> +(user-error "move point within the source block body to split 
> it"))

Please start error message from capital letter. It is Elisp convention
(see `user-error' docstring).

> +  (setq parts (mapcar (lambda (p) (buffer-substring (car p) (cdr p)))
> +  (seq-mapn #'cons parts (cdr parts
> +  (delete-region before beyond)
> +  (deactivate-mark)

AFAIK, `deactivate-mark' should be unnecessary here. To indicate that
region should be deactivated after finishing a command, we simply set
`deactivate-mark' _variable_ to t. See the docstring.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: [PATCH] org-babel-demarcate-block: split using element API

[gerard . vermeulen]

On 04.01.2024 15:43, Ihor Radchenko wrote:

gerard.vermeu...@posteo.net writes:


Attached you'll find a new version of the patch that addresses your
comments.  I have modified the ERT test so that it checks most of
your examples showing where the older versions of the patch failed.
The test is now called `test-ob/demarcate-block'

It also allows to split in three blocks when a region is selected (main
does this contrary to my older patches).

Below, I compare region splitting using main or my patch.  White-space
differs between main and the patch and one might argue that the result
produced by the patch is more consistent.  Maybe, the indenting of the
input code block is somewhat contrived, because all code is moved
completely to the left after calling `org-indent-block'.

* main does this
#+begin_src emacs-lisp :results silent
  (setopt org-adapt-indentation t
  org-src-preserve-indentation t
  org-edit-src-content-indentation 2)
#+end_src
 before C-u org-babel-demarcate-block region splitting
 #+begin_src emacs-lisp
   (defun above ()
 (message "above"))

   (defun region ()
 (message "mark region with leading and trailing blank 
lines"))


   (defun below ()
 (message "below"))
 #+end_src
 after C-u org-babel-demarcate-block region splitting
 #+begin_src emacs-lisp
   (defun above ()
 (message "above"))
#+end_src

#+begin_src emacs-lisp
   (defun region ()
 (message "mark region with leading and trailing blank 
lines"))


   #+end_src

   #+begin_src emacs-lisp
   (defun below ()
 (message "below"))
 #+end_src
* end main does this

* patch does this
#+begin_src emacs-lisp :results silent
  (setopt org-adapt-indentation t
  org-src-preserve-indentation t
  org-edit-src-content-indentation 2)
#+end_src
 before C-u org-babel-demarcate-block region splitting
 #+begin_src emacs-lisp
   (defun above ()
 (message "above"))

   (defun region ()
 (message "mark region with leading and trailing blank 
lines"))


   (defun below ()
 (message "below"))
 #+end_src
 after C-u org-babel-demarcate-block region splitting
 #+begin_src emacs-lisp
(defun above ()
  (message "above"))
 #+end_src

 #+begin_src emacs-lisp

(defun region ()
  (message "mark region with leading and trailing blank lines"))
 #+end_src

 #+begin_src emacs-lisp
(defun below ()
  (message "below"))
 #+end_src
* end patch does this



I have tried to clean up the code.  I have also tried to get 
`body-beg'

and
`body-end' marking the text between the #+begin_src and #+end_src 
lines

from the element API, but I failed and had to fall back to
`org-babel-where-is-src-block-head'.  But only for that.


org-element API does not provide this information for now. Maybe it is 
a

good opportunity to alter the parser, so that code boundaries are
provided...


 (defun org-babel-demarcate-block ( arg)
...
-When called within blank lines after a code block, create a new code
-block of the same language with the previous."


Is there any reason why you dropped this feature?

When I try

#+begin_src emacs-lisp
(+ 1 2)
#+end_src


M-x org-babel-demarcate-block throws an error with your patch.
It creates a new block with the same language before your patch.


Agreed, this is wrong. A partial explanation is that I attached too
much value to the doc-string of `org-babel-get-src-block-info'
telling "Return nil if point is not on a source block.  Otherwise," 
which

is for me in contradiction with documentation (string and start
comment) in `org-babel-demarcate-block'.

I have patched the doc-string of `org-babel-get-src-block-info' to
add the "blank lines below condition".

This patch reverts all changes that are due to my misunderstanding
of what `org-babel-get-src-block-info' does.

Now demarcating with point below a source block works again and
checking this is part of the ERT test.



+  (let ((copy (org-element-copy (org-element-at-point)))
+(stars (concat (make-string (or (org-current-level) 1) ?*) " 
")))

+(if (eq 'src-block (car copy))


You can instead use `org-element-type-p'
This is now back to the original (if (and info start) ;; At src block, 
but ...


+;; Keep this branch in sync with 
test-ob/demarcate-block-split.
+;; _start is never nil, since there is a source block element 
at point.


May you elaborate what you mean by "keep in sync"?

"keep in sync" is a kind of reminder to myself, because I think that
test-ob/demarcate-block-split was fragile wrt where point is after
demarcation.

The test is now called test-ob/demarcate-block and I tried to make
it more robust.



+(let* ((_start (org-babel-where-is-src-block-head))


Are