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

[Ihor Radchenko <yantar92@posteo.net>]

gerard.vermeulen@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"
<point>  "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 <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>