emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* org-insert-heading rewritten from scratch
@ 2013-08-08  6:43 Carsten Dominik
  2013-08-08  7:41 ` Eric Abrahamsen
  2013-09-09 16:42 ` Michael Brand
  0 siblings, 2 replies; 27+ messages in thread
From: Carsten Dominik @ 2013-08-08  6:43 UTC (permalink / raw)
  To: Emacs Org mode mailing list

Hi,

I have rewritten org-insert-heading, because it had become an unmaintainable beast.
Please follow up in this thread if you find problems with the new implementation.
Very likely there will be bugs, but now I am at least confident they can be fixed.

- Carsten

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: org-insert-heading rewritten from scratch
  2013-08-08  6:43 org-insert-heading rewritten from scratch Carsten Dominik
@ 2013-08-08  7:41 ` Eric Abrahamsen
  2013-08-31 14:00   ` Carsten Dominik
  2013-09-09 16:42 ` Michael Brand
  1 sibling, 1 reply; 27+ messages in thread
From: Eric Abrahamsen @ 2013-08-08  7:41 UTC (permalink / raw)
  To: emacs-orgmode

Carsten Dominik <carsten.dominik@gmail.com> writes:

> Hi,
>
> I have rewritten org-insert-heading, because it had become an unmaintainable beast.
> Please follow up in this thread if you find problems with the new implementation.
> Very likely there will be bugs, but now I am at least confident they can be fixed.
>
> - Carsten

Awesome! Thanks very much for doing this. Will report back with bugs.

E

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: org-insert-heading rewritten from scratch
  2013-08-08  7:41 ` Eric Abrahamsen
@ 2013-08-31 14:00   ` Carsten Dominik
  2013-08-31 14:34     ` Nicolas Goaziou
                       ` (2 more replies)
  0 siblings, 3 replies; 27+ messages in thread
From: Carsten Dominik @ 2013-08-31 14:00 UTC (permalink / raw)
  To: Eric Abrahamsen; +Cc: emacs-orgmode


On 8.8.2013, at 09:41, Eric Abrahamsen <eric@ericabrahamsen.net> wrote:

> Carsten Dominik <carsten.dominik@gmail.com> writes:
> 
>> Hi,
>> 
>> I have rewritten org-insert-heading, because it had become an unmaintainable beast.
>> Please follow up in this thread if you find problems with the new implementation.
>> Very likely there will be bugs, but now I am at least confident they can be fixed.
>> 
>> - Carsten
> 
> Awesome! Thanks very much for doing this. Will report back with bugs.

I take it none have been found?

- Carsten

> 
> E
> 
> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: org-insert-heading rewritten from scratch
  2013-08-31 14:00   ` Carsten Dominik
@ 2013-08-31 14:34     ` Nicolas Goaziou
  2013-09-01  6:13       ` Carsten Dominik
  2013-09-01  2:38     ` Eric Abrahamsen
  2013-09-01  3:11     ` Samuel Wales
  2 siblings, 1 reply; 27+ messages in thread
From: Nicolas Goaziou @ 2013-08-31 14:34 UTC (permalink / raw)
  To: Carsten Dominik; +Cc: Eric Abrahamsen, emacs-orgmode

Hello,

Carsten Dominik <carsten.dominik@gmail.com> writes:

> On 8.8.2013, at 09:41, Eric Abrahamsen <eric@ericabrahamsen.net> wrote:
>
>> Carsten Dominik <carsten.dominik@gmail.com> writes:
>> 
>>> Hi,
>>> 
>>> I have rewritten org-insert-heading, because it had become an unmaintainable beast.
>>> Please follow up in this thread if you find problems with the new implementation.
>>> Very likely there will be bugs, but now I am at least confident they can be fixed.
>>> 
>>> - Carsten
>> 
>> Awesome! Thanks very much for doing this. Will report back with bugs.
>
> I take it none have been found?

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?

Also in this case, I think C-RET should create the new headline _after_
the subtree, since that's its whole point anyway, AFAIU.

Eventually, it seems that behaviour towards empty lines is a bit
unpredictable. In the following example, M-RET will be behave
differently on each blank line between Paragraph and "H2". C-RET will be
consistent. Note: I have `auto' as value for `heading' key in
`org-blank-before-new-entry'.

  * H1

    Paragraph



  * H2

IMO, using M-RET and C-RET should be as smooth an experience as
possible.

I also suggest to write function specifications as tests in test-org.el.


Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: org-insert-heading rewritten from scratch
  2013-08-31 14:00   ` Carsten Dominik
  2013-08-31 14:34     ` Nicolas Goaziou
@ 2013-09-01  2:38     ` Eric Abrahamsen
  2013-09-01  3:11     ` Samuel Wales
  2 siblings, 0 replies; 27+ messages in thread
From: Eric Abrahamsen @ 2013-09-01  2:38 UTC (permalink / raw)
  To: emacs-orgmode

Carsten Dominik <carsten.dominik@gmail.com> writes:

> On 8.8.2013, at 09:41, Eric Abrahamsen <eric@ericabrahamsen.net> wrote:
>
>> Carsten Dominik <carsten.dominik@gmail.com> writes:
>> 
>>> Hi,
>>> 
>>> I have rewritten org-insert-heading, because it had become an unmaintainable beast.
>>> Please follow up in this thread if you find problems with the new implementation.
>>> Very likely there will be bugs, but now I am at least confident they can be fixed.
>>> 
>>> - Carsten
>> 
>> Awesome! Thanks very much for doing this. Will report back with bugs.
>
> I take it none have been found?
>
> - Carsten
>

Nope, I've been using this heavily and haven't seen anything surprising.
I do confirm Nicolas' observations on whitespace, though...

E

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: org-insert-heading rewritten from scratch
  2013-08-31 14:00   ` Carsten Dominik
  2013-08-31 14:34     ` Nicolas Goaziou
  2013-09-01  2:38     ` Eric Abrahamsen
