[PATCH 1/2] Fix narrowing for 1-line subtrees

[PATCH 1/2] Fix narrowing for 1-line subtrees

[Leo Vivier]

* lisp/org.el (org-narrow-to-subtree): Ensure newline at end of
  subtree.
  (org-tree-to-indirect-buffer): Ensure newline at end of
  subtree.
  (org-widen): Create wrapper for `widen' to handle end newline.

* lisp/org-capture.el (org-capture-finalize): Replace `widen' with
  `org-widen'.
  (org-capture-narrow): Ensure newline at end of subtree.

* lisp/org-keys.el (org-remap): Remap `widen' to `org-widen'.

There was a problem with narrowed 1-line subtrees which caused
clocking and scheduling commands to print their modifications outside
of the current narrowing, e.g. `org-clock-in'. This also prevented
some commands from working as expected,
e.g. `org-clock-out-when-done', since the clock-drawer isn't visible.

Previous commits have addressed this problem by patching those
commands to act on a widened buffer within a `save-restriction'
environment (cf. 8fc22d464, 503ede74b, and 6872088c7) but this does
not address the original problem, namely the modifications not being
visible in the narrowed buffer.

The problem is inherent to Emacs's narrowing.  In org-mode, the
narrowing commands use `org-end-of-subtree' to retrieve the
end-position of the region to be narrowed.  However, with a 1-line
subtree, `org-end-of-subtree' moves the point to the end of the line
which is before the position where clocking and scheduling commands
print their modifications, i.e. right below the headline.

To address the problem, we need to change the way we narrow and widen
buffers in org-mode:
- We patch the narrowing commands in org-mode to always add a newline
  at the end of subtrees (not only the 1-line subtrees).  This ensures
  that clocking and scheduling commands print their modifications
  within the narrowed buffer.
- We create a wrapper for `widen' within org-mode (called `org-widen')
  which deletes the newline added when we narrowed the buffer to the
  subtree.

However, for this solution to be complete, we need to ensure that the
newlines added by the narrowing commands cannot be deleted by the
user.
---
This is an implementation of the solution I've discussed in 'Bug:
org-clock commands spawn drawer outside of narrowing' on Fri, 15 Feb
2019 09:48:00 +0100.

I've tried it with `emacs -q' and with different numbers of
blank-lines between headings, and  I haven't encountered any problem so
far.  The code doesn't make any assumption about how many lines you
should have between your headings, which means that it shouldn't
interfere with `org-blank-before-new-entry'.

If there are more idiomatic ways to write some of the functions,
please let me know.

Thank you.

 lisp/org-capture.el | 12 +++++++++---
 lisp/org-keys.el    |  1 +
 lisp/org.el         | 26 +++++++++++++++++++++-----
 3 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/lisp/org-capture.el b/lisp/org-capture.el
index debf1808c..ff3134fb4 100644
--- a/lisp/org-capture.el
+++ b/lisp/org-capture.el
@@ -746,7 +746,7 @@ captured item after finalizing."
   (let ((abort-note nil))
     ;; Store the size of the capture buffer
     (org-capture-put :captured-entry-size (- (point-max) (point-min)))
-    (widen)
+    (org-widen)
     ;; Store the insertion point in the target buffer
     (org-capture-put :insertion-point (point))
 
@@ -1416,8 +1416,14 @@ Of course, if exact position has been required, just put it there."
 (defun org-capture-narrow (beg end)
   "Narrow, unless configuration says not to narrow."
   (unless (org-capture-get :unnarrowed)
-    (narrow-to-region beg end)
-    (goto-char beg)))
+    (goto-char beg)
+    (narrow-to-region
+     beg
+     (progn (org-end-of-subtree t t)
+            (when (and (org-at-heading-p) (not (eobp)))
+              (backward-char 1)
+              (insert "\n"))
+            (point)))))
 
 (defun org-capture-empty-lines-before (&optional n)
   "Set the correct number of empty lines before the insertion point.
diff --git a/lisp/org-keys.el b/lisp/org-keys.el
index d103957a9..90e8139b0 100644
--- a/lisp/org-keys.el
+++ b/lisp/org-keys.el
@@ -532,6 +532,7 @@ COMMANDS is a list of alternating OLDDEF NEWDEF command names."
 	   'delete-char            'org-delete-char
 	   'delete-backward-char   'org-delete-backward-char
 	   'kill-line              'org-kill-line
+	   'widen                  'org-widen
 	   'open-line              'org-open-line
 	   'yank                   'org-yank
 	   'comment-dwim           'org-comment-dwim
diff --git a/lisp/org.el b/lisp/org.el
index ebec2befa..3110f14ba 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -7391,7 +7391,9 @@ frame is not changed."
       (setq beg (point)
 	    heading (org-get-heading 'no-tags))
       (org-end-of-subtree t t)
-      (when (org-at-heading-p) (backward-char 1))
+      (when (and (org-at-heading-p) (not (eobp)))
+              (backward-char 1)
+              (insert "\n"))
       (setq end (point)))
     (when (and (buffer-live-p org-last-indirect-buffer)
 	       (not (eq org-indirect-buffer-display 'new-frame))
@@ -8381,16 +8383,21 @@ If yes, remember the marker and the distance to BEG."
     (move-marker (car x) (+ beg (cdr x))))
   (setq org-markers-to-move nil))
 
-(defun org-narrow-to-subtree ()
-  "Narrow buffer to the current subtree."
-  (interactive)
+(defun org-narrow-to-subtree (&optional newline)
+  "Narrow buffer to the current subtree.
+
+When called interactively, ensures that there’s a newline at the
+end of the narrowed tree."
+  (interactive "p")
   (save-excursion
     (save-match-data
       (org-with-limited-levels
        (narrow-to-region
 	(progn (org-back-to-heading t) (point))
 	(progn (org-end-of-subtree t t)
-	       (when (and (org-at-heading-p) (not (eobp))) (backward-char 1))
+	       (when (and (org-at-heading-p) (not (eobp)))
+                 (backward-char 1)
+                 (when newline (insert "\n")))
 	       (point)))))))
 
 (defun org-toggle-narrow-to-subtree ()
@@ -8410,6 +8417,15 @@ If yes, remember the marker and the distance to BEG."
 	(narrow-to-region (car blockp) (cdr blockp))
       (user-error "Not in a block"))))
 
+(defun org-widen ()
+  "Widen buffer."
+  (interactive)
+  (save-excursion
+    (goto-char (point-max))
+    (when (string-match-p "^\\s-*$" (thing-at-point 'line))
+      (delete-char -1)))
+  (widen))
+
 (defun org-clone-subtree-with-time-shift (n &optional shift)
   "Clone the task (subtree) at point N times.
 The clones will be inserted as siblings.
-- 
2.20.1

[PATCH 2/2] Prevent deletion of newline added by narrowing

[Leo Vivier]

* lisp/org.el (org-delete-backward-char): Prevent deletion of newline
  added by narrowing
  (org-delete-char): Prevent deletion of newline added by narrowing
  (org-kill-line): Prevent deletion of newline added by narrowing
  (org-kill-region): Create wrapper for `kill-region' to prevent
  deletion of newline added by narrowing

* lisp/org-keys.el (org-remap): Remap `kill-region' to
  `org-kill-region'

This ensures that the newline added by the narrowing commands cannot
be deleted by the user.

It does so by having every interactive deletion command check whether
it would delete the last newline of a narrowed buffer.  If it would,
the new command deletes whatever the original command normally would
but keep the last newline.  If the original command would have
resulted in a movement, e.g. `org-delete-backward-char', the new
command also moves the point as if the last newline had been deleted.
---
 lisp/org-keys.el |  1 +
 lisp/org.el      | 28 ++++++++++++++++++++++++----
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/lisp/org-keys.el b/lisp/org-keys.el
index 90e8139b0..26a3852b3 100644
--- a/lisp/org-keys.el
+++ b/lisp/org-keys.el
@@ -532,6 +532,7 @@ COMMANDS is a list of alternating OLDDEF NEWDEF command names."
 	   'delete-char            'org-delete-char
 	   'delete-backward-char   'org-delete-backward-char
 	   'kill-line              'org-kill-line
+	   'kill-region            'org-kill-region
 	   'widen                  'org-widen
 	   'open-line              'org-open-line
 	   'yank                   'org-yank
diff --git a/lisp/org.el b/lisp/org.el
index 3110f14ba..02130ab6a 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -18851,7 +18851,11 @@ because, in this case the deletion might narrow the column."
 	     (looking-at-p ".*?|")
 	     (org-at-table-p))
 	(progn (forward-char -1) (org-delete-char 1))
-      (backward-delete-char N)
+      (if (and (eobp)
+               (save-excursion (forward-char -1)
+                               (looking-at "\n")))
+          (forward-char -1)
+        (backward-delete-char N))
       (org-fix-tags-on-the-fly))))
 
 (defun org-delete-char (N)
@@ -18868,7 +18872,9 @@ because, in this case the deletion might narrow the column."
 	  (eq (char-after) ?|)
 	  (save-excursion (skip-chars-backward " \t") (bolp))
 	  (not (org-at-table-p)))
-      (delete-char N)
+      (unless (and (save-excursion (forward-char) (eobp))
+                    (looking-at "\n"))
+         (delete-char N))
       (org-fix-tags-on-the-fly))
      ((looking-at ".\\(.*?\\)|")
       (let* ((update? org-table-may-need-update)
@@ -22301,8 +22307,12 @@ depending on context."
       (user-error
        (substitute-command-keys
 	"`\\[org-kill-line]' aborted as it would kill a hidden subtree")))
