emacs-orgmode
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: gerard . vermeulen
Subject: Re: [PATCH] org-babel-demarcate-block: split using element API
Date: Sat, 13 Jan 2024 14:04:31 +0000

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

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


reply via email to

[Prev in Thread] Current Thread [Next in Thread]