@ 2013-09-01  3:11     ` Samuel Wales
  2013-09-01  3:13       ` Samuel Wales
  2 siblings, 1 reply; 27+ messages in thread
From: Samuel Wales @ 2013-09-01  3:11 UTC (permalink / raw)
  To: Carsten Dominik; +Cc: Eric Abrahamsen, emacs-orgmode

I've found four, but they are minor.  In capture, you can create a
blank line with m-ret at the top.Repeated invocation creates invalid
headlines.  There is no c-ret way to create a headline above the first
child.  M-ret is slow.

On 8/31/13, Carsten Dominik <carsten.dominik@gmail.com> wrote:
> I take it none have been found?

-- 
The Kafka Pandemic: http://thekafkapandemic.blogspot.com

The disease DOES progress.  MANY people have died from it.  ANYBODY can get it.

Denmark: free Karina Hansen NOW.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: org-insert-heading rewritten from scratch
  2013-09-01  3:11     ` Samuel Wales
@ 2013-09-01  3:13       ` Samuel Wales
  0 siblings, 0 replies; 27+ messages in thread
From: Samuel Wales @ 2013-09-01  3:13 UTC (permalink / raw)
  To: Carsten Dominik; +Cc: Eric Abrahamsen, emacs-orgmode

Clarification: it is no more slow than the original and it fixes bugs.
 Just answering your question about whether there are any bugs.

-- 
The Kafka Pandemic: http://thekafkapandemic.blogspot.com

The disease DOES progress.  MANY people have died from it.  ANYBODY can get it.

Denmark: free Karina Hansen NOW.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: org-insert-heading rewritten from scratch
  2013-08-31 14:34     ` Nicolas Goaziou
@ 2013-09-01  6:13       ` Carsten Dominik
  2013-09-01  8:19         ` Nicolas Goaziou
  0 siblings, 1 reply; 27+ messages in thread
From: Carsten Dominik @ 2013-09-01  6:13 UTC (permalink / raw)
  To: Nicolas Goaziou; +Cc: Eric Abrahamsen, emacs-orgmode

Hi Nicolas,

On 31.8.2013, at 16:34, Nicolas Goaziou <n.goaziou@gmail.com> wrote:

> Hello,
> 
> Carsten Dominik <carsten.dominik@gmail.com> writes:
> 
>> On 8.8.2013, at 09:41, Eric Abrahamsen <eric@ericabrahamsen.net> wrote:
>> 
>>> Carsten Dominik <carsten.dominik@gmail.com> writes:
>>> 
>>>> Hi,
>>>> 
>>>> I have rewritten org-insert-heading, because it had become an unmaintainable beast.
>>>> Please follow up in this thread if you find problems with the new implementation.
>>>> Very likely there will be bugs, but now I am at least confident they can be fixed.
>>>> 
>>>> - Carsten
>>> 
>>> Awesome! Thanks very much for doing this. Will report back with bugs.
>> 
>> I take it none have been found?
> 
> 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?

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.

To create a headline before a nonempty line, I use `C-o M-RET'

To be sure:  I am happy to change the behavior if that is what people
want and if it makes logically sense.

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

> 
> Eventually, it seems that behaviour towards empty lines is a bit
> unpredictable. In the following example, M-RET will be behave
> differently on each blank line between Paragraph and "H2". C-RET will be
> consistent.

Because it does what M-RET does in the last line of the subtree.

> Note: I have `auto' as value for `heading' key in
> `org-blank-before-new-entry'.
> 
>  * H1
> 
>    Paragraph
> 
> 
> 
>  * H2


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?

> 
> IMO, using M-RET and C-RET should be as smooth an experience as
> possible.

I fully agree.

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

- Carsten

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: org-insert-heading rewritten from scratch
  2013-09-01  6:13       ` Carsten Dominik
@ 2013-09-01  8:19         ` Nicolas Goaziou
  2013-09-01 12:04           ` Carsten Dominik
  0 siblings, 1 reply; 27+ messages in thread
From: Nicolas Goaziou @ 2013-09-01  8:19 UTC (permalink / raw)
  To: Carsten Dominik; +Cc: Eric Abrahamsen, emacs-orgmode

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: org-insert-heading rewritten from scratch
  2013-09-01  8:19         ` Nicolas Goaziou
@ 2013-09-01 12:04           ` Carsten Dominik
  2013-09-01 12:25             ` Nicolas Goaziou
  2013-09-03 13:16             ` Nicolas Goaziou
  0 siblings, 2 replies; 27+ messages in thread