-    (call-interactively
-     (if (bound-and-true-p visual-line-mode) 'kill-visual-line 'kill-line)))
+    (unless (and (looking-at-p "\n")
+		 (save-excursion
+		   (forward-char 1)
+		   (eobp)))
+      (call-interactively
+       (if (bound-and-true-p visual-line-mode) 'kill-visual-line 'kill-line))))
    ((org-match-line org-tag-line-re)
     (let ((end (save-excursion
 		 (goto-char (match-beginning 1))
@@ -22314,6 +22324,16 @@ depending on context."
     (org-align-tags))
    (t (kill-region (point) (line-end-position)))))
 
+(defun org-kill-region (beg end &optional region)
+  (interactive (list (mark) (point) 'region))
+  (kill-region
+   beg
+   end
+   region)
+  (save-excursion
+    (when (eobp)
+	(insert "\n"))))
+
 (defun org-yank (&optional arg)
   "Yank.  If the kill is a subtree, treat it specially.
 This command will look at the current kill and check if is a single
-- 
2.20.1

[PATCH] Fix other commands for exiting narrowing

[Leo Vivier]

* lisp/org.el (org-kill-buffer): Create a wrapper for kill-buffer to
  handle last newline deletion.
  (org-kill-buffer-and-window): Create a wrapper for
  kill-buffer-and-window to handle last newline deletion.

* lisp/org-keys.el (org-remap): Remap kill-buffer and
  kill-buffer-and-window to org wrappers.
---
I'd forgotten to patch the commands for exiting indirect buffers
spawned by `org-tree-to-indirect-buffer'.

This needs to be squashed with the first commit.

Sorry for the bother.
  
 lisp/org-keys.el |  2 ++
 lisp/org.el      | 21 +++++++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/lisp/org-keys.el b/lisp/org-keys.el
index 26a3852b3..0f4fd5b6d 100644
--- a/lisp/org-keys.el
+++ b/lisp/org-keys.el
@@ -533,6 +533,8 @@ COMMANDS is a list of alternating OLDDEF NEWDEF command names."
 	   'delete-backward-char   'org-delete-backward-char
 	   'kill-line              'org-kill-line
 	   'kill-region            'org-kill-region
+	   'kill-buffer            'org-kill-bufer
+	   'kill-buffer-and-window 'org-kill-buffer-and-window
 	   'widen                  'org-widen
 	   'open-line              'org-open-line
 	   'yank                   'org-yank
diff --git a/lisp/org.el b/lisp/org.el
index 02130ab6a..292807138 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -7442,6 +7442,27 @@ frame is not changed."
         (make-indirect-buffer buffer bname 'clone)
       (error (make-indirect-buffer buffer bname)))))
 
+(defun org-kill-buffer (&optional buffer-or-name)
+  "Kill the buffer specified by BUFFER-OR-NAME.
+The argument may be a buffer or the name of an existing buffer.
+Argument nil or omitted means kill the current buffer.  Return t if the
+buffer is actually killed, nil otherwise.
+
+Wrapper for org.  See `kill-buffer' for more info."
+  (interactive)
+  (when (buffer-base-buffer)
+    (org-widen))
+  (kill-buffer buffer-or-name))
+
+(defun org-kill-buffer-and-window ()
+  "Kill the current buffer and delete the selected window.
+
+Wrapper for org.  See `kill-buffer-and-window' for more info."
+  (interactive)
+  (when (buffer-base-buffer)
+    (org-widen))
+  (kill-buffer-and-window))
+
 (defun org-set-frame-title (title)
   "Set the title of the current frame to the string TITLE."
   (modify-frame-parameters (selected-frame) (list (cons 'name title))))
-- 
2.20.1

[PATCH] Fix other commands for exiting narrowing (2)

[Leo Vivier]

* lisp/org.el (org-toggle-narrow-to-subtree): Different interactive
  calls and use wrapper for widen
---
Same deal as the last email.  Sorry for only noticing those commands
now; I don't use them in my workflow.

The change in `interactive' is to ensure that only the interactive calls
to `org-narrow-to-subtree' produce the last newline.

 lisp/org.el | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index 292807138..ef86423e8 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -8421,12 +8421,12 @@ end of the narrowed tree."
                  (when newline (insert "\n")))
 	       (point)))))))
 
-(defun org-toggle-narrow-to-subtree ()
+(defun org-toggle-narrow-to-subtree (&optional newline)
   "Narrow to the subtree at point or widen a narrowed buffer."
-  (interactive)
+  (interactive "p")
   (if (buffer-narrowed-p)
-      (widen)
-    (org-narrow-to-subtree)))
+      (org-widen)
+    (org-narrow-to-subtree newline)))
 
 (defun org-narrow-to-block ()
   "Narrow buffer to the current block."
-- 
2.20.1

[PATCH] Fix problems

[Leo Vivier]

* lisp/org-capture.el (org-capture-narrow): Fix point position after
  narrowing.

* lisp/org-keys.el (org-remap): Remove remaps for `kill-buffer' and
  `kill-buffer-and-window'.

* lisp/org.el (org-tree-check-narrowing): Use `kill-buffer-hook'
  instead of wrappers for kill-region commands.
  (org-kill-region): Add docstring.

There was a problem in org-capture with templates which didn't specify
`%?'.  It was due to the position of the point upon exiting
`org-capture-narrow' which caused the `search-backward' and
`search-forward' at the end of `org-capture-place-entry' to potentially
act on region outside the viewport.

I've moved away from wrappers for `kill-buffer' and
`kill-buffer-and-window' in favour of a hook to `kill-buffer-hook'.
Problems would have been likely to arise with user-written commands
using `kill-buffer' instead of `org-kill-buffer' (it did for me).
Running `org-tree-check-narrowing' at `kill-buffer-hook' avoids this
problem and is a lot more convenient.

There's also a minor problem which I do not know if we can address.
When the user switches between an indirect buffer and the buffer which
spawned it, the last newline of the subtree isn't protected in the
spawning buffer.  Deleting that newline in the spawning buffer also
deletes it in the indirect buffer, thereby undermining all our efforts
to protect it.  However, if that's the only edge case we have to deal
with, I'd consider it a minor nuisance.
---
 lisp/org-capture.el | 14 +++++++-------
 lisp/org-keys.el    |  2 --
 lisp/org.el         | 40 +++++++++++++---------------------------
 3 files changed, 20 insertions(+), 36 deletions(-)

diff --git a/lisp/org-capture.el b/lisp/org-capture.el
index ff3134fb4..fbc601875 100644
--- a/lisp/org-capture.el
+++ b/lisp/org-capture.el
@@ -1416,14 +1416,14 @@ Of course, if exact position has been required, just put it there."
 (defun org-capture-narrow (beg end)
   "Narrow, unless configuration says not to narrow."
   (unless (org-capture-get :unnarrowed)
-    (goto-char beg)
     (narrow-to-region
-     beg
-     (progn (org-end-of-subtree t t)
-            (when (and (org-at-heading-p) (not (eobp)))
-              (backward-char 1)
-              (insert "\n"))
-            (point)))))
+     (goto-char beg)
+     (save-excursion
+       (org-end-of-subtree t t)
+       (when (and (org-at-heading-p) (not (eobp)))
+         (backward-char 1)
+         (insert "\n"))
+       (point)))))
 
 (defun org-capture-empty-lines-before (&optional n)
   "Set the correct number of empty lines before the insertion point.
diff --git a/lisp/org-keys.el b/lisp/org-keys.el
index 0f4fd5b6d..26a3852b3 100644
--- a/lisp/org-keys.el
+++ b/lisp/org-keys.el
@@ -533,8 +533,6 @@ COMMANDS is a list of alternating OLDDEF NEWDEF command names."
 	   'delete-backward-char   'org-delete-backward-char
 	   'kill-line              'org-kill-line
 	   'kill-region            'org-kill-region
-	   'kill-buffer            'org-kill-bufer
-	   'kill-buffer-and-window 'org-kill-buffer-and-window
 	   'widen                  'org-widen
 	   'open-line              'org-open-line
 	   'yank                   'org-yank
diff --git a/lisp/org.el b/lisp/org.el
index ef86423e8..7846a27b7 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -4415,6 +4415,13 @@ If yes, offer to stop it and to save the buffer with the changes."
   (when (org-match-line "^[ \t]*#\\+BEGIN:[ \t]+clocktable\\>")
     (org-clocktable-shift dir n)))
 
+(defun org-tree-check-narrowing ()
+  "Check if the current buffer is a narrowed indirect subtree.
+If yes, widen the buffer."
+  (when (and (derived-mode-p 'org-mode)
+	     (buffer-base-buffer))
+    (org-widen)))
+
 ;;;###autoload
 (defun org-clock-persistence-insinuate ()
   "Set up hooks for clock persistence."
@@ -5369,6 +5376,7 @@ The following commands are available:
   (add-hook 'before-change-functions 'org-before-change-function nil 'local)
   ;; Check for running clock before killing a buffer
   (add-hook 'kill-buffer-hook 'org-check-running-clock nil 'local)
+  (add-hook 'kill-buffer-hook 'org-tree-check-narrowing nil 'local)
   ;; Initialize macros templates.
   (org-macro-initialize-templates)
   ;; Initialize radio targets.
@@ -7442,27 +7450,6 @@ frame is not changed."
         (make-indirect-buffer buffer bname 'clone)
       (error (make-indirect-buffer buffer bname)))))
 
-(defun org-kill-buffer (&optional buffer-or-name)
-  "Kill the buffer specified by BUFFER-OR-NAME.
-The argument may be a buffer or the name of an existing buffer.
-Argument nil or omitted means kill the current buffer.  Return t if the
-buffer is actually killed, nil otherwise.
-
-Wrapper for org.  See `kill-buffer' for more info."
-  (interactive)
-  (when (buffer-base-buffer)
-    (org-widen))
-  (kill-buffer buffer-or-name))
-
-(defun org-kill-buffer-and-window ()
-  "Kill the current buffer and delete the selected window.
-
-Wrapper for org.  See `kill-buffer-and-window' for more info."
-  (interactive)
-  (when (buffer-base-buffer)
-    (org-widen))
-  (kill-buffer-and-window))
-
 (defun org-set-frame-title (title)
   "Set the title of the current frame to the string TITLE."
   (modify-frame-parameters (selected-frame) (list (cons 'name title))))
@@ -22346,14 +22333,13 @@ depending on context."
    (t (kill-region (point) (line-end-position)))))
 
 (defun org-kill-region (beg end &optional region)
+  "Kill (\"cut\") text between point and mark.
+
+Wrapper for org.  See `kill-region' for more info."
   (interactive (list (mark) (point) 'region))
-  (kill-region
-   beg
-   end
-   region)
+  (kill-region beg end region)
   (save-excursion
-    (when (eobp)
-	(insert "\n"))))
+    (when (eobp) (insert "\n"))))
 
 (defun org-yank (&optional arg)
   "Yank.  If the kill is a subtree, treat it specially.
-- 
2.20.1

Re: [PATCH 1/2] Fix narrowing for 1-line subtrees

[Nicolas Goaziou]

Hello,j

Leo Vivier <leo.vivier@gmail.com> writes:

> The problem is inherent to Emacs's narrowing.  In org-mode, the
> narrowing commands use `org-end-of-subtree' to retrieve the
> end-position of the region to be narrowed.  However, with a 1-line
> subtree, `org-end-of-subtree' moves the point to the end of the line
> which is before the position where clocking and scheduling commands
> print their modifications, i.e. right below the headline.
>
> To address the problem, we need to change the way we narrow and widen
> buffers in org-mode:

Thank you for your effort. 

However, I don't think this is going into a good direction. Narrowing
should probably be the same everywhere in Emacs, including Org mode.

> - We patch the narrowing commands in org-mode to always add a newline
>   at the end of subtrees (not only the 1-line subtrees).  This ensures
>   that clocking and scheduling commands print their modifications
>   within the narrowed buffer.
> - We create a wrapper for `widen' within org-mode (called `org-widen')
>   which deletes the newline added when we narrowed the buffer to the
>   subtree.
>
> However, for this solution to be complete, we need to ensure that the
> newlines added by the narrowing commands cannot be deleted by the
> user.

IMO, this is very hackish. You imply that narrowing is only done
interactively, but this is not always the case. In non-interactive
usage, messing with newline characters could be a bad idea.

In a nutshell, I suggest not going this route. Sorry for not having been
clear about it earlier.

Regards,

-- 
Nicolas Goaziou

Re: [PATCH 1/2] Fix narrowing for 1-line subtrees

[Leo Vivier]

Hello,

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> However, I don't think this is going into a good direction. Narrowing
> should probably be the same everywhere in Emacs, including Org mode.

I understand.  The rationale behind this idea was that it would only
modify the way narrowing works for subtrees just as AUCTeX's
`LaTeX-narrow-to-environment' works for environments.  That's why I
didn't think it was a problem.

> IMO, this is very hackish. You imply that narrowing is only done
> interactively, but this is not always the case. In non-interactive
> usage, messing with newline characters could be a bad idea.

I don't think if I've implied so, and I've written the code so that it
wouldn't change non-interactive calls:

--------------------------------[START]--------------------------------
                              <--------------->
(defun org-narrow-to-subtree (&optional newline)
  "Narrow buffer to the current subtree.

When called interactively, ensures that there’s a newline at the
end of the narrowed tree."
->(interactive "p")
  (save-excursion
    (save-match-data
      (org-with-limited-levels
       (narrow-to-region
	(progn (org-back-to-heading t) (point))
	(progn (org-end-of-subtree t t)
	       (when (and (org-at-heading-p) (not (eobp)))
                 (backward-char 1)
->               (when newline (insert "\n")))
	       (point)))))))
---------------------------------[END]---------------------------------

I've tried to touch as few commands as possible, and I haven't seen any
weird behaviours so far.  But I understand your wariness.

> In a nutshell, I suggest not going this route. Sorry for not having been
> clear about it earlier.

You don't need to apologise, I went down this route because it was an
interesting project to address my problems with 1-line subtrees.  As
I've said in the commit message, even if we address the problem of other
commands not seeing content added outside of the narrowing, we're still
left with the other problem which is that the user is not seeing those
changes.  I wasn't content with this solution, and that's what prompted
me to write this.

Could I suggest you try out this patch with its latest commit (sent on
Mon, 18 Feb 2019 18:18:47 +0100) and gauge whether it affects your
workflow negatively?  I know you’ve only said that this patch was
heading in a wrong direction rather than saying that all hell would
break loose upon applying the patch, but the later commits have tried to
iron out the kinks, and that might quell your wariness.

At any rate, thank you for time.

HTH.

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

[PATCH] Fix narrowing for 1-line subtrees (squashed)

[Leo Vivier]

This is a squashed version of all the commits I’ve done on that
branch to make it easier to apply.
---
 lisp/org-capture.el | 12 ++++++--
 lisp/org-keys.el    |  2 ++
 lisp/org.el         | 69 ++++++++++++++++++++++++++++++++++++---------
 3 files changed, 67 insertions(+), 16 deletions(-)

diff --git a/lisp/org-capture.el b/lisp/org-capture.el
index debf1808c..fbc601875 100644
--- a/lisp/org-capture.el
+++ b/lisp/org-capture.el
@@ -746,7 +746,7 @@ captured item after finalizing."
   (let ((abort-note nil))
     ;; Store the size of the capture buffer
     (org-capture-put :captured-entry-size (- (point-max) (point-min)))
-    (widen)
+    (org-widen)
     ;; Store the insertion point in the target buffer
     (org-capture-put :insertion-point (point))
 
@@ -1416,8 +1416,14 @@ Of course, if exact position has been required, just put it there."
 (defun org-capture-narrow (beg end)
   "Narrow, unless configuration says not to narrow."
   (unless (org-capture-get :unnarrowed)
-    (narrow-to-region beg end)
-    (goto-char beg)))
+    (narrow-to-region
+     (goto-char beg)
+     (save-excursion
+       (org-end-of-subtree t t)
+       (when (and (org-at-heading-p) (not (eobp)))
+         (backward-char 1)
+         (insert "\n"))
+       (point)))))
 
 (defun org-capture-empty-lines-before (&optional n)
   "Set the correct number of empty lines before the insertion point.
diff --git a/lisp/org-keys.el b/lisp/org-keys.el
index d103957a9..26a3852b3 100644
--- a/lisp/org-keys.el
+++ b/lisp/org-keys.el
@@ -532,6 +532,8 @@ COMMANDS is a list of alternating OLDDEF NEWDEF command names."
 	   'delete-char            'org-delete-char
 	   'delete-backward-char   'org-delete-backward-char
 	   'kill-line              'org-kill-line
+	   'kill-region            'org-kill-region
+	   'widen                  'org-widen
 	   'open-line              'org-open-line
 	   'yank                   'org-yank
 	   'comment-dwim           'org-comment-dwim
diff --git a/lisp/org.el b/lisp/org.el
index ef6e40ca9..1f662a01a 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -4415,6 +4415,13 @@ If yes, offer to stop it and to save the buffer with the changes."
   (when (org-match-line "^[ \t]*#\\+BEGIN:[ \t]+clocktable\\>")
     (org-clocktable-shift dir n)))
 
+(defun org-tree-check-narrowing ()
+  "Check if the current buffer is a narrowed indirect subtree.
+If yes, widen the buffer."
+  (when (and (derived-mode-p 'org-mode)
+	     (buffer-base-buffer))
+    (org-widen)))
+
 ;;;###autoload
 (defun org-clock-persistence-insinuate ()
   "Set up hooks for clock persistence."
@@ -5369,6 +5376,7 @@ The following commands are available:
   (add-hook 'before-change-functions 'org-before-change-function nil 'local)
   ;; Check for running clock before killing a buffer
   (add-hook 'kill-buffer-hook 'org-check-running-clock nil 'local)
+  (add-hook 'kill-buffer-hook 'org-tree-check-narrowing nil 'local)
   ;; Initialize macros templates.
   (org-macro-initialize-templates)
   ;; Initialize radio targets.
@@ -7392,7 +7400,9 @@ frame is not changed."
       (setq beg (point)
 	    heading (org-get-heading 'no-tags))
       (org-end-of-subtree t t)
-      (when (org-at-heading-p) (backward-char 1))
+      (when (and (org-at-heading-p) (not (eobp)))
+              (backward-char 1)
+              (insert "\n"))
       (setq end (point)))
     (when (and (buffer-live-p org-last-indirect-buffer)
 	       (not (eq org-indirect-buffer-display 'new-frame))
@@ -8382,24 +8392,29 @@ If yes, remember the marker and the distance to BEG."
     (move-marker (car x) (+ beg (cdr x))))
   (setq org-markers-to-move nil))
 
-(defun org-narrow-to-subtree ()
-  "Narrow buffer to the current subtree."
-  (interactive)
+(defun org-narrow-to-subtree (&optional newline)
+  "Narrow buffer to the current subtree.
+
+When called interactively, ensures that there’s a newline at the
+end of the narrowed tree."
+  (interactive "p")
   (save-excursion
     (save-match-data
       (org-with-limited-levels
        (narrow-to-region
 	(progn (org-back-to-heading t) (point))
 	(progn (org-end-of-subtree t t)
-	       (when (and (org-at-heading-p) (not (eobp))) (backward-char 1))
+	       (when (and (org-at-heading-p) (not (eobp)))
+                 (backward-char 1)
+                 (when newline (insert "\n")))
 	       (point)))))))
 
-(defun org-toggle-narrow-to-subtree ()
+(defun org-toggle-narrow-to-subtree (&optional newline)
   "Narrow to the subtree at point or widen a narrowed buffer."
-  (interactive)
+  (interactive "p")
   (if (buffer-narrowed-p)
-      (widen)
-    (org-narrow-to-subtree)))
+      (org-widen)
+    (org-narrow-to-subtree newline)))
 
 (defun org-narrow-to-block ()
   "Narrow buffer to the current block."
@@ -8411,6 +8426,15 @@ If yes, remember the marker and the distance to BEG."
 	(narrow-to-region (car blockp) (cdr blockp))
       (user-error "Not in a block"))))
 
+(defun org-widen ()
+  "Widen buffer."
+  (interactive)
+  (save-excursion
+    (goto-char (point-max))
+    (when (string-match-p "^\\s-*$" (thing-at-point 'line))
+      (delete-char -1)))
+  (widen))
+
 (defun org-clone-subtree-with-time-shift (n &optional shift)
   "Clone the task (subtree) at point N times.
 The clones will be inserted as siblings.
@@ -18836,7 +18860,11 @@ because, in this case the deletion might narrow the column."
 	     (looking-at-p ".*?|")
 	     (org-at-table-p))
 	(progn (forward-char -1) (org-delete-char 1))
-      (backward-delete-char N)
+      (if (and (eobp)
+               (save-excursion (forward-char -1)
+                               (looking-at "\n")))
+          (forward-char -1)
+        (backward-delete-char N))
       (org-fix-tags-on-the-fly))))
 
 (defun org-delete-char (N)
@@ -18853,7 +18881,9 @@ because, in this case the deletion might narrow the column."
 	  (eq (char-after) ?|)
 	  (save-excursion (skip-chars-backward " \t") (bolp))
 	  (not (org-at-table-p)))
-      (delete-char N)
+      (unless (and (save-excursion (forward-char) (eobp))
+                    (looking-at "\n"))
+         (delete-char N))
       (org-fix-tags-on-the-fly))
      ((looking-at ".\\(.*?\\)|")
       (let* ((update? org-table-may-need-update)
@@ -22286,8 +22316,12 @@ depending on context."
       (user-error
        (substitute-command-keys
 	"`\\[org-kill-line]' aborted as it would kill a hidden subtree")))
-    (call-interactively
-     (if (bound-and-true-p visual-line-mode) 'kill-visual-line 'kill-line)))
+    (unless (and (looking-at-p "\n")
+		 (save-excursion
+		   (forward-char 1)
+		   (eobp)))
+      (call-interactively
+       (if (bound-and-true-p visual-line-mode) 'kill-visual-line 'kill-line))))
    ((org-match-line org-tag-line-re)
     (let ((end (save-excursion
 		 (goto-char (match-beginning 1))
@@ -22299,6 +22333,15 @@ depending on context."
     (org-align-tags))
    (t (kill-region (point) (line-end-position)))))
 
+(defun org-kill-region (beg end &optional region)
+  "Kill (\"cut\") text between point and mark.
+
+Wrapper for org.  See `kill-region' for more info."
+  (interactive (list (mark) (point) 'region))
+  (kill-region beg end region)
+  (save-excursion
+    (when (eobp) (insert "\n"))))
+
 (defun org-yank (&optional arg)
   "Yank.  If the kill is a subtree, treat it specially.
 This command will look at the current kill and check if is a single
-- 
2.20.1

Re: [PATCH 1/2] Fix narrowing for 1-line subtrees

[Nicolas Goaziou]

Hello,

Leo Vivier <leo.vivier@gmail.com> writes:

> I understand.  The rationale behind this idea was that it would only
> modify the way narrowing works for subtrees just as AUCTeX's
> `LaTeX-narrow-to-environment' works for environments.  That's why I
> didn't think it was a problem.

It doesn't work the way `LaTeX-narrow-to-environment' works. In
particular, AUCTeX's function /does not modify the buffer/. This is
a big no-no, really.

> You don't need to apologise, I went down this route because it was an
> interesting project to address my problems with 1-line subtrees.  As
> I've said in the commit message, even if we address the problem of other
> commands not seeing content added outside of the narrowing, we're still
> left with the other problem which is that the user is not seeing those
> changes.  I wasn't content with this solution, and that's what prompted
> me to write this.

I suggest to not use narrowing, then. Maybe try editing remotely
a subtree, similar to what is done for footnotes. I have the feeling
this would have its own set of issues, too.

> Could I suggest you try out this patch with its latest commit (sent on
> Mon, 18 Feb 2019 18:18:47 +0100) and gauge whether it affects your
> workflow negatively?

It is not about my workflow. I don't use 1-line subtrees. But anything
related to narrowing or widening should not alter the buffer, per
design. I may sound stubborn, but I don't think this is a way to handle
the problem.


Regards,

-- 
Nicolas Goaziou

Re: [PATCH 1/2] Fix narrowing for 1-line subtrees

[Leo Vivier]

Hello again,

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Hello,
>
> It doesn't work the way `LaTeX-narrow-to-environment' works. In
> particular, AUCTeX's function /does not modify the buffer/. This is
> a big no-no, really.

I see your point, and I understand why it would be strange for narrowing
commands to modify the buffer.  I’d built this patch under three
assumptions:

1. We should only change the interactive behaviour of
  `org-narrow-to-subtree' so as to not disturb other commands/functions.
2. When called interactively, as long as our wrapper for `widen' cancels
   out what's been added by `org-narrow-to-subtree', changing the buffer
   is acceptable.
3. If the buffer were to be closed between `org-narrow-to-subtree' and
   our wrapper for `widen', the only thing you'd have is a spurious
   newline.  This wouldn't be a big deal because some commands in org
   already do that in a narrowed context [1].

That said, I completely understand your reticence and you've made me
understand that my solution was more 'hackish' than I intended it to be.

> I suggest to not use narrowing, then. Maybe try editing remotely
> a subtree, similar to what is done for footnotes. I have the feeling
> this would have its own set of issues, too.

I thought about other options before heading into this.  One of them was
to yank the subtrees to a temporary buffer to edit them and hook onto
the saving commands to update the corresponding buffer accordingly.  In
retrospect, that seems a lot more 'hackish'.  Maybe we could salvage
some of the patch for `org-capture' since it's different from narrowing,
but I've got a better idea.

> It is not about my workflow. I don't use 1-line subtrees. But anything
> related to narrowing or widening should not alter the buffer, per
> design. I may sound stubborn, but I don't think this is a way to handle
> the problem.

I'd like to suggest a middle ground which I think would be more
acceptable.  You've asked me in a previous exchange to make a list of
the commands which didn't work as expected when the buffer was narrowed
to a 1-line subtree [2].  Would it be possible to patch those commands
so that they conditionally refresh the narrowing of the buffer if the
information they add would be spawned *outside* of the restriction?

The rationale behind it is that, in Emacs, commands trying to act on
regions outside the current restriction throw an error.  Therefore, in
the context of 1-line subtrees, we could justify that conditional
behaviour by saying that it prevents your command from working outside
of the current restriction.

I was pleased to see that property-adding functions didn't behave badly
with 1-line subtrees.  Maybe we could investigate those commands and
patch their behaviour onto the problematic ones?

If that sounds good to you, I could work on it and submit another patch.

Thank you.

HTH.


Footnotes:
[1] As a quick aside, here's an example for 3. where X represents `point':
--------------------------------[START]--------------------------------
\|   * Tree
\|   |X1|2|
---------------------------------[END]---------------------------------
When narrowed (or at the end of a buffer), if you press <TAB>:
- `point' moves to '2'.
- Table is realigned.
- Newline is added at the end of the table.

[2] We've already addressed `org-clock-out-when-done', but I've found
another one.  Although adding scheduling/deadline information works
within a 1-line subtree (the information isn't visible, but it's there
in the widened buffer), you cannot remove them from within that
environment.


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

Re: [PATCH 1/2] Fix narrowing for 1-line subtrees

[Leo Vivier]

Hello again,

Leo Vivier <leo.vivier+org@gmail.com> writes:

> I was pleased to see that property-adding functions didn't behave badly
> with 1-line subtrees.  Maybe we could investigate those commands and
> patch their behaviour onto the problematic ones?
>
> If that sounds good to you, I could work on it and submit another patch.

I’ve investigated the problem, and I’ve stumbled upon something
interesting.

I’ve started by looking at the differences between `org-set-property'
and `org-schedule' which respectively led me to
`org-insert-property-drawer' and `org-add-planing-info'.  The
interesting difference is in the way they handle newline insertion:


`org-insert-property-drawer':
--------------------------------[START]--------------------------------
...
(insert "\n:PROPERTIES:\n:END:")
...
---------------------------------[END]---------------------------------


`org-add-planing-info':
--------------------------------[START]--------------------------------
...                      
(insert-before-markers "\n")                    ; Inside a cond
...
(insert (cl-case what                           ; Inside a later cond
                   (closed org-closed-string)
                   ...
                   )
		 " ")
...
---------------------------------[END]---------------------------------


By adapting the `org-add-planing-info' to insert the newline with the
scheduling information, I could get it to insert its text *inside* the
narrowing with a 1-line subtree.

I'm providing a patch at the end of this email, but it's rough around
the edges.  Notably, I didn't have time to make it work with the
condition tree, which means that re-scheduling as well as adding a
deadline on top of a scheduled time results in spurious newlines.

Correct me if I'm wrong, but I believe we'd be departing the 'hackish'
territory with that solution, which would be great.

Here's the patch:
--------------------------------[START]--------------------------------
Move newline insertion

---
 lisp/org.el | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index ef6e40ca9..6c43d9b25 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -13046,18 +13046,19 @@ WHAT entry will also be removed."
 		    (unless (= (skip-chars-backward " \t" p) 0)
 		      (delete-region (point) (line-end-position)))))))
 	     ((not what) (throw 'exit nil)) ; Nothing to do.
-	     (t (insert-before-markers "\n")
-		(backward-char 1)
+	     (t (backward-char 1)
 		(when org-adapt-indentation
 		  (indent-to-column (1+ (org-outline-level))))))
        (when what
 	 ;; Insert planning keyword.
-	 (insert (cl-case what
-		   (closed org-closed-string)
-		   (deadline org-deadline-string)
-		   (scheduled org-scheduled-string)
-		   (otherwise (error "Invalid planning type: %s" what)))
+	 (insert "\n"
+		 (cl-case what
+                   (closed org-closed-string)
+                   (deadline org-deadline-string)
+                   (scheduled org-scheduled-string)
+                   (otherwise (error "Invalid planning type: %s" what)))
 		 " ")
+	 (end-of-line)
 	 ;; Insert associated timestamp.
 	 (let ((ts (org-insert-time-stamp
 		    time
-- 
2.20.1
---------------------------------[END]---------------------------------

HTH.

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

Re: [PATCH 1/2] Fix narrowing for 1-line subtrees

[Leo Vivier]

Sorry, the patch I've submitted wasn't right: it had part of another
test I was running, hence the `end-of-line'.

Here's the proper version:
--------------------------------[START]--------------------------------
lisp/org.el | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index ef6e40ca9..4514407e7 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -13046,17 +13046,17 @@ WHAT entry will also be removed."
 		    (unless (= (skip-chars-backward " \t" p) 0)
 		      (delete-region (point) (line-end-position)))))))
 	     ((not what) (throw 'exit nil)) ; Nothing to do.
-	     (t (insert-before-markers "\n")
-		(backward-char 1)
+	     (t (backward-char 1)
 		(when org-adapt-indentation
 		  (indent-to-column (1+ (org-outline-level))))))
        (when what
 	 ;; Insert planning keyword.
-	 (insert (cl-case what
-		   (closed org-closed-string)
-		   (deadline org-deadline-string)
-		   (scheduled org-scheduled-string)
-		   (otherwise (error "Invalid planning type: %s" what)))
+	 (insert "\n"
+		 (cl-case what
+                   (closed org-closed-string)
+                   (deadline org-deadline-string)
+                   (scheduled org-scheduled-string)
+                   (otherwise (error "Invalid planning type: %s" what)))
 		 " ")
 	 ;; Insert associated timestamp.
 	 (let ((ts (org-insert-time-stamp
-- 
2.20.1
---------------------------------[END]---------------------------------

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

Re: [PATCH 1/2] Fix narrowing for 1-line subtrees

[Nicolas Goaziou]

Hello,

Leo Vivier <leo.vivier+org@gmail.com> writes:

> Here's the proper version:

Thank you. It looks good. Could you write a few tests about that?

Regards,

-- 
Nicolas Goaziou

Re: [PATCH 1/2] Fix narrowing for 1-line subtrees

[Leo Vivier]

Hello,

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Thank you. It looks good. Could you write a few tests about that?

I’ve never done it, but it should be pretty easy to figure out how to
write them with all the macros.  I’ll look into it.

I’ll also continue the work on the patch to figure out how to handle the
condition tree for `org-add-planing-info'.

I’ll update you when I’ve made progress.

Thanks.

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

[PATCH] Fix narrowed 1-line subtrees

[Leo Vivier]

* lisp/org.el (org-add-planning-info): Ensure insertion in current
  restriction.
  (org-remove-timestamp-with-keyword): Respect ambiguous newline when
  narrowed to 1-line-subtree.
  (org-remove-empty-drawer-at): Respect ambiguous newline when
  narrowed to 1-line subtree.
  (org-entry-delete): Respect ambiguous newline when narrowed to
  1-line subtree.

* lisp/org-clock.el (org-clock-find-position): Ensure clock-drawer
  insertion in current restriction.

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.

Typical problem:
-----------------------------------------------------------------------
* Tree 1
:PROPERTIES:
:TEST: t
:END:
* Tree 2
-----------------------------------------------------------------------
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:
-----------------------------------------------------------------------
* Tree 1* Tree 2
-----------------------------------------------------------------------

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

The problem is due to two things:
- The way that narrowing works in Emacs, notably how restrictions are
  restored after `save-restriction'.
- The ambiguous newline between the end of a 1-line subtree and a
  following subtree.

The solution is to stop the problematic commands from interacting with
the ambiguous newline in order to preserve the narrowed region's
`point-max'.  This is done by inserting or removing newlines from
the top of a heading rather than its bottom.

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

We delete the following one:
-----------------------------------------------------------------------
* Tree 1{
:PROPERTIES:
:TEST: t
:END:}
* Tree 2
-----------------------------------------------------------------------
---
Please see my reply to this message for a detailed account of the
problem and the solution.

 lisp/org-clock.el |  9 +++++----
 lisp/org.el       | 16 +++++++++-------
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/lisp/org-clock.el b/lisp/org-clock.el
index b20158df6..5624af32a 100644
--- a/lisp/org-clock.el
+++ b/lisp/org-clock.el
@@ -1508,12 +1508,13 @@ line and position cursor in that line."
 	  (when (and org-clock-into-drawer
 		     (or (not (wholenump org-clock-into-drawer))
 			 (< org-clock-into-drawer 2)))
-	    (let ((beg (point)))
-	      (insert ":" drawer ":\n:END:\n")
+	    (let ((beg (1- (point))))
+	      (forward-char -1)
+	      (insert "\n:" drawer ":\n:END:")
 	      (org-indent-region beg (point))
 	      (org-flag-region
-	       (line-end-position -1) (1- (point)) t 'org-hide-drawer)
-	      (forward-line -1))))
+	       (line-end-position 0) (point) t 'org-hide-drawer)
+	      (beginning-of-line))))
 	 ;; When a clock drawer needs to be created because of the
 	 ;; number of clock items or simply if it is missing, collect
 	 ;; all clocks in the section and wrap them within the drawer.
diff --git a/lisp/org.el b/lisp/org.el
index ef6e40ca9..ae9494672 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -12937,7 +12937,7 @@ nil."
   "Remove all time stamps with KEYWORD in the current entry."
   (let ((re (concat "\\<" (regexp-quote keyword) " +<[^>\n]+>[ \t]*"))
 	beg)
-    (save-excursion
+    (org-with-wide-buffer
       (org-back-to-heading t)
       (setq beg (point))
       (outline-next-heading)
@@ -12949,7 +12949,8 @@ nil."
 	  (when (string-match "^[ \t]*$" (buffer-substring
 					  (point-at-bol) (point-at-eol)))
 	    (delete-region (point-at-bol)
-			   (min (point-max) (1+ (point-at-eol))))))))))
+			   (point-at-eol)
+			   (delete-char -1))))))))
 
 (defvar org-time-was-given) ; dynamically scoped parameter
 (defvar org-end-time-was-given) ; dynamically scoped parameter
@@ -13046,8 +13047,8 @@ WHAT entry will also be removed."
 		    (unless (= (skip-chars-backward " \t" p) 0)
 		      (delete-region (point) (line-end-position)))))))
 	     ((not what) (throw 'exit nil)) ; Nothing to do.
-	     (t (insert-before-markers "\n")
-		(backward-char 1)
+	     (t (backward-char 1)
+		(insert "\n")
 		(when org-adapt-indentation
 		  (indent-to-column (1+ (org-outline-level))))))
        (when what
@@ -13306,8 +13307,8 @@ POS may also be a marker."
 	 (delete-region (org-element-property :begin drawer)
 			(progn (goto-char (org-element-property :end drawer))
 			       (skip-chars-backward " \r\t\n")
-			       (forward-line)
-			       (point))))))))
+			       (point)))
+	 (delete-char -1))))))
 
 (defvar org-ts-type nil)
 (defun org-sparse-tree (&optional arg type)
@@ -15222,7 +15223,8 @@ non-nil when a property was removed."
 	 ;; If drawer is empty, remove it altogether.
 	 (when (= begin end)
 	   (delete-region (line-beginning-position 0)
-			  (line-beginning-position 2)))
+			  (point-at-eol))
+	   (delete-char -1))
 	 ;; Return non-nil if some property was removed.
 	 (prog1 (/= end origin) (set-marker end nil))))
       (_ nil))))
-- 
2.20.1

Re: [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

[PATCH] Fix spaces with `org-remove-timestamp-with-keyword'

[Leo Vivier]

* lisp/org.el (org-remove-timestamp-with-keyword): Fix space deletion
  between timestamps

When an entry had a CLOSED, a DEADLINE and a SCHEDULED timestamps,
removing the middle one caused the space between the 1st and 3rd to be
removed as well.  Checking whether we’re at the end of the line before
deleting the space fixes it.
---
Here’s a little unrelated patch for an issue I’ve stumbled upon whilst
testing the previous patch.

 lisp/org.el | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lisp/org.el b/lisp/org.el
index ae9494672..4c3c3cd78 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -12944,6 +12944,7 @@ nil."
       (while (re-search-backward re beg t)
 	(replace-match "")
 	(if (and (string-match "\\S-" (buffer-substring (point-at-bol) (point)))
+		 (eolp)
 		 (equal (char-before) ?\ ))
 	    (backward-delete-char 1)
 	  (when (string-match "^[ \t]*$" (buffer-substring
-- 
2.20.1

Re: [PATCH] Fix narrowed 1-line subtrees

[Leo Vivier]

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

Hello,

I've found and fixed three new functions which didn’t behave properly
when the buffer was restricted to a subtree:
* lisp/org.el (org-log-beginning): Fix drawer creation.
* lisp/org.el (org-store-log-note): Fix drawer-less logging.
* lisp/org-capture.el (org-clock-in): Fix drawer-less clocking.

You'll find those three patches at the bottom alongside another with all
the patches until now squashed together (except the patch for
`org-remove-timestamp-with-keyword' which wasn't related).

HTH.

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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-log-beginning-Fix-drawer-creation.patch --]
[-- Type: text/x-patch, Size: 1278 bytes --]

From 745e106406a5f5b296bbd9dbda9f9dbd965a2e30 Mon Sep 17 00:00:00 2001
From: Leo Vivier <leo.vivier+org@gmail.com>
Date: Fri, 22 Feb 2019 18:03:24 +0100
Subject: [PATCH 1/3] org-log-beginning: Fix drawer creation

* lisp/org.el (org-log-beginning): Ensure insertion in current
  restriction.

This commit ensures that the log-drawer for state-changes and notes is
created within the current restriction.
---
 lisp/org.el | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index 4c3c3cd78..f22f8b807 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -13118,12 +13118,13 @@ narrowing."
 	   ;; No drawer found.  Create one, if permitted.
 	   (when create
 	     (unless (bolp) (insert "\n"))
-	     (let ((beg (point)))
-	       (insert ":" drawer ":\n:END:\n")
+	     (let ((beg (1- (point))))
+	       (forward-char -1)
+	       (insert "\n:" drawer ":\n:END:")
 	       (org-indent-region beg (point))
 	       (org-flag-region
-		(line-end-position -1) (1- (point)) t 'org-hide-drawer))
-	     (end-of-line -1)))))
+		(line-end-position 0) (point) t 'org-hide-drawer))
+	     (end-of-line 0)))))
       (t
        (org-end-of-meta-data org-log-state-notes-insert-after-drawers)
        (skip-chars-forward " \t\n")
-- 
2.20.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-org-store-log-note-Fix-drawer-less-logging.patch --]
[-- Type: text/x-patch, Size: 1120 bytes --]

From c94c86fdac09a933337267c29f7e3d4dcf5c3398 Mon Sep 17 00:00:00 2001
From: Leo Vivier <leo.vivier+org@gmail.com>
Date: Fri, 22 Feb 2019 18:17:35 +0100
Subject: [PATCH 2/3] org-store-log-note: Fix drawer-less logging

* lisp/org.el (org-log-beginning): Ensure insertion in current
  restriction.

This commit ensures that drawer-less state-changes and notes are
created within the current restriction.
---
 lisp/org.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/org.el b/lisp/org.el
index f22f8b807..27cd2bbd7 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -13263,7 +13263,7 @@ EXTRA is additional text that will be inserted into the notes buffer."
 	 ;; Note associated to a clock is to be located right after
 	 ;; the clock.  Do not move point.
 	 (unless (eq org-log-note-purpose 'clock-out)
-	   (goto-char (org-log-beginning t)))
+	   (goto-char (1- (org-log-beginning t))))
 	 ;; Make sure point is at the beginning of an empty line.
 	 (cond ((not (bolp)) (let ((inhibit-read-only t)) (insert "\n")))
 	       ((looking-at "[ \t]*\\S-") (save-excursion (insert "\n"))))
-- 
2.20.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0003-org-clock-in-Fix-drawer-less-clocking.patch --]
[-- Type: text/x-patch, Size: 1156 bytes --]

From 2fc86ae438725e5f0656c8966eaa4935e0203ee4 Mon Sep 17 00:00:00 2001
From: Leo Vivier <leo.vivier+org@gmail.com>
Date: Fri, 22 Feb 2019 18:23:40 +0100
Subject: [PATCH 3/3] org-clock-in: Fix drawer-less clocking

* lisp/org-clock.el (org-clock-in): Ensure insertion in current
  restriction.

This commit ensures that drawer-less clock-lines are created within
the current restriction.
---
 lisp/org-clock.el | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lisp/org-clock.el b/lisp/org-clock.el
index 5624af32a..5c9b0a1cf 100644
--- a/lisp/org-clock.el
+++ b/lisp/org-clock.el
@@ -1292,6 +1292,7 @@ the default behavior."
 		(org-todo org-clock-in-switch-to-state)))
 	 (setq org-clock-heading (org-clock--mode-line-heading))
 	 (org-clock-find-position org-clock-in-resume)
+	 (forward-char -1)
 	 (cond
 	  ((and org-clock-in-resume
 		(looking-at
@@ -1315,8 +1316,8 @@ the default behavior."
 	   (sit-for 2)
 	   (throw 'abort nil))
 	  (t
-	   (insert-before-markers "\n")
-	   (backward-char 1)
+	   (insert "\n")
+	   (org-indent-line)
 	   (org-indent-line)
 	   (when (and (save-excursion
 			(end-of-line 0)
-- 
2.20.1


[-- Attachment #5: 0001-Fix-spaces-with-org-remove-timestamp-with-keyword.patch --]
[-- Type: text/x-patch, Size: 1152 bytes --]

From bb5a7feee1684cf47f1e8a29805c442c8ae64c37 Mon Sep 17 00:00:00 2001
From: Leo Vivier <leo.vivier+org@gmail.com>
Date: Thu, 21 Feb 2019 12:44:26 +0100
Subject: [PATCH] Fix spaces with `org-remove-timestamp-with-keyword'
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* lisp/org.el (org-remove-timestamp-with-keyword): Fix space deletion
  between timestamps

When an entry had a CLOSED, a DEADLINE and a SCHEDULED timestamps,
removing the middle one caused the space between the 1st and 3rd to be
removed as well.  Checking whether we’re at the end of the line before
deleting the space fixes it.
---
 lisp/org.el | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lisp/org.el b/lisp/org.el
index ef6e40ca9..b8e378e73 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -12944,6 +12944,7 @@ nil."
       (while (re-search-backward re beg t)
 	(replace-match "")
 	(if (and (string-match "\\S-" (buffer-substring (point-at-bol) (point)))
+		 (eolp)
 		 (equal (char-before) ?\ ))
 	    (backward-delete-char 1)
 	  (when (string-match "^[ \t]*$" (buffer-substring
-- 
2.20.1

Re: [PATCH] Fix narrowed 1-line subtrees

[Leo Vivier]

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

Hello again,

Leo Vivier <leo.vivier+org@gmail.com> writes:

> You'll find those three patches at the bottom alongside another with all
> the patches until now squashed together (except the patch for
> `org-remove-timestamp-with-keyword' which wasn't related).

I ended up sending the wrong patch, i.e. the one for
`org-remove-timestamp-with-keyword'.  You'll find the squashed patch
below.

Sorry for that.

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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-narrowed-1-line-subtrees-squashed.patch --]
[-- Type: text/x-patch, Size: 7773 bytes --]

From c00c911f06ba059b61d8246f25e679f06a8f8491 Mon Sep 17 00:00:00 2001
From: Leo Vivier <leo.vivier+org@gmail.com>
Date: Thu, 21 Feb 2019 00:16:27 +0100
Subject: [PATCH] Fix narrowed 1-line subtrees (squashed)

* lisp/org.el (org-add-planning-info): Ensure insertion in current
  restriction.
  (org-remove-timestamp-with-keyword): Respect ambiguous newline when
  narrowed to 1-line-subtree.
  (org-remove-empty-drawer-at): Respect ambiguous newline when
  narrowed to 1-line subtree.
  (org-entry-delete): Respect ambiguous newline when narrowed to
  1-line subtree.
  (org-log-beginning): Ensure insertion in current restriction.
  (org-store-log-note): Ensure insertion in current restriction.

* lisp/org-clock.el (org-clock-find-position): Ensure clock-drawer
  insertion in current restriction.
  (org-clock-in): Ensure insertion in current
  restriction.

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.

Typical problem:
-----------------------------------------------------------------------
* Tree 1
:PROPERTIES:
:TEST: t
:END:
* Tree 2
-----------------------------------------------------------------------
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:
-----------------------------------------------------------------------
* Tree 1* Tree 2
-----------------------------------------------------------------------

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

The problem is due to two things:
- The way that narrowing works in Emacs, notably how restrictions are
  restored after `save-restriction'.
- The ambiguous newline between the end of a 1-line subtree and a
  following subtree.

The solution is to stop the problematic commands from interacting with
the ambiguous newline in order to preserve the narrowed region's
`point-max'.  This is done by inserting or removing newlines from
the top of a heading rather than its bottom.

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

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

org-log-beginning: Fix drawer creation

* lisp/org.el

This commit ensures that the log-drawer for state-changes and notes is
created within the current restriction.

org-store-log-note: Fix drawer-less logging

* lisp/org.el (org-log-beginning):
---
 lisp/org-clock.el | 14 ++++++++------
 lisp/org.el       | 27 +++++++++++++++------------
 2 files changed, 23 insertions(+), 18 deletions(-)

diff --git a/lisp/org-clock.el b/lisp/org-clock.el
index b20158df6..5c9b0a1cf 100644
--- a/lisp/org-clock.el
+++ b/lisp/org-clock.el
@@ -1292,6 +1292,7 @@ the default behavior."
 		(org-todo org-clock-in-switch-to-state)))
 	 (setq org-clock-heading (org-clock--mode-line-heading))
 	 (org-clock-find-position org-clock-in-resume)
+	 (forward-char -1)
 	 (cond
 	  ((and org-clock-in-resume
 		(looking-at
@@ -1315,8 +1316,8 @@ the default behavior."
 	   (sit-for 2)
 	   (throw 'abort nil))
 	  (t
-	   (insert-before-markers "\n")
-	   (backward-char 1)
+	   (insert "\n")
+	   (org-indent-line)
 	   (org-indent-line)
 	   (when (and (save-excursion
 			(end-of-line 0)
@@ -1508,12 +1509,13 @@ line and position cursor in that line."
 	  (when (and org-clock-into-drawer
 		     (or (not (wholenump org-clock-into-drawer))
 			 (< org-clock-into-drawer 2)))
-	    (let ((beg (point)))
-	      (insert ":" drawer ":\n:END:\n")
+	    (let ((beg (1- (point))))
+	      (forward-char -1)
+	      (insert "\n:" drawer ":\n:END:")
 	      (org-indent-region beg (point))
 	      (org-flag-region
-	       (line-end-position -1) (1- (point)) t 'org-hide-drawer)
-	      (forward-line -1))))
+	       (line-end-position 0) (point) t 'org-hide-drawer)
+	      (beginning-of-line))))
 	 ;; When a clock drawer needs to be created because of the
 	 ;; number of clock items or simply if it is missing, collect
 	 ;; all clocks in the section and wrap them within the drawer.
diff --git a/lisp/org.el b/lisp/org.el
index b8e378e73..27cd2bbd7 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -12937,7 +12937,7 @@ nil."
   "Remove all time stamps with KEYWORD in the current entry."
   (let ((re (concat "\\<" (regexp-quote keyword) " +<[^>\n]+>[ \t]*"))
 	beg)
-    (save-excursion
+    (org-with-wide-buffer
       (org-back-to-heading t)
       (setq beg (point))
       (outline-next-heading)
@@ -12950,7 +12950,8 @@ nil."
 	  (when (string-match "^[ \t]*$" (buffer-substring
 					  (point-at-bol) (point-at-eol)))
 	    (delete-region (point-at-bol)
-			   (min (point-max) (1+ (point-at-eol))))))))))
+			   (point-at-eol)
+			   (delete-char -1))))))))
 
 (defvar org-time-was-given) ; dynamically scoped parameter
 (defvar org-end-time-was-given) ; dynamically scoped parameter
@@ -13047,8 +13048,8 @@ WHAT entry will also be removed."
 		    (unless (= (skip-chars-backward " \t" p) 0)
 		      (delete-region (point) (line-end-position)))))))
 	     ((not what) (throw 'exit nil)) ; Nothing to do.
-	     (t (insert-before-markers "\n")
-		(backward-char 1)
+	     (t (backward-char 1)
+		(insert "\n")
 		(when org-adapt-indentation
 		  (indent-to-column (1+ (org-outline-level))))))
        (when what
@@ -13117,12 +13118,13 @@ narrowing."
 	   ;; No drawer found.  Create one, if permitted.
 	   (when create
 	     (unless (bolp) (insert "\n"))
-	     (let ((beg (point)))
-	       (insert ":" drawer ":\n:END:\n")
+	     (let ((beg (1- (point))))
+	       (forward-char -1)
+	       (insert "\n:" drawer ":\n:END:")
 	       (org-indent-region beg (point))
 	       (org-flag-region
-		(line-end-position -1) (1- (point)) t 'org-hide-drawer))
-	     (end-of-line -1)))))
+		(line-end-position 0) (point) t 'org-hide-drawer))
+	     (end-of-line 0)))))
       (t
        (org-end-of-meta-data org-log-state-notes-insert-after-drawers)
        (skip-chars-forward " \t\n")
@@ -13261,7 +13263,7 @@ EXTRA is additional text that will be inserted into the notes buffer."
 	 ;; Note associated to a clock is to be located right after
 	 ;; the clock.  Do not move point.
 	 (unless (eq org-log-note-purpose 'clock-out)
-	   (goto-char (org-log-beginning t)))
+	   (goto-char (1- (org-log-beginning t))))
 	 ;; Make sure point is at the beginning of an empty line.
 	 (cond ((not (bolp)) (let ((inhibit-read-only t)) (insert "\n")))
 	       ((looking-at "[ \t]*\\S-") (save-excursion (insert "\n"))))
@@ -13307,8 +13309,8 @@ POS may also be a marker."
 	 (delete-region (org-element-property :begin drawer)
 			(progn (goto-char (org-element-property :end drawer))
 			       (skip-chars-backward " \r\t\n")
-			       (forward-line)
-			       (point))))))))
+			       (point)))
+	 (delete-char -1))))))
 
 (defvar org-ts-type nil)
 (defun org-sparse-tree (&optional arg type)
@@ -15223,7 +15225,8 @@ non-nil when a property was removed."
 	 ;; If drawer is empty, remove it altogether.
 	 (when (= begin end)
 	   (delete-region (line-beginning-position 0)
-			  (line-beginning-position 2)))
+			  (point-at-eol))
+	   (delete-char -1))
 	 ;; Return non-nil if some property was removed.
 	 (prog1 (/= end origin) (set-marker end nil))))
       (_ nil))))
