[PATCH] Fix bug in org-indent-region when org-adapt-indentation is set to headline-data (was: [feature] Consistent fixed indentation of headline data)

[Ihor Radchenko <yantar92@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2521 bytes --]

Valentin Lab <valentin.lab@kalysto.org> writes:

>>> @@ -1216,6 +1259,13 @@
>>>               (org-test-with-temp-text "* H\n:PROPERTIES:\n:key:\n:END:"
>>>                 (org-indent-region (point-min) (point-max))
>>>                 (buffer-string)))))
>>> +  ;; ;; Indent property drawers according to `org-adapt-indentation'.
>>> +  ;; (let ((org-adapt-indentation 'headline-data))
>>> +  ;;   (should
>>> +  ;;    (equal "* H\n  :PROPERTIES:\n  :key:\n  :END:\n\ncontent2"
>>> +  ;;           (org-test-with-temp-text "* H\n:PROPERTIES:\n:key:\n:END:\n\ncontent"
>>> +  ;;             (org-indent-region (point-min) (point-max))
>>> +  ;;             (buffer-string)))))
>> 
>> This test is commented. Is it intentional?
>
> My bad ! and an interesting talking point. I'm removing these commented 
> line in the upcoming patch. They were here (and inadvertently committed) 
> because while trying to test that my addition would not indent beyond 
> the headline data, I noticed that actually `org-adapt-indentation' set 
> to `headline-data' was actually indenting beyond headline data ! As I 
> don't want to break anything, I was left quite puzzled with what to do:
> - I can fix this, but fixing this is for me subject to another 
> submission, and will touch behaviors that might be wanted,
> - Not fixing this make me submitting a feature that carries what I see 
> like a "bug".
>
> But, is that a bug ? Here is the case:
>
> --8<---------------cut here---------------start------------->8---
> * H
> :PROPERTIES:
> :key:
> :END:
>
> content
> --8<---------------cut here---------------start------------->8---
>
> Using `org-indent-region' on all the content, with 
> `org-adapt-indentation' set to `headline-data', will result to:
>
> --8<---------------cut here---------------start------------->8---
> * H
>    :PROPERTIES:
>    :key:
>    :END:
>
>    content
> --8<---------------cut here---------------start------------->8---
>
> My issue is with the treatment of the 'content' line that is not 
> headline-data for me, and should not have been indented. Am I right in 
> my expectation ?

Yes are right and we got a bug here.

I am attaching the tentative fix.

Bastien, I feel that the current implementation of
`org--get-expected-indentation' is wrong since we introduced
`org-adapt-indentation' 'headline-data value. AFAIU, it was done by you
in e3b79ad2b. See the commit message in the patch below for the
explanation why I think that something is going wrong.

Am I missing something?

Best,
Ihor


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-indent-region-Fix-when-org-adapt-indentation-is-.patch --]
[-- Type: text/x-patch, Size: 2942 bytes --]

From 9917d69543226c68ca9e749e4f53e5f3e8ec8e79 Mon Sep 17 00:00:00 2001
Message-Id: <9917d69543226c68ca9e749e4f53e5f3e8ec8e79.1658149652.git.yantar92@gmail.com>
From: Ihor Radchenko <yantar92@gmail.com>
Date: Mon, 18 Jul 2022 21:00:13 +0800
Subject: [PATCH] org-indent-region: Fix when `org-adapt-indentation' is
 'headline-data

* lisp/org.el (org--get-expected-indentation): Remove the extra
condition added in e3b79ad2b in the cond branch for first line in an
element.  Checking `org-adapt-indentation' t value here trigger the
last default cond branch that assumes that we are _not_ at the first
line.

The new logic explicitly avoids inheriting indentation from previous
sibling when `org-adapt-indentation' is set to 'headline-data and the
previous sibling is satisfying "headline data" condition as in
`org-indent-line'.  The case when `org-adapt-indentation' is set to t
is already handled correctly when calculating the CONTENTSP
indentation for parent headline.

Fixes https://orgmode.org/list/c13cab60-bbc9-e69e-6d0d-7fe75c5908d6@kalysto.org
---
 lisp/org.el | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index 99e5d0dc7..64b148d9c 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -18455,9 +18455,9 @@ (defun org--get-expected-indentation (element contentsp)
 	(org-element-property :parent element) t))
       ;; At first line: indent according to previous sibling, if any,
       ;; ignoring footnote definitions and inline tasks, or parent's
-      ;; contents.
-      ((and ( = (line-beginning-position) start)
-	    (eq org-adapt-indentation t))
+      ;; contents.  If `org-adapt-indentation' is `headline-data', ignore
+      ;; previous headline data siblings.
+      ((= (line-beginning-position) start)
        (catch 'exit
 	 (while t
 	   (if (= (point-min) start) (throw 'exit 0)
@@ -18474,6 +18474,21 @@ (defun org--get-expected-indentation (element contentsp)
 		((memq (org-element-type previous)
 		       '(footnote-definition inlinetask))
 		 (setq start (org-element-property :begin previous)))
+                ;; Do not indent like previous when the previous
+                ;; element is headline data and `org-adapt-indentation'
+                ;; is set to `headline-data'.
+                ((save-excursion
+                   (goto-char start)
+                   (and
+                    (eq org-adapt-indentation 'headline-data)
+                    (not (or (org-at-clock-log-p)
+                           (org-at-planning-p)))
+                    (progn
+                      (beginning-of-line 1)
+                      (skip-chars-backward "\n")
+                      (or (org-at-heading-p)
+                          (looking-back ":END:.*" (point-at-bol))))))
+                 (throw 'exit 0))
 		(t (goto-char (org-element-property :begin previous))
 		   (throw 'exit
 			  (if (bolp) (current-indentation)
-- 
2.35.1