Re: [O] [PATCH] Fix narrowed 1-line subtrees

[Leo Vivier]

Hello,

Here’s a detailed account of the problem and solution of the patch
I’ve just sent.

I don’t have the time to write the tests today, but I’ll get on it as
soon as I can.


-----------------------------------------------------------------------


This patch addresses multiple issues occuring when running commands on
a 1-line subtree when the buffer is narrowed to it.  A 1-line subtree
is a subtree only containing a heading and a newline at the end.

The problem is due to the way narrowing works in Emacs.  It requires a
region defined by two bounds on which to anchor the narrowing.  The
bounds respectively become the `point-min' and `point-max of the
narrowed buffer.

As the content within the narrowed region evolves, `point-max' is
pushed or shrinked to accommodate the modifications to the content.
Since a position within a buffer in Emacs is defined as the number of
characters between the top of the file (whose value is 1) and `point',
that means that everything after `point-max' evolves in unisson with
the narrowed buffer.

For example, in an org-mode buffer narrowed to a subtree, adding a
newline at the end of a heading adds 1 character to the buffer
which then pushes `point-max' *and* everything after it in the widened
buffer by 1.

The problem occurs when the bounds of the region to narrow are
ambiguous, as is the case with 1-line subtrees.  When you narrow an
org-mode buffer to a 1-line subtree, the end of the line becomes
`point-max'.  Remembering our definition of a 1-line subtree above,
you might wonder why we're not including the newline, but the reason
is simple: that newline might also belong to another subtree.

Going back to our example, if narrowing to a 1-line subtree included
that last newline, we could delete it inside our narrowed buffer.  If
that newline was also the beginning of a new subtree, the subtree
would not be functional anymore, since we'd end up with something like
this: `*Subtree 1* Subtree 2'.


-----------------------------------------------------------------------
Example:
--------------------------------[START]--------------------------------
* Tree 1
:PROPERTIES:
:TEST: t
:END:
* Tree 2
---------------------------------[END]---------------------------------
With point on `Tree 1', run the following:
(progn
  (org-narrow-to-subtree)
  (org-delete-property "TEST")
  (org-back-to-heading)
  (end-of-line)
  (delete-char 1)
  (widen))

Result:
--------------------------------[START]--------------------------------
* Tree 1* Tree 2
---------------------------------[END]---------------------------------

Observation:
The newline between the two headings has been removed despite the fact
that it wasn't in the buffer restriction.
-----------------------------------------------------------------------


This ambiguous newline causes a lot of unexpected behaviours with
commands inserting or removing content, e.g. clocking, scheduling as
well as manipulating deadlines, properties, etc.

Some of those commands act on a widened buffer which prevents them
from inadvertently deleting that newline.  That's the case for
clocking in a task, since it adds `CLOCK:' lines below the heading at
point.

However, because those commands act on a widened buffer, they do not
have access to the narrowed buffer's `point-max'.  The consequence is
that, when the restriction of the buffer is restored after
`save-restriction', the narrowing function sees that the content
between `point-min' and `point-max' hasn't changed (there's still a
newline at the end) and restores the region as if nothing had happened.
The command worked, but there's no way to see it in the narrowed
buffer.

Another example of an unexpected behaviour with commands acting on a
widened buffer is when the command creates a 1-line subtree.  That's
the case when removing a :PROPERTIES: drawer.  When the command
removes the content *and* the last newline, upon restoring the
restriction, `point-max' is seen to have shrunk, and becomes the first
character which hasn't changed, which is the newline after the
heading.  The problem is that this is the ambiguous newline we
discussed above, and that deleting it could break the following
subtree if there was any.

The solution to this problem is to ensure that those commands never
act beyond the `point-max' of the narrowed buffer even when working in
the widened buffer.

As an example, when clocking in a task, rather than inserting a
newline *after* the last char which isn't part of the heading,
i.e. the ambiguous newline, we insert it after the last unambiguous
character.  This works because when narrowing, as we saw,
`point-max' *is* that unambiguous character, and adding characters
before it simply pushes `point-max' by as many characters as you've
inserted, and this is tracked by `save-restriction'.

This happens because `save-restriction' adds a special property
to *all* the characters within the current restriction, not only
`point-min' and `point-max'.  Upon restoring the previous restriction,
`save-restriction' looks for those special characters and try to
include them all inside the new restriction.  Practically, this is
done by looking for the first and last characters with that special
property and using them as the new `point-min' and `point-max'.

This last bit is important to understand why the second example with
the :PROPERTIES: drawer didn't behave properly.  The original command
deletes the drawer from the ambiguous newline to the bottom of the
heading, which means that the newline at the end of the heading isn't
touched.  When `save-restriction' attempts to resume the previous
narrowing, since the former `point-max' has been deleted (it was the
`:' at the end of the :PROPERTIES: drawer), it looks for the first
special character. But the problem is that this first character is the
bottom of the heading, and that it has now become ambiguous.

The solution is the same: we do not touch the ambiguous newline.
Instead, we delete the newline at the end of the heading so that upon
restoring the restriction, it becomes the last special character.

Visually, instead of deleting the following bracketed region...
--------------------------------[START]--------------------------------
* Tree 1
{:PROPERTIES:
:TEST: t
:END:
}* Tree 2
---------------------------------[END]---------------------------------

We delete the following one:
--------------------------------[START]--------------------------------
* Tree 1{
:PROPERTIES:
:TEST: t
:END:}
* Tree 2
---------------------------------[END]---------------------------------

It's likely that I haven't addressed all the commands that do not play
well with the ambiguous newlines.  However, the solutions I've
presented here should be enough to address them.


-----------------------------------------------------------------------


HTH

Best,
--
Leo Vivier
English Studies & General Linguistics
Master Student, English Department
Université Rennes 2