-- 
2.20.1

Re: [PATCH] Fix narrowed 1-line subtrees

[Samuel Wales]

i have not been able to follow this conversation, but is it possible
that /all/ of this complexity is due to outline-mode's dubious choice
of not considering the final newline in an entry to be part of an
entry?

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

What is misopathy?
https://thekafkapandemic.blogspot.com/2013/10/why-some-diseases-are-wronged.html

The disease DOES progress. MANY people have died from it. And ANYBODY
can get it at any time.

Re: [PATCH] Fix narrowed 1-line subtrees

[Leo Vivier]

Hello,

Samuel Wales <samologist@gmail.com> writes:

> i have not been able to follow this conversation, but is it possible
> that /all/ of this complexity is due to outline-mode's dubious choice
> of not considering the final newline in an entry to be part of an
> entry?

I don't know much about outline-mode, but I doubt it would be the case.
In my email sent on Thu, 21 Feb 2019 16:41:43 +0100, I've investigated
the problem, and one of my conclusions was the following:

--------------------------------[START]--------------------------------
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'.
---------------------------------[END]---------------------------------

So, if we kept the newline, we'd put the user at risk of breaking the
following subtree.  An option could be to protect that newline by
modifying our deletion commands, but after having done that in a
previous implementation, it'd bring its fair share of complexity.

