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

2019-02-23 Thread Leo Vivier
Hello,

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



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

2019-02-22 Thread 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  wrote:
> Hello,
>
> Samuel Wales  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: 

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: [O] [PATCH] Fix narrowed 1-line subtrees

2019-02-22 Thread Leo Vivier
Hello,

Samuel Wales  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: [O] [PATCH] Fix narrowed 1-line subtrees

2019-02-22 Thread 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: 

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: [O] [PATCH] Fix narrowed 1-line subtrees

2019-02-22 Thread Leo Vivier
Hello again,

Leo Vivier  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
>From c00c911f06ba059b61d8246f25e679f06a8f8491 Mon Sep 17 00:00:00 2001
From: Leo Vivier 
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 

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

2019-02-22 Thread Leo Vivier
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
>From 745e106406a5f5b296bbd9dbda9f9dbd965a2e30 Mon Sep 17 00:00:00 2001
From: Leo Vivier 
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

>From c94c86fdac09a97267c29f7e3d4dcf5c3398 Mon Sep 17 00:00:00 2001
From: Leo Vivier 
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

>From 2fc86ae438725e5f0656c8966eaa4935e0203ee4 Mon Sep 17 00:00:00 2001
From: Leo Vivier 
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

>From bb5a7feee1684cf47f1e8a29805c442c8ae64c37 Mon Sep 17 00:00:00 2001
From: Leo Vivier 
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 

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

2019-02-21 Thread 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 

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

2019-02-21 Thread 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+