[Top][All Lists]

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

Re: [feature] Consistent fixed indentation of headline data

From: Valentin Lab
Subject: Re: [feature] Consistent fixed indentation of headline data
Date: Mon, 11 Jul 2022 21:02:52 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.1

Many thanks for your warm welcome, kind review and suggestions.
I do not provide a corrective patch in this email, but it'll come soon depending on possible other remarks or follow-ups of this email.

On 07/07/2022 12:41, Ihor Radchenko wrote:
Valentin Lab <valentin.lab@kalysto.org> writes:

Note that we rarely change the defaults. This particular change came
after extensive discussion:
Also, it has been documented in https://orgmode.org/Changes.html
I recommend reviewing the changes every time you update Org to new

These links are very informative and give me much more insight on the current situation. Sorry if I didn't know how to get this info by myself. I've read the full thread you've linked.

Note that introducing a new customization should be documented in
etc/ORG-NEWS file and probably also in the manual (17.4.2 Hard

Yes, I was aware of that, but didn't want to spend to much time if my contribution was deemed not pertinent.

Also, I am not sure if we really need a new custom variable. We already
have many. What about simply allowing an integer value of

Well, my guess was that the "adapt" word in `org-adapt-indentation' was referring to adaptive (in other words: "changing") indentation depending on the depth of the outline. It seemed at first un-natural to force a fixed behavior here. This is why I went with a sub-behavior of when `org-adapt-indentation' was set to nil, a way to tell that we didn't want adaptive indentation. It also had the advantage to separate the implementation and help writing code that would not break the existing behaviors.

Of course, I'm completely okay to go with your suggestion. Although, I'm at a lack of inspiration about what would be the best option here: wouldn't a simple integer as you suggest hide the fact that this will target only the headline data ? Could we think of more structured value, perhaps like a cons =(`headline-data . 2)= ? I'm afraid these additions could be seen as bringing some unwanted complexity.

Although I'm expressing some doubts, I'm ready to implement it using an integer on `org-adapt-indentation' as you suggest. Just wanted to make sure it seem sound to everyone before committing to a change that is not that trivial (well, compared to actual changes).


Signed-off-by: Valentin Lab <valentin.lab@kalysto.org>

Note that your patch is _not_ a tiny change (not <15 LOC).
See https://orgmode.org/worg/org-contribute.html#first-patch and
Would you consider signing the copyright assignment papers with FSF?

Ok, I was not sure about that (counting the actual functional code change was still under 15LOC, but I guess test counts also). I'm in the process of signing the copyright assignment papers.

@@ -18371,6 +18386,9 @@ ELEMENT."
                            ;; a footnote definition.
                             (org-element-property :parent previous) t))))))))))
+      ((and (not (eq org-headline-data-fixed-indent-level nil))
+         (memq type '(drawer property-drawer planning node-property clock)))
+         org-headline-data-fixed-indent-level)
        ;; Otherwise, move to the first non-blank line above.

Why clock? It does not belong to property drawers.

I probably lack some important knowledge here to understand your point. Here's what I understand: `clock' type targets the 'CLOCK:' keyword (and not direct timestamps, I added a test to make sure). AFAIK, these 'CLOCK:' lines are made with 'C-c C-x C-i/o' and usually appears in between a ':LOGBOOK:' drawer. However older implementation of org-mode did not include these 'CLOCK: ...' lines in logbook drawers. My intention here, was to make sure that even these orphaned 'CLOCK: ...' lines, when appearing before any content, would be considered as headline data, and as such, be indented by the fixed amount.

I definitively consider the logbook (and clock out of logbooks), property drawer, and planning info ("SCHEDULED", "DEADLINE") as headline data and are all targeted for indentation. In my code, if I remove `clock' in the list of types, the intended test about 'CLOCK:' fails...

@@ -1216,6 +1259,13 @@
              (org-test-with-temp-text "* H\n:PROPERTIES:\n:key:\n:END:"
                (org-indent-region (point-min) (point-max))
+  ;; ;; 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 "* 
+  ;;             (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

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

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

Here is the test that fails (that can be copy/pasted):

#+begin_src emacs-lisp
  ;; ...
  (let ((org-adapt-indentation 'headline-data))
     (equal "* H\n  :PROPERTIES:\n  :key:\n  :END:\n\ncontent"
(org-test-with-temp-text "* H\n:PROPERTIES:\n:key:\n:END:\n\ncontent"
              (org-indent-region (point-min) (point-max))
  ;; ...

Many thanks for any insights on this point.

Thanks for your review, suggestions and advices,

Valentin Lab

reply via email to

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