From my point of view, I do not see it as 'complex', but rather as a
different way from doing what we'd been doing so far.  Most of the
functions are only *slightly* modified to preserve the ambiguous
newline.

HTH.

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

Re: [PATCH] Fix narrowed 1-line subtrees

[Samuel Wales]

ok, i can't follow this so thanks for the comments.  i'll trust that
you know what you are doing.

however, fyi, long ago, i discovered various bugs in this newline
area, most of which were trackable to outline mode's decision to
define an entry as ending before its final newline.

most users and many writers of code are likely to mark a region by
going to bol, not eol.  they don't likely think the region should end
before the final newline, in most cases.

but org calls some outline function or two that grabs or narrows to or
marks a region that is shorter by 1 char than imo it should be.

i reported one bug, which joined two lines [including headers], that
had remained unfixed for many years.

this is probably because noticing certain types of data corruption /at
the time of corruption/ can sometimes be tricky.  thus, linking it to
user behavior or org code changes can be tricky and the bug report
would be like "um, i have no mwe but...".

in this particular case, when you did find joined lines, it was likely
to be long after the corruption.  [low s/n.]

the problem was that when you sorted headers in a narrowed region, it
joined lines.  the region was off by one because the outline function
is [imo] off by one.

it would not surprise me if long-term users checked their files now
for joined lines, such as headers, they'd find old corruption.


