From: Ihor Radchenko <yantar92@gmail.com>
To: Valentin Lab <valentin.lab@kalysto.org>
Cc: emacs-orgmode@gnu.org
Subject: Re: [feature] Consistent fixed indentation of headline data
Date: Mon, 18 Jul 2022 21:26:19 +0800 [thread overview]
Message-ID: <87cze2h7r8.fsf@localhost> (raw)
In-Reply-To: <c13cab60-bbc9-e69e-6d0d-7fe75c5908d6@kalysto.org>
Valentin Lab <valentin.lab@kalysto.org> writes:
>> 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
>> org-adapt-indentation?
>>
>
> 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).
Your doubts make sense. In fact, I somehow missed that you only offered
to indent the headline data to fixed indentation.
Now, after more thinking and thoughtfully reading the relevant code, I
find a new variable more reasonable. However, I do not think that the
new variable should be `org-headline-data-fixed-indent-level'. Instead,
we can offer something like `org-indent-level' with default value 'auto
referring to (1+ (org-current-level)) and possible value of integer -
what you propose. Then, org-adapt-indentation will control the
indentation as usual, but the actual indentation will be calculated
depending on the `org-indent-level' value.
As an added bonus, users will also be able to set fixed indentation
using the combination of org-adapt-indentation=t and
org-indent-level=some-number. Not that I find it useful, but it will be
consistent. Moreover, the implementation will be nothing but changing
the calculation of headline contents indentation from
(1+ (org-current-level)) to a slightly more complex branching.
>> Note that your patch is _not_ a tiny change (not <15 LOC).
>> See https://orgmode.org/worg/org-contribute.html#first-patch and
>> https://orgmode.org/worg/org-contribute.html#copyright
>> 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.
Note that FSF should reply within 5 working days. If they don't, let us know.
>>> @@ -18371,6 +18386,9 @@ ELEMENT."
>>> ;; a footnote definition.
>>> (org--get-expected-indentation
>>> (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...
AFAIU, the correct test is the one used inside org-indent-line:
(and (eq org-adapt-indentation 'headline-data)
(not (or (org-at-clock-log-p)
(org-at-planning-p)))
(save-excursion
(beginning-of-line 1)
(skip-chars-backward "\n")
(or (org-at-heading-p)
(looking-back ":END:.*" (point-at-bol)))))
(inverse of)
Best,
Ihor
prev parent reply other threads:[~2022-07-18 13:26 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-05 14:59 [feature] Consistent fixed indentation of headline data Valentin Lab
2022-07-07 10:41 ` Ihor Radchenko
2022-07-11 19:02 ` Valentin Lab
2022-07-18 13:11 ` [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
2022-07-18 17:28 ` [PATCH] Fix bug in org-indent-region when org-adapt-indentation is set to headline-data Bastien Guerry
2022-07-19 13:59 ` Ihor Radchenko
2022-07-18 13:26 ` Ihor Radchenko [this message]
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=87cze2h7r8.fsf@localhost \
--to=yantar92@gmail.com \
--cc=emacs-orgmode@gnu.org \
--cc=valentin.lab@kalysto.org \
/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).