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 <yantar92@posteo.net>

to the commit message.  I think you should.

On 09.01.2024 15:49, Ihor Radchenko wrote:
> 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.
> --------------------

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

Done, using a better initialization of parts, the test simplifies to:

           ;; Point or region are within body when parts is in 
increasing order.
           (unless (apply #'<= parts)
             (user-error "Select within the source block body to split 
it"))

Regards -- Gerard

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