On 2/22/19, Leo Vivier <leo.vivier+org@gmail.com> wrote:
> Hello,
>
> Samuel Wales <samologist@gmail.com> writes:
>
>> i have not been able to follow this conversation, but is it possible
>> that /all/ of this complexity is due to outline-mode's dubious choice
>> of not considering the final newline in an entry to be part of an
>> entry?
>
> I don't know much about outline-mode, but I doubt it would be the case.
> In my email sent on Thu, 21 Feb 2019 16:41:43 +0100, I've investigated
> the problem, and one of my conclusions was the following:
>
> --------------------------------[START]--------------------------------
> 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'.
> ---------------------------------[END]---------------------------------
>
> So, if we kept the newline, we'd put the user at risk of breaking the
> following subtree.  An option could be to protect that newline by
> modifying our deletion commands, but after having done that in a
> previous implementation, it'd bring its fair share of complexity.
>
> From my point of view, I do not see it as 'complex', but rather as a
> different way from doing what we'd been doing so far.  Most of the
> functions are only *slightly* modified to preserve the ambiguous
> newline.
>
> HTH.
>
> Best,
> --
> Leo Vivier
> English Studies & General Linguistics
> Master Student, English Department
> Université Rennes 2
>


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

What is misopathy?
https://thekafkapandemic.blogspot.com/2013/10/why-some-diseases-are-wronged.html

