Valentin Lab <valentin....@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