From: Carsten Dominik @ 2013-09-01 12:04 UTC (permalink / raw)
  To: Nicolas Goaziou; +Cc: Eric Abrahamsen, emacs-orgmode

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: org-insert-heading rewritten from scratch
  2013-09-01 12:04           ` Carsten Dominik
@ 2013-09-01 12:25             ` Nicolas Goaziou
  2013-09-03 13:16             ` Nicolas Goaziou
  1 sibling, 0 replies; 27+ messages in thread
From: Nicolas Goaziou @ 2013-09-01 12:25 UTC (permalink / raw)
  To: Carsten Dominik; +Cc: Eric Abrahamsen, emacs-orgmode

Carsten Dominik <carsten.dominik@gmail.com> writes:

> Please take another look.

There is no new commit in maint or master.


Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: org-insert-heading rewritten from scratch
  2013-09-01 12:04           ` Carsten Dominik
  2013-09-01 12:25             ` Nicolas Goaziou
@ 2013-09-03 13:16             ` Nicolas Goaziou
  2013-09-03 13:25               ` Carsten Dominik
  1 sibling, 1 reply; 27+ messages in thread
From: Nicolas Goaziou @ 2013-09-03 13:16 UTC (permalink / raw)
  To: Carsten Dominik; +Cc: Eric Abrahamsen, emacs-orgmode

Hello,

Carsten Dominik <carsten.dominik@gmail.com> writes:

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

Point taken. I thought C-u had another meaning too.

> Please take another look.

It looks better. Though it seems to ignore a nil
`org-blank-before-new-entry' value for `heading':

  * H

  X

Here I get,

  * H

  * X

but, with a nil value, it should be:

  * H
  * X


Thanks.

Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: org-insert-heading rewritten from scratch
  2013-09-03 13:16             ` Nicolas Goaziou
@ 2013-09-03 13:25               ` Carsten Dominik
  2013-09-03 13:33                 ` Carsten Dominik
  2013-09-03 13:38                 ` Nicolas Goaziou
  0 siblings, 2 replies; 27+ messages in thread
From: Carsten Dominik @ 2013-09-03 13:25 UTC (permalink / raw)
  To: Nicolas Goaziou; +Cc: Eric Abrahamsen, emacs-orgmode


On Sep 3, 2013, at 3:16 PM, Nicolas Goaziou <n.goaziou@gmail.com> wrote:

> Hello,
> 
> Carsten Dominik <carsten.dominik@gmail.com> writes:
> 
>>> 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.
> 
> Point taken. I thought C-u had another meaning too.
> 
>> Please take another look.
> 
> It looks better. Though it seems to ignore a nil
> `org-blank-before-new-entry' value for `heading':
> 
>  * H
> 
>  X
> 
> Here I get,
> 
>  * H
> 
>  * X
> 
> but, with a nil value, it should be:
> 
>  * H
>  * X


Hmmm, I thought you just asked me to implement exactly what you see, namely that M-RET will not remove empty lines above the cursor - only add them.  Did I misunderstand?

Thanks

- Carsten

> 
> 
> Thanks.
> 
> Regards,
> 
> -- 
> Nicolas Goaziou

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: org-insert-heading rewritten from scratch
  2013-09-03 13:25               ` Carsten Dominik
@ 2013-09-03 13:33                 ` Carsten Dominik
  2013-09-03 13:38                 ` Nicolas Goaziou
  1 sibling, 0 replies; 27+ messages in thread
From: Carsten Dominik @ 2013-09-03 13:33 UTC (permalink / raw)
  To: Nicolas Goaziou; +Cc: Eric Abrahamsen, emacs-orgmode


On Sep 3, 2013, at 3:25 PM, Carsten Dominik <carsten.dominik@gmail.com> wrote:

> 
> On Sep 3, 2013, at 3:16 PM, Nicolas Goaziou <n.goaziou@gmail.com> wrote:
> 
>> Hello,
>> 
>> Carsten Dominik <carsten.dominik@gmail.com> writes:
>> 
>>>> 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.
>> 
>> Point taken. I thought C-u had another meaning too.
>> 
>>> Please take another look.
>> 
>> It looks better. Though it seems to ignore a nil
>> `org-blank-before-new-entry' value for `heading':
>> 
>> * H
>> 
>> X
>> 
>> Here I get,
>> 
>> * H
>> 
>> * X
>> 
>> but, with a nil value, it should be:
>> 
>> * H
>> * X
> 
> 
> Hmmm, I thought you just asked me to implement exactly what you see, namely that M-RET will not remove empty lines above the cursor - only add them.  Did I misunderstand?


OK, I just re-read your earlier mail, and you had only made this request for auto and t values of org-blank before new entry.

I'll contemplate this.

- Carsten

> 
> Thanks
> 
> - Carsten
> 
>> 
>> 
>> Thanks.
>> 
>> Regards,
>> 
>> -- 
>> Nicolas Goaziou
> 

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: org-insert-heading rewritten from scratch
  2013-09-03 13:25               ` Carsten Dominik
  2013-09-03 13:33                 ` Carsten Dominik
@ 2013-09-03 13:38                 ` Nicolas Goaziou
  2013-09-03 13:47                   ` Carsten Dominik
  1 sibling, 1 reply; 27+ messages in thread
From: Nicolas Goaziou @ 2013-09-03 13:38 UTC (permalink / raw)
  To: Carsten Dominik; +Cc: Eric Abrahamsen, emacs-orgmode

Carsten Dominik <carsten.dominik@gmail.com> writes:

> Hmmm, I thought you just asked me to implement exactly what you see,
> namely that M-RET will not remove empty lines above the cursor - only
> add them.  Did I misunderstand?

I think so.

M-RET should not remove (or add) anything when it has no information
whatsoever about what the user want, namely when behaviour is set to
`auto' _and_ number of blank lines (or lack thereof) cannot be deduced
from other headlines.

When `org-blank-before-new-entry' is set to t (respectively nil), the
message is pretty clear. There are no heuristics involved and the
function can add (respectively remove) blank lines to its heart's
contents.