The disease DOES progress. MANY people have died from it. And ANYBODY
can get it at any time.

Re: [PATCH] Fix narrowed 1-line subtrees

[Leo Vivier]

Hello,

Samuel Wales <samologist@gmail.com> writes:

> most users and many writers of code are likely to mark a region by
> going to bol, not eol.  they don't likely think the region should end
> before the final newline, in most cases.

I don't know about most users, but that's what I do.  It's easier to
mark the beginning of a region and then `forward-line' your way to the
end of the region.

> but org calls some outline function or two that grabs or narrows to or
> marks a region that is shorter by 1 char than imo it should be.

I'm not sure.  It might be a case of Stockholm syndrome, but I've found
that not including the final newline in the narrowed subtree holds some
semantic value, especially when you like to keep your .org files tight
with only 1 newline between two headings, i.e. `*Tree 1^J*Tree 2'.

When `(org-end-of-subtree t)' lands you somewhere where the hl-line does
not extend to the right fringe, it means that you (or a misbehaving
commands) haven't introduced any spurious newline.

> i reported one bug, which joined two lines [including headers], that
> had remained unfixed for many years.
>
> this is probably because noticing certain types of data corruption /at
> the time of corruption/ can sometimes be tricky.  thus, linking it to
> user behavior or org code changes can be tricky and the bug report
> would be like "um, i have no mwe but...".

Thank you for your insight.  I think this is something we could address
with an arsenal of tests.  The idea would be to run in a narrowed
subtree all the commands which would add or remove content below it.
Then, we widen the buffer and check whether the following tree has been
corrupted.  I'll get on it as soon as I can.

> in this particular case, when you did find joined lines, it was likely
> to be long after the corruption.  [low s/n.]
>
> the problem was that when you sorted headers in a narrowed region, it
> joined lines.  the region was off by one because the outline function
> is [imo] off by one.
>
> it would not surprise me if long-term users checked their files now
> for joined lines, such as headers, they'd find old corruption.

Maybe we could add a function to `org-lint' which would throw a warning
when a regex like "\\(\\sw\\|\\s.\\)\\(\\* .*$\\)" matches something.

With the following structure:
--------------------------------[START]--------------------------------
* Test 1* Corrupted 1

* Test 2
With text in the body.* Corrupted 2

* Test 3 * Not corrupted
---------------------------------[END]---------------------------------
The regex correctly matches `Corrupted 1' and `Corrupted 2', but we'd
obviously need to find a way to make it safer.  But as it stands, it
could be used as a way to address past corruptions.

So, to recap:
1. We address *potential corruptions* by adding more tests in order to
   catch them before they get a chance to corrupt anything.
2. We address *past corruptions* by adding a function to `org-lint'
   which catches corrupted subtrees and throw a warning.

HTH.

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