Re: org-insert-heading rewritten from scratch

[Carsten Dominik <carsten.dominik@gmail.com>]

Hi Nioclas,

thank you for the feedback.

On 1.9.2013, at 10:19, Nicolas Goaziou <n.goaziou@gmail.com> wrote:

> Hello,
> 
> Carsten Dominik <carsten.dominik@gmail.com> writes:
> 
>> On 31.8.2013, at 16:34, Nicolas Goaziou <n.goaziou@gmail.com> wrote:
> 
>>> Not really a bug, but I find some behaviour surprising: when at a the
>>> beginning of a regular text line, there is no way to create a headline
>>> just above it. In the following example:
>>> 
>>> XCursor is at "X"
>>> 
>>> Neither M-RET, C-u M-RET, C-RET nor C-u C-RET can do it. Is it intended?
>> 
>> Which behavior would you propose?  I guess you mean that, at the beginning of a line,
>> the result is so different for normal lines versus headlines?
> 
> Yes.
> 
> I never, ever, want to turn a regular text line into a headline. OTOH
> during note taking, I very often write paragraphs and, afterwards,
> decide to split them into sections.

OK, I'll go along with this.  `C-c *' if for turning a line into a headline,
M-RET for making a *new* headline.  That is logical.  Fixed now.

> 
> Moreover, AFAICT, there's no more difference between C-u M-RET, which
> meant "create headline right here" and M-RET.

C-u means: Do not ask org-insert-item, just go ahead and insert a heading.
That is supposed to be the only difference.

> 
>> The way I was thinking about the behavior at the beginning of a non-headline
>> is that it is the same as in the middle of a line:  Take the rest of the line
>> and turn it into a headline.p
>> 
>> To create a headline before a nonempty line, I use `C-o M-RET'
> 
> That's what I do. But that's not optimal and it introduces another
> problem. In the following example
> 
>  * H
> 
>  XText
> 
> I want to create a headline above "Text" and point is at "X". I use C-o
> M-RET and the latter greedily eat the blank line above, resulting in
> 
>  * H
>  * X
>  Text
> 
> as if I had typed C-p M-RET instead.
> 
> Again, I have `org-blank-before-new-entry' set to `auto' for headlines.
> Since there is no information about how many blank lines I usually want
> before headlines, I think the algorithm should trust me and do not
> remove any blank lines (see `org-list-separating-blank-lines-number').

I believe this is now better, please check.

> 
> In the same vein, in the following situation
> 
>  * H1
> 
>    Text1
> 
>  * H2
> 
>    XText2
> 
> C-o M-RET should leave blank above "Text2" because it has information
> about my preferences.

I think this is fixed as well.

> 
> Oddly, in an empty buffer, it will create a blank line above.

Fixed.

> 
>>> Also in this case, I think C-RET should create the new headline _after_
>>> the subtree, since that's its whole point anyway, AFAIU.
>> 
>> That is what happens for me.  It does not for you?
> 
> It does, sorry about the noise.
> 
>> Hmm, I do find this behavior consistent. M-RET does not change the number
>> of while lines after the current, only before, in order to either have
>> an empty line or not.
>> 
>> Which behavior would you propose?
> 
> Well same as above: I think it eats blank lines where it shouldn't. It
> the following cases:
> 
>  * H1
> 
>  ** H2
> 
>     H
>  X
> 
> and
> 
>  * H1
> 
>  * H2
> 
>    H
> 
>  X  
> 
> I don't think there's any reason for M-RET to eat blank line before
> point with either `org-blank-before-new-entry' set to `auto' or t. It
> should know that a blank line is expected before the new entry and
> therefore should create the headline at point.

Please take another look.

Thank you!

- Carsten

> 
>>> I also suggest to write function specifications as tests in test-org.el.
>> 
>> Yes, I have yet to write my first test.  Need to figure out how that
>> works.
> 
> That's simple. You can use the following pattern:
> 
>  (ert-deftest test-org/insert-heading ()
>    "Test specifications for heading insertion."
>    ;; In an empty buffer, headline should be created at its beginning,
>    ;; notwithstanding value for `org-blank-before-new-entry'.
>    (should
>     (equal "* "
>            (org-test-with-temp-text ""
>              (let ((org-blank-before-new-entry '((heading . nil))))
>                (org-insert-heading))
>              (buffer-string))))
>    (should
>     (equal "* "
>            (org-test-with-temp-text ""
>              (let ((org-blank-before-new-entry '((heading . t))))
>                (org-insert-heading))
>              (buffer-string))))
>    (should
>     (equal "* "
>            (org-test-with-temp-text ""
>              (let ((org-blank-before-new-entry '((heading . auto))))
>                (org-insert-heading))
>              (buffer-string))))
>    ;; At the end of a single headline: Create headline below, following
>    ;; `org-blank-before-new-entry' specifications.  When it is `auto',
>    ;; since there's not enough information to deduce what is expected,
>    ;; create it just below.
>    (should
>     (equal "* H\n* "
>            (org-test-with-temp-text "* H"
>              (end-of-line)
>              (let ((org-blank-before-new-entry '((heading . nil))))
>                (org-insert-heading))
>              (buffer-string))))
>    (should
>     (equal "* H\n\n* "
>            (org-test-with-temp-text "* H"
>              (end-of-line)
>              (let ((org-blank-before-new-entry '((heading . t))))
>                (org-insert-heading))
>              (buffer-string))))
>    (should
>     (equal "* H\n* "
>            (org-test-with-temp-text "* H"
>              (end-of-line)
>              (let ((org-blank-before-new-entry '((heading . auto))))
>                (org-insert-heading))
>              (buffer-string))))
>    ;; Etc.
>    )
> 
> I suggest to always put the `should' (or `should-not', `should-error')
> outside each test: it makes it easier to inspect results from partial
> evaluations.
> 
> You run each test individually with C-x C-e at the end of the `should'
> sexp. You run all tests with "make test" from "org/" directory.
> 
> 
> Regards,
> 
> -- 
> Nicolas Goaziou