Re: org-insert-heading rewritten from scratch

[Nicolas Goaziou <n.goaziou@gmail.com>]

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.

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

> 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').

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.

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

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

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