Re: [PATCH] org-babel-demarcate-block: split using org-element instead of regexp

[Ihor Radchenko <yantar92@posteo.net>]

gerard.vermeulen@posteo.net writes:

>> I made some adjustments to the patch, making use of org-element API.
>> See the attached updated version of the patch.

Thanks! See my comments inline.

> 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 (&optional 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
<point>

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

> +  (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'

> +        ;; 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"?

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

Are you using (org-babel-where-is-src-block-head) for side effect of
modifying the match data? If so, please do it outside let, with
appropriate comment.

> +          (if (not org-adapt-indentation)
> +              ;; Move point to the left of the lower block line #+begin_src.
> +              (org-previous-block 1)
> +            ;; Adapt the indentation: upper block first and lower block second.
> +            (org-previous-block 2)
> +            (org-indent-block)
> +            ;; Move point to the left of the lower block line #+begin_src.
> +            (org-next-block 1)
> +            (org-indent-block)))

`org-indent-block' should honor `org-adapt-indentation'. You do not need
to call it conditionally. Re-indenting unconditionally should be better
here.

>        (let ((start (point))
> -	    (lang (or (car info) ; Reuse language from previous block.
> -                      (completing-read
> -		       "Lang: "
> -		       (mapcar #'symbol-name
> -			       (delete-dups
> -			        (append (mapcar #'car org-babel-load-languages)
> -				        (mapcar (lambda (el) (intern (car el)))
> -					        org-src-lang-modes)))))))
> +            ;; (org-babel-get-src-block-info 'no-eval) returns nil,
> +            ;; since there is no source block at point.  Therefore, this
> +            ;; cannot be used to get the language of a neighbour block.

Why nil? The condition was

  (and info start) ;; At src block, but not within blank lines after it.

So, this branch of the if used to be INFO - non-nil, and START nil ->
re-use the information. And if INFO were nil, query.

> +            ;; Deleted code indicated that this may have worked in the past.
> +            ;; I have removed upper-case-p, since it could never be true here.

The idea of UPPER-CASE-P is to keep user preference for keyword style
(upper case or lower case). There is no reason to remove this feature.
Although, since we are using `org-element-interpret-data', it might be a
good idea to extend org-element parser to preserve the keyword case
information.

-- 
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>