Sorry if I wasn't clear enough.


Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: org-insert-heading rewritten from scratch
  2013-09-03 13:38                 ` Nicolas Goaziou
@ 2013-09-03 13:47                   ` Carsten Dominik
  2013-09-03 13:58                     ` Nicolas Goaziou
  0 siblings, 1 reply; 27+ messages in thread
From: Carsten Dominik @ 2013-09-03 13:47 UTC (permalink / raw)
  To: Nicolas Goaziou; +Cc: Eric Abrahamsen, emacs-orgmode

I am still not clear about this.  In your earlier mail you made this example:

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

WIth `auto', there will be an empty line before the next entry in both cases.  In both cases it looks at the H2 headline and sees the empty line before it.  Are you saying the behaviour should be different in both cases?

Cheers

- Carsten


On Sep 3, 2013, at 3:38 PM, Nicolas Goaziou <n.goaziou@gmail.com> wrote:

> Carsten Dominik <carsten.dominik@gmail.com> writes:
> 
>> Hmmm, I thought you just asked me to implement exactly what you see,
>> namely that M-RET will not remove empty lines above the cursor - only
>> add them.  Did I misunderstand?
> 
> I think so.
> 
> M-RET should not remove (or add) anything when it has no information
> whatsoever about what the user want, namely when behaviour is set to
> `auto' _and_ number of blank lines (or lack thereof) cannot be deduced
> from other headlines.
> 
> When `org-blank-before-new-entry' is set to t (respectively nil), the
> message is pretty clear. There are no heuristics involved and the
> function can add (respectively remove) blank lines to its heart's
> contents.
> 
> Sorry if I wasn't clear enough.
> 
> 
> Regards,
> 
> -- 
> Nicolas Goaziou

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: org-insert-heading rewritten from scratch
  2013-09-03 13:47                   ` Carsten Dominik
@ 2013-09-03 13:58                     ` Nicolas Goaziou
  2013-09-03 14:04                       ` Carsten Dominik
  0 siblings, 1 reply; 27+ messages in thread
From: Nicolas Goaziou @ 2013-09-03 13:58 UTC (permalink / raw)
  To: Carsten Dominik; +Cc: Eric Abrahamsen, emacs-orgmode

Carsten Dominik <carsten.dominik@gmail.com> writes:

> I am still not clear about this.  In your earlier mail you made this example:
>
>> 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.
>
> WIth `auto', there will be an empty line before the next entry in both
> cases. In both cases it looks at the H2 headline and sees the empty
> line before it. Are you saying the behaviour should be different in
> both cases?

Yes, it should.

