From: Ihor Radchenko <yantar92@posteo.net>
To: gerard.vermeulen@posteo.net
Cc: Emacs orgmode <emacs-orgmode@gnu.org>
Subject: Re: [PATCH] org-babel-demarcate-block: split using element API
Date: Mon, 08 Jan 2024 12:08:15 +0000 [thread overview]
Message-ID: <878r50yqmo.fsf@localhost> (raw)
In-Reply-To: <37fdcc4bfcf734c2e5ec439d40b4f7d8@posteo.net>
gerard.vermeulen@posteo.net writes:
> On 04.01.2024 15:43, Ihor Radchenko wrote:
>> gerard.vermeulen@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 <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>
next prev parent reply other threads:[~2024-01-08 12:06 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-30 19:13 [PATCH] org-babel-demarcate-block: duplicate switches too gerard.vermeulen
2023-12-31 14:28 ` Ihor Radchenko
2024-01-01 12:52 ` gerard.vermeulen
2024-01-02 10:48 ` Ihor Radchenko
2024-01-02 20:20 ` [PATCH] org-babel-demarcate-block: split using org-element instead of regexp gerard.vermeulen
2024-01-03 15:11 ` Ihor Radchenko
2024-01-04 8:59 ` gerard.vermeulen
2024-01-04 14:43 ` Ihor Radchenko
2024-01-07 18:49 ` [PATCH] org-babel-demarcate-block: split using element API gerard.vermeulen
2024-01-08 12:08 ` Ihor Radchenko [this message]
2024-01-08 20:25 ` gerard.vermeulen
2024-01-09 7:49 ` gerard.vermeulen
2024-01-09 10:50 ` gerard.vermeulen
2024-01-09 14:49 ` Ihor Radchenko
2024-01-13 14:04 ` gerard.vermeulen
2024-01-13 15:17 ` Ihor Radchenko
2024-01-13 20:16 ` gerard.vermeulen
2024-01-14 10:53 ` gerard.vermeulen
2024-01-14 12:16 ` Ihor Radchenko
2024-01-14 19:18 ` gerard.vermeulen
2024-01-15 9:37 ` gerard.vermeulen
2024-01-16 13:34 ` Ihor Radchenko
2024-02-19 9:46 ` Ihor Radchenko
2024-02-19 13:01 ` gerard.vermeulen
2024-02-21 9:40 ` Ihor Radchenko
2024-02-21 18:19 ` gerard.vermeulen
2024-02-22 16:28 ` gerard.vermeulen
2024-02-23 13:43 ` Ihor Radchenko
2024-02-25 12:06 ` gerard.vermeulen
2024-02-25 12:21 ` Ihor Radchenko
2024-02-26 8:51 ` gerard.vermeulen
2024-02-28 11:54 ` Ihor Radchenko
2024-02-29 9:50 ` gerard.vermeulen
2024-02-29 11:56 ` Ihor Radchenko
2024-02-29 17:33 ` gerard.vermeulen
2024-03-03 13:08 ` Ihor Radchenko
2024-03-03 15:45 ` gerard.vermeulen
2024-03-04 10:12 ` Ihor Radchenko
2024-03-04 11:40 ` gerard.vermeulen
2024-03-04 11:51 ` Ihor Radchenko
2024-02-26 9:06 ` gerard.vermeulen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.orgmode.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=878r50yqmo.fsf@localhost \
--to=yantar92@posteo.net \
--cc=emacs-orgmode@gnu.org \
--cc=gerard.vermeulen@posteo.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).