As you point out, in both cases the algorithm knows that there should be
a blank line before the new entry (with the assumption that behaviour is
set to `auto'). In the first case, if it inserts the headline at point,
there will be none, so it has to add one. In the second case, there is
no need to add one since creating it at point will fulfill the
requirement (which is "a blank line before new headline").

Re-reading myself, I agree that my quoted explanations are a bit
confusing. I hope this should clarify my point.


Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: org-insert-heading rewritten from scratch
  2013-09-03 13:58                     ` Nicolas Goaziou
@ 2013-09-03 14:04                       ` Carsten Dominik
  2013-09-03 14:12                         ` Nicolas Goaziou
  2013-09-07 10:50                         ` Nicolas Goaziou
  0 siblings, 2 replies; 27+ messages in thread
From: Carsten Dominik @ 2013-09-03 14:04 UTC (permalink / raw)
  To: Nicolas Goaziou; +Cc: Eric Abrahamsen, emacs-orgmode


On Sep 3, 2013, at 3:58 PM, Nicolas Goaziou <n.goaziou@gmail.com> wrote:

> Carsten Dominik <carsten.dominik@gmail.com> writes:
> 
>> I am still not clear about this.  In your earlier mail you made this example:
>> 
>>> 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.
>> 
>> WIth `auto', there will be an empty line before the next entry in both
>> cases. In both cases it looks at the H2 headline and sees the empty
>> line before it. Are you saying the behaviour should be different in
>> both cases?
> 
> Yes, it should.
> 
> As you point out, in both cases the algorithm knows that there should be
> a blank line before the new entry (with the assumption that behaviour is
> set to `auto'). In the first case, if it inserts the headline at point,
> there will be none, so it has to add one. In the second case, there is
> no need to add one since creating it at point will fulfill the
> requirement (which is "a blank line before new headline").

Yes.  But you agree that the *result* should be the same, i.e. that there will
be an empty line before the newly inserted headline.

I think/hope we do agree now.

> 
> Re-reading myself, I agree that my quoted explanations are a bit
> confusing. I hope this should clarify my point.

I think so.

Thank you for your patience.

- Carsten

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: org-insert-heading rewritten from scratch
  2013-09-03 14:04                       ` Carsten Dominik
@ 2013-09-03 14:12                         ` Nicolas Goaziou
  2013-09-07 10:50                         ` Nicolas Goaziou
  1 sibling, 0 replies; 27+ messages in thread
From: Nicolas Goaziou @ 2013-09-03 14:12 UTC (permalink / raw)
  To: Carsten Dominik; +Cc: Eric Abrahamsen, emacs-orgmode

Carsten Dominik <carsten.dominik@gmail.com> writes:

> Yes.  But you agree that the *result* should be the same, i.e. that there will
> be an empty line before the newly inserted headline.
>
> I think/hope we do agree now.

We do.


Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: org-insert-heading rewritten from scratch
  2013-09-03 14:04                       ` Carsten Dominik
  2013-09-03 14:12                         ` Nicolas Goaziou
@ 2013-09-07 10:50                         ` Nicolas Goaziou
  2013-09-07 12:01                           ` Nicolas Goaziou
  1 sibling, 1 reply; 27+ messages in thread
From: Nicolas Goaziou @ 2013-09-07 10:50 UTC (permalink / raw)
  To: Carsten Dominik; +Cc: Eric Abrahamsen, emacs-orgmode

Hello,

There seem to be a bug when using M-RET at "X" in the following buffer:

  * H1
  TextX

Try it with different values for `org-blank-before-new-entry'.


Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: org-insert-heading rewritten from scratch
  2013-09-07 10:50                         ` Nicolas Goaziou
@ 2013-09-07 12:01                           ` Nicolas Goaziou
  0 siblings, 0 replies; 27+ messages in thread
From: Nicolas Goaziou @ 2013-09-07 12:01 UTC (permalink / raw)
  To: Carsten Dominik; +Cc: Eric Abrahamsen, emacs-orgmode

Nicolas Goaziou <n.goaziou@gmail.com> writes:

> There seem to be a bug when using M-RET at "X" in the following buffer:
>
>   * H1
>   TextX
>
> Try it with different values for `org-blank-before-new-entry'.

Well, please scratch that: I was on a dubious local branch. Sorry for
the noise.


Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: org-insert-heading rewritten from scratch
  2013-08-08  6:43 org-insert-heading rewritten from scratch Carsten Dominik
  2013-08-08  7:41 ` Eric Abrahamsen
@ 2013-09-09 16:42 ` Michael Brand
  2013-09-09 19:10   ` Nicolas Goaziou
  1 sibling, 1 reply; 27+ messages in thread
From: Michael Brand @ 2013-09-09 16:42 UTC (permalink / raw)
  To: Carsten Dominik; +Cc: Emacs Org mode mailing list

[-- Attachment #1: Type: text/plain, Size: 1063 bytes --]

Hi Carsten

On Thu, Aug 8, 2013 at 8:43 AM, Carsten Dominik
<carsten.dominik@gmail.com> wrote:
> I have rewritten org-insert-heading, because it had become an unmaintainable beast.
> Please follow up in this thread if you find problems with the new implementation.
> Very likely there will be bugs, but now I am at least confident they can be fixed.

On the way of rewiring my muscle memory from C-RET to M-RET for some
cases, I stumbled across this:

#+DRAWERS: MyStructuredDrawer
:MyStructuredDrawer:
- a
:END:

To insert a new item I once changed to use C-RET also on items. How is
one supposed to do this now within a drawer? M-RET just inserts an
empty line. I would like to suggest the attached patches with an ERT.
They change (fix?) org-meta-return to insert a new item in this case.

About the following different issue I don't care as much and only
wanted to report: C-RET before any headline when within a drawer, or
generally before any headline(?), could bark instead of changing to a
headline leading to invalid Org syntax within a drawer.

Michael

[-- Attachment #2: 0001-Add-ERTs-for-org-meta-return.patch.txt --]
[-- Type: text/plain, Size: 1621 bytes --]

From 6bc4c15c4a76a98c841e8a200c75f5a0737ffece Mon Sep 17 00:00:00 2001
From: Michael Brand <michael.ch.brand@gmail.com>
Date: Mon, 9 Sep 2013 18:38:58 +0200
Subject: [PATCH 1/2] Add ERTs for org-meta-return

* testing/lisp/test-org-list.el (test-org-list/insert-item): Adapt
docstring.
* (test-org-list/meta-return): New `ert-deftest' to test
`org-meta-return'.
---
 testing/lisp/test-org-list.el | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/testing/lisp/test-org-list.el b/testing/lisp/test-org-list.el
index ac81d4d..ea19606 100644
--- a/testing/lisp/test-org-list.el
+++ b/testing/lisp/test-org-list.el
@@ -627,7 +627,7 @@
     (should (org-invisible-p2))))
 
 (ert-deftest test-org-list/insert-item ()
-  "Test item insertion."
+  "Test item insertion with `org-insert-item'."
   ;; Blank lines specifications.
   ;;
   ;; Non-nil `org-blank-before-new-entry': insert a blank line, unless
@@ -713,6 +713,22 @@
        (forward-line -1)
        (looking-at "$")))))
 
+(ert-deftest test-org-list/meta-return ()
+  "Test item insertion with `org-meta-return'."
+  (should
+   (org-test-with-temp-text "- a"
+     (org-meta-return)
+     (beginning-of-line)
+     (looking-at "- $")))
+  ;; TODO Insert an item also in a drawer.
+  (should
+   (let ((org-drawers '("MYDRAWER")))
+     (org-test-with-temp-text ":MYDRAWER:\n- a\n:END:"
+       (forward-line)
+       (org-meta-return)
+       (forward-line -1)
+       (looking-at "$")))))
+
 (ert-deftest test-org-list/repair ()
   "Test `org-list-repair' specifications."
   ;; Repair indentation.
-- 
1.7.12.4 (Apple Git-37)


[-- Attachment #3: 0002-org-meta-return-Insert-an-item-also-in-a-drawer.patch.txt --]
[-- Type: text/plain, Size: 1846 bytes --]

From 9896499fb7f497a13857b5b86f33cfbf1b918029 Mon Sep 17 00:00:00 2001
From: Michael Brand <michael.ch.brand@gmail.com>
Date: Mon, 9 Sep 2013 18:40:07 +0200
Subject: [PATCH 2/2] org-meta-return: Insert an item also in a drawer

* lisp/org.el (org-meta-return): Exclude item from cond for drawer.
* testing/lisp/test-org-list.el (test-org-list/meta-return): On an
item in a drawer expect an item to be inserted.
---
 lisp/org.el                   | 3 ++-
 testing/lisp/test-org-list.el | 6 +++---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index 59a22a2..edc8725 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -20698,7 +20698,8 @@ See the individual commands for more information."
   (org-check-before-invisible-edit 'insert)
   (cond
    ((run-hook-with-args-until-success 'org-metareturn-hook))
-   ((or (org-at-drawer-p) (org-in-drawer-p) (org-at-property-p))
+   ((and (or (org-at-drawer-p) (org-in-drawer-p) (org-at-property-p))
+	 (not (org-in-item-p)))
     (newline-and-indent))
    ((org-at-table-p)
     (call-interactively 'org-table-wrap-region))
diff --git a/testing/lisp/test-org-list.el b/testing/lisp/test-org-list.el
index ea19606..f3ced15 100644
--- a/testing/lisp/test-org-list.el
+++ b/testing/lisp/test-org-list.el
@@ -720,14 +720,14 @@
      (org-meta-return)
      (beginning-of-line)
      (looking-at "- $")))
-  ;; TODO Insert an item also in a drawer.
+  ;; Insert an item also in a drawer.
   (should
    (let ((org-drawers '("MYDRAWER")))
      (org-test-with-temp-text ":MYDRAWER:\n- a\n:END:"
        (forward-line)
        (org-meta-return)
-       (forward-line -1)
-       (looking-at "$")))))
+       (beginning-of-line)
+       (looking-at "- $")))))
 
 (ert-deftest test-org-list/repair ()
   "Test `org-list-repair' specifications."
-- 
1.7.12.4 (Apple Git-37)


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: org-insert-heading rewritten from scratch
  2013-09-09 16:42 ` Michael Brand
@ 2013-09-09 19:10   ` Nicolas Goaziou
  2013-09-09 20:37     ` Michael Brand
  0 siblings, 1 reply; 27+ messages in thread
From: Nicolas Goaziou @ 2013-09-09 19:10 UTC (permalink / raw)
  To: Michael Brand; +Cc: Emacs Org mode mailing list, Carsten Dominik

Hello,

Michael Brand <michael.ch.brand@gmail.com> writes:

> About the following different issue I don't care as much and only
> wanted to report: C-RET before any headline when within a drawer, or
> generally before any headline(?), could bark instead of changing to a
> headline leading to invalid Org syntax within a drawer.

I don't think it should bark, but it should insert the headline after
the drawer, i.e. just before the first headline, if there's one at all,
or at the end of the buffer.

> From 9896499fb7f497a13857b5b86f33cfbf1b918029 Mon Sep 17 00:00:00 2001
> From: Michael Brand <michael.ch.brand@gmail.com>
> Date: Mon, 9 Sep 2013 18:40:07 +0200
> Subject: [PATCH 2/2] org-meta-return: Insert an item also in a drawer
>
> * lisp/org.el (org-meta-return): Exclude item from cond for drawer.
> * testing/lisp/test-org-list.el (test-org-list/meta-return): On an
> item in a drawer expect an item to be inserted.

FWIW, I think the test belongs to test-org.el, not to test-org-list.el.


Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: org-insert-heading rewritten from scratch
  2013-09-09 19:10   ` Nicolas Goaziou
@ 2013-09-09 20:37     ` Michael Brand
  2013-09-12 20:20       ` Carsten Dominik
  0 siblings, 1 reply; 27+ messages in thread
From: Michael Brand @ 2013-09-09 20:37 UTC (permalink / raw)
  To: Nicolas Goaziou; +Cc: Emacs Org mode mailing list

[-- Attachment #1: Type: text/plain, Size: 303 bytes --]

Hi Nicolas

On Mon, Sep 9, 2013 at 9:10 PM, Nicolas Goaziou <n.goaziou@gmail.com> wrote:
> FWIW, I think the test belongs to test-org.el, not to test-org-list.el.

Ok, I assume, then better also with more tests than only for list items.
See the attached patches that replace the previous ones.

Michael

[-- Attachment #2: 0001-Add-ERTs-for-org-meta-return.patch.txt --]
[-- Type: text/plain, Size: 1892 bytes --]

From 66f6d15234bda97fc2e197efd2f9cb7c6439fef1 Mon Sep 17 00:00:00 2001
From: Michael Brand <michael.ch.brand@gmail.com>
Date: Mon, 9 Sep 2013 22:32:36 +0200
Subject: [PATCH 1/2] Add ERTs for org-meta-return

* testing/lisp/test-org.el (test-org/meta-return): New `ert-deftest'
to test `org-meta-return'.
---
 testing/lisp/test-org.el | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el
index 8a1e9f1..4944c24 100644
--- a/testing/lisp/test-org.el
+++ b/testing/lisp/test-org.el
@@ -365,6 +365,51 @@
 
 
 \f
+;;; Editing
+
+;;;; Insert elements
+
+(ert-deftest test-org/meta-return ()
+  "Test M-RET (`org-meta-return')."
+  ;; In a table field insert a row above.
+  (should
+   (org-test-with-temp-text "| a |"
+     (forward-char)
+     (org-meta-return)
+     (forward-line -1)
+     (looking-at "|   |$")))
+  ;; In a paragraph change current line into a header.
+  (should
+   (org-test-with-temp-text "a"
+     (org-meta-return)
+     (beginning-of-line)
+     (looking-at "\* a$")))
+  ;; In an item insert an item, in this case above.
+  (should
+   (org-test-with-temp-text "- a"
+     (org-meta-return)
+     (beginning-of-line)
+     (looking-at "- $")))
+  ;; In a drawer and paragraph insert an empty line, in this case above.
+  (should
+   (let ((org-drawers '("MYDRAWER")))
+     (org-test-with-temp-text ":MYDRAWER:\na\n:END:"
+       (forward-line)
+       (org-meta-return)
+       (forward-line -1)
+       (looking-at "$"))))
+  ;; TODO In a drawer and item insert an item, in this case above.
+  (should
+   (let ((org-drawers '("MYDRAWER")))
+     (org-test-with-temp-text ":MYDRAWER:\n- a\n:END:"
+       (forward-line)
+       (org-meta-return)
+       (forward-line -1)
+       (looking-at "$")))))
+
+
+
+\f
 ;;; Links
 
 ;;;; Fuzzy Links
-- 
1.7.12.4 (Apple Git-37)


[-- Attachment #3: 0002-org-meta-return-Insert-an-item-also-in-a-drawer.patch.txt --]
[-- Type: text/plain, Size: 1762 bytes --]

From fddcaaed03630e7acc26ed701937b2ca17184fe5 Mon Sep 17 00:00:00 2001
From: Michael Brand <michael.ch.brand@gmail.com>
Date: Mon, 9 Sep 2013 22:33:47 +0200
Subject: [PATCH 2/2] org-meta-return: Insert an item also in a drawer

* lisp/org.el (org-meta-return): Exclude item from cond for drawer.
* testing/lisp/test-org.el (test-org/meta-return): In a drawer and
item insert an item.
---
 lisp/org.el              | 3 ++-
 testing/lisp/test-org.el | 6 +++---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index 59a22a2..edc8725 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -20698,7 +20698,8 @@ See the individual commands for more information."
   (org-check-before-invisible-edit 'insert)
   (cond
    ((run-hook-with-args-until-success 'org-metareturn-hook))
-   ((or (org-at-drawer-p) (org-in-drawer-p) (org-at-property-p))
+   ((and (or (org-at-drawer-p) (org-in-drawer-p) (org-at-property-p))
+	 (not (org-in-item-p)))
     (newline-and-indent))
    ((org-at-table-p)
     (call-interactively 'org-table-wrap-region))
diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el
index 4944c24..3538242 100644
--- a/testing/lisp/test-org.el
+++ b/testing/lisp/test-org.el
@@ -398,14 +398,14 @@
        (org-meta-return)
        (forward-line -1)
        (looking-at "$"))))
-  ;; TODO In a drawer and item insert an item, in this case above.
+  ;; In a drawer and item insert an item, in this case above.
   (should
    (let ((org-drawers '("MYDRAWER")))
      (org-test-with-temp-text ":MYDRAWER:\n- a\n:END:"
        (forward-line)
        (org-meta-return)
-       (forward-line -1)
-       (looking-at "$")))))
+       (beginning-of-line)
+       (looking-at "- $")))))
 
 
 
-- 
1.7.12.4 (Apple Git-37)


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: org-insert-heading rewritten from scratch
  2013-09-09 20:37     ` Michael Brand
@ 2013-09-12 20:20       ` Carsten Dominik
  2013-09-12 20:52         ` Michael Brand
  0 siblings, 1 reply; 27+ messages in thread
From: Carsten Dominik @ 2013-09-12 20:20 UTC (permalink / raw)
  To: Michael Brand; +Cc: Emacs Org mode mailing list, Nicolas Goaziou

[-- Attachment #1: Type: text/plain, Size: 764 bytes --]

Hi Michael,

thanks for the patch - I think we will go with Nicolas' patch for fixing org-insert-heading.  May I ask you to check if your tests still to the right thing, and resubmit the test patch?

Many thanks in advance.

- Carsten

On 9.9.2013, at 22:37, Michael Brand <michael.ch.brand@gmail.com> wrote:

> Hi Nicolas
> 
> On Mon, Sep 9, 2013 at 9:10 PM, Nicolas Goaziou <n.goaziou@gmail.com> wrote:
>> FWIW, I think the test belongs to test-org.el, not to test-org-list.el.
> 
> Ok, I assume, then better also with more tests than only for list items.
> See the attached patches that replace the previous ones.
> 
> Michael
> <0001-Add-ERTs-for-org-meta-return.patch.txt><0002-org-meta-return-Insert-an-item-also-in-a-drawer.patch.txt>


[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: org-insert-heading rewritten from scratch
  2013-09-12 20:20       ` Carsten Dominik
@ 2013-09-12 20:52         ` Michael Brand
  2013-09-12 20:58           ` Carsten Dominik
  0 siblings, 1 reply; 27+ messages in thread
From: Michael Brand @ 2013-09-12 20:52 UTC (permalink / raw)
  To: Carsten Dominik; +Cc: Emacs Org mode mailing list, Nicolas Goaziou

[-- Attachment #1: Type: text/plain, Size: 396 bytes --]

Hi Carsten

On Thu, Sep 12, 2013 at 10:20 PM, Carsten Dominik
<carsten.dominik@gmail.com> wrote:
> thanks for the patch - I think we will go with Nicolas' patch for fixing org-insert-heading.  May I ask you to check if your tests still to the right thing, and resubmit the test patch?
>
> Many thanks in advance.

Yes, merged patch attached. No changes in the test cases were necessary.

Michael

[-- Attachment #2: 0001-Add-ERTs-for-org-meta-return.patch.txt --]
[-- Type: text/plain, Size: 1888 bytes --]

From d87dd6c70c594b55abcabdb67ea6852b261a1a7c Mon Sep 17 00:00:00 2001
From: Michael Brand <michael.ch.brand@gmail.com>
Date: Thu, 12 Sep 2013 22:48:16 +0200
Subject: [PATCH] Add ERTs for org-meta-return

* testing/lisp/test-org.el (test-org/meta-return): New `ert-deftest'
to test `org-meta-return'.
---
 testing/lisp/test-org.el | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el
index 8a1e9f1..3538242 100644
--- a/testing/lisp/test-org.el
+++ b/testing/lisp/test-org.el
@@ -365,6 +365,51 @@
 
 
 \f
+;;; Editing
+
+;;;; Insert elements
+
+(ert-deftest test-org/meta-return ()
+  "Test M-RET (`org-meta-return')."
+  ;; In a table field insert a row above.
+  (should
+   (org-test-with-temp-text "| a |"
+     (forward-char)
+     (org-meta-return)
+     (forward-line -1)
+     (looking-at "|   |$")))
+  ;; In a paragraph change current line into a header.
+  (should
+   (org-test-with-temp-text "a"
+     (org-meta-return)
+     (beginning-of-line)
+     (looking-at "\* a$")))
+  ;; In an item insert an item, in this case above.
+  (should
+   (org-test-with-temp-text "- a"
+     (org-meta-return)
+     (beginning-of-line)
+     (looking-at "- $")))
+  ;; In a drawer and paragraph insert an empty line, in this case above.
+  (should
+   (let ((org-drawers '("MYDRAWER")))
+     (org-test-with-temp-text ":MYDRAWER:\na\n:END:"
+       (forward-line)
+       (org-meta-return)
+       (forward-line -1)
+       (looking-at "$"))))
+  ;; In a drawer and item insert an item, in this case above.
+  (should
+   (let ((org-drawers '("MYDRAWER")))
+     (org-test-with-temp-text ":MYDRAWER:\n- a\n:END:"
+       (forward-line)
+       (org-meta-return)
+       (beginning-of-line)
+       (looking-at "- $")))))
+
+
+
+\f
 ;;; Links
 
 ;;;; Fuzzy Links
-- 
1.7.12.4 (Apple Git-37)


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: org-insert-heading rewritten from scratch
  2013-09-12 20:52         ` Michael Brand
@ 2013-09-12 20:58           ` Carsten Dominik
  0 siblings, 0 replies; 27+ messages in thread
From: Carsten Dominik @ 2013-09-12 20:58 UTC (permalink / raw)
  To: Michael Brand; +Cc: Emacs Org mode mailing list, Nicolas Goaziou

[-- Attachment #1: Type: text/plain, Size: 632 bytes --]

Hi Michael,

I have applied your testing patch, thank you!

- Carsten

On 12.9.2013, at 22:52, Michael Brand <michael.ch.brand@gmail.com> wrote:

> Hi Carsten
> 
> On Thu, Sep 12, 2013 at 10:20 PM, Carsten Dominik
> <carsten.dominik@gmail.com> wrote:
>> thanks for the patch - I think we will go with Nicolas' patch for fixing org-insert-heading.  May I ask you to check if your tests still to the right thing, and resubmit the test patch?
>> 
>> Many thanks in advance.
> 
> Yes, merged patch attached. No changes in the test cases were necessary.
> 
> Michael
> <0001-Add-ERTs-for-org-meta-return.patch.txt>


[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2013-09-12 20:58 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-08  6:43 org-insert-heading rewritten from scratch Carsten Dominik
2013-08-08  7:41 ` Eric Abrahamsen
2013-08-31 14:00   ` Carsten Dominik
2013-08-31 14:34     ` Nicolas Goaziou
2013-09-01  6:13       ` Carsten Dominik
2013-09-01  8:19         ` Nicolas Goaziou
2013-09-01 12:04           ` Carsten Dominik
2013-09-01 12:25             ` Nicolas Goaziou
2013-09-03 13:16             ` Nicolas Goaziou
2013-09-03 13:25               ` Carsten Dominik
2013-09-03 13:33                 ` Carsten Dominik
2013-09-03 13:38                 ` Nicolas Goaziou
2013-09-03 13:47                   ` Carsten Dominik
2013-09-03 13:58                     ` Nicolas Goaziou
2013-09-03 14:04                       ` Carsten Dominik
2013-09-03 14:12                         ` Nicolas Goaziou
2013-09-07 10:50                         ` Nicolas Goaziou
2013-09-07 12:01                           ` Nicolas Goaziou
2013-09-01  2:38     ` Eric Abrahamsen
2013-09-01  3:11     ` Samuel Wales
2013-09-01  3:13       ` Samuel Wales
2013-09-09 16:42 ` Michael Brand
2013-09-09 19:10   ` Nicolas Goaziou
2013-09-09 20:37     ` Michael Brand
2013-09-12 20:20       ` Carsten Dominik
2013-09-12 20:52         ` Michael Brand
2013-09-12 20:58           ` Carsten Dominik

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