Catching invisible edits: problem understanding doc

Catching invisible edits: problem understanding doc

[Alain.Cochard]

Hello.  The documentation (version 9.6) says:

   2.2.3 Catching invisible edits
   ------------------------------

   Sometimes you may inadvertently edit an invisible part of the
   buffer and be confused on what has been edited and how to undo the
   mistake.  Setting ‘org-fold-catch-invisible-edits’ to non-‘nil’
   helps preventing this.


I visit a new file.  I type

   * foo

then
   
   <RET>

then

   bar

Then I move the cursor one line up and press

   <TAB>

So now the headline is folded and I see

   * foo...

In my understanding, 'bar' is now in an invisible part of the buffer.
But whether org-fold-catch-invisible-edits is set to 'nil, 'error',
'show', 'show-and-error' or nothing (i.e., 'smart' by default in 9.6),
if then I do

   M-x undo

I visually see no change, but I can observe by unfolding the headline
that 'bar' has disappeared.  In my understanding of the documentation
above and of the docstring for org-fold-catch-invisible-edits, this
should only happen with 'nil'.

What am I understanding incorrectly?  Thank you.

Tests performed with 'emacs -Q -l .emacs.git' with '.emacs.git' being 

   (add-to-list 'load-path "~/Org/Coch-git/org-mode/lisp")
   (custom-set-variables
    '(org-fold-catch-invisible-edits 'nil)
    ;; or 'error or 'show or 'show-and-error (or nothing)
    )

giving:

   Org mode version 9.6.1 (release_9.6.1-239-g739ccf @
   /home/cochard/Org/Coch-git/org-mode/lisp/)

PS: a very minor incidental point: after 'bar' has disappeared, the
headline is then void, so I tend to say that the ellipsis should
immediately disappear as well.


-- 
EOST (École et Observatoire des Sciences de la Terre) 
ITE (Institut Terre & Environnement) | alain.cochard@unistra.fr
5 rue René Descartes   [bureau 110]  | Phone: +33 (0)3 68 85 50 44 
F-67084 Strasbourg Cedex, France     | [ slot available for rent ]

Should we extend org-catch-invisible-edits to more interactive commands? (was: Catching invisible edits: problem understanding doc)

[Ihor Radchenko]

Alain.Cochard@unistra.fr writes:

>    M-x undo
>
> I visually see no change, but I can observe by unfolding the headline
> that 'bar' has disappeared.  In my understanding of the documentation
> above and of the docstring for org-fold-catch-invisible-edits, this
> should only happen with 'nil'.
>
> What am I understanding incorrectly?  Thank you.

Only a handful of interactive commands support invisible edit checks. In
particular: self-insert-command (typing), deleting char
forward/backward, and `org-meta-return'.

I guess we may instead provide a defcustom and hook the check into
`pre-command-hook'. Would it be of interest?

> PS: a very minor incidental point: after 'bar' has disappeared, the
> headline is then void, so I tend to say that the ellipsis should
> immediately disappear as well.

The headline still contains newlines.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: Should we extend org-catch-invisible-edits to more interactive commands? (was: Catching invisible edits: problem understanding doc)

[Alain.Cochard]

Ihor Radchenko writes on Fri 10 Feb 2023 09:56:

 > Only a handful of interactive commands support invisible edit
 > checks. In particular: self-insert-command (typing), deleting char
 > forward/backward, and `org-meta-return'.
 > 
 > I guess we may instead provide a defcustom and hook the check into
 > `pre-command-hook'. Would it be of interest?

Thank you for the feedback.  Let me add that the issue I raised was
not at all a theoretical/abstract one for me.  I constantly use 'undo'
to see where in the buffer I was working (then, I insert a space, do
'undo undo', and can resume what I was doing).  So obviously it would
be of a huge interest for me if org-catch-invisible-edits worked in
that case...

At any rate, shouldn't "2.2.3 Catching invisible edits" be a little
bit more specific about what kind of invisible edits are concerned?
Or perhaps just a warning that it is not all of them.



 > > PS: a very minor incidental point: after 'bar' has disappeared, the
 > > headline is then void, so I tend to say that the ellipsis should
 > > immediately disappear as well.
 > 
 > The headline still contains newlines.

Right.  Thanks.

-- 
EOST (École et Observatoire des Sciences de la Terre) 
ITE (Institut Terre & Environnement) | alain.cochard@unistra.fr
5 rue René Descartes   [bureau 110]  | Phone: +33 (0)3 68 85 50 44 
F-67084 Strasbourg Cedex, France     | [ slot available for rent ]

Re: Should we extend org-catch-invisible-edits to more interactive commands? (was: Catching invisible edits: problem understanding doc)

[Ihor Radchenko]

Alain.Cochard@unistra.fr writes:

> At any rate, shouldn't "2.2.3 Catching invisible edits" be a little
> bit more specific about what kind of invisible edits are concerned?
> Or perhaps just a warning that it is not all of them.

We can indeed at such warning, but it will probably be not very helpful.
Only typing, deleting char forward/backward and meta-return are
considering invisible edits now.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: Should we extend org-catch-invisible-edits to more interactive commands? (was: Catching invisible edits: problem understanding doc)

[Alain.Cochard]

Ihor Radchenko writes on Sat 11 Feb 2023 18:22:

 > We can indeed at such warning, but it will probably be not very
 > helpful.

I don't understand this.  And isn't it better to have a more accurate
manual anyway?

Apart from that, since the default value in 9.6 is 'smart' ('nil' in
9.5), I wonder if the sentence

   "Setting ‘org-fold-catch-invisible-edits’ to non-‘nil’ helps
   preventing this."

is still the most appropriate.

Also, this is another instance where 'C-h v' with the cursor on
'org-fold-catch-invisible-edits' does not offer it right away in the
minibuffer.  I am surprised since it does so in 9.5 (with
'org-catch-invisible-edit').

 > Only typing, deleting char forward/backward and meta-return are
 > considering invisible edits now.

Thanks.  It is helpful to me to know that.

-- 
EOST (École et Observatoire des Sciences de la Terre) 
ITE (Institut Terre & Environnement) | alain.cochard@unistra.fr
5 rue René Descartes   [bureau 110]  | Phone: +33 (0)3 68 85 50 44 
F-67084 Strasbourg Cedex, France     | [ slot available for rent ]

[PATCH] Allow customizing commands affected by `org-fold-catch-invisible-edits' (was: Should we extend org-catch-invisible-edits to more interactive commands? (was: Catching invisible edits: problem understanding doc))

[Ihor Radchenko]

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

Alain.Cochard@unistra.fr writes:

> Ihor Radchenko writes on Sat 11 Feb 2023 18:22:
>
>  > We can indeed at such warning, but it will probably be not very
>  > helpful.
>
> I don't understand this.  And isn't it better to have a more accurate
> manual anyway?

Sure. But I want to implement more generic feature.
See the attached patch.

> Also, this is another instance where 'C-h v' with the cursor on
> 'org-fold-catch-invisible-edits' does not offer it right away in the
> minibuffer.  I am surprised since it does so in 9.5 (with
> 'org-catch-invisible-edit').

The variable has been moved to a different file.
C-h v not working is an Emacs bug.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-fold-Allow-customizing-commands-where-invisible-.patch --]
[-- Type: text/x-patch, Size: 8228 bytes --]

From 653005e1b383b4c7ad9086fc0b7e40d262dab744 Mon Sep 17 00:00:00 2001
Message-Id: <653005e1b383b4c7ad9086fc0b7e40d262dab744.1676215378.git.yantar92@posteo.net>
From: Ihor Radchenko <yantar92@posteo.net>
Date: Sun, 12 Feb 2023 18:13:36 +0300
Subject: [PATCH] org-fold: Allow customizing commands where invisible edits
 are checked

* lisp/org-fold.el (org-fold-catch-invisible-edits-commands): New
custom option.
(org-fold-catch-invisible-edits): Mention the new custom option in the
docstring.
(org-fold-check-before-invisible-edit-maybe): New function checking
if edits are safe for `this-command'.
(org-fold--advice-edit-commands): New function advising the functions
with `org-fold-check-before-invisible-edit-maybe'.
* lisp/org.el (org-mode): Advice functions on Org startup.
(org-self-insert-command):
(org-delete-backward-char):
(org-delete-char):
(org-meta-return): Do not call `org-fold-check-before-invisible-edit'
and rely on the new advise mechanism instead.
* etc/ORG-NEWS (Commands affected by ~org-fold-catch-invisible-edits~
can now be customized): Announce the change.
* doc/org-manual.org (Catching invisible edits): Mention new
customization.
---
 doc/org-manual.org | 12 +++++++++---
 etc/ORG-NEWS       | 12 ++++++++++++
 lisp/org-fold.el   | 45 ++++++++++++++++++++++++++++++++++++++++++++-
 lisp/org.el        |  6 ++----
 4 files changed, 67 insertions(+), 8 deletions(-)

diff --git a/doc/org-manual.org b/doc/org-manual.org
index 2d38cf76c..2b836a026 100644
--- a/doc/org-manual.org
+++ b/doc/org-manual.org
@@ -622,11 +622,17 @@ *** Catching invisible edits
 #+cindex: edits, catching invisible
 
 #+vindex: org-fold-catch-invisible-edits
+#+vindex: org-fold-catch-invisible-edits-commands
 Sometimes you may inadvertently edit an invisible part of the buffer
 and be confused on what has been edited and how to undo the mistake.
-Setting ~org-fold-catch-invisible-edits~ to non-~nil~ helps preventing
-this.  See the docstring of this option on how Org should catch
-invisible edits and process them.
+By default, Org prevents such edits for a limited set of user
+commands.  Users can control which commands are affected by
+customizing ~org-fold-catch-invisible-edits-commands~.
+
+The strategy used to decide if a given edit is dangerous is controlled
+by ~org-fold-catch-invisible-edits~.  See the docstring of this option
+on the available strategies.  Set the option to ~nil~ to disable
+catching invisible edits completely.
 
 ** Motion
 :PROPERTIES:
diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 87ecd77cd..b85c46758 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -24,6 +24,18 @@ consider [[https://gitlab.com/jackkamm/ob-python-mode-mode][ob-python-mode-mode]
 has been ported to.
 
 ** New and changed options
+*** Commands affected by ~org-fold-catch-invisible-edits~ can now be customized
+
+New user option ~org-fold-catch-invisible-edits-commands~ controls
+which commands trigger checking for invisible edits.
+
+The full list of affected commands is:
+- ~org-self-insert-command~
+- ~org-delete-backward-char~
+- ~org-delete-char~
+- ~org-meta-return~
+- ~org-return~ (not checked in earlier Org versions)
+
 *** New escape in ~org-beamer-environments-extra~ for labels in Beamer export
 The escape =%l= in ~org-beamer-environments-extra~ inserts the label
 obtained from ~org-beamer--get-label~.  This is added to the default
diff --git a/lisp/org-fold.el b/lisp/org-fold.el
index 1b7ca22b0..3d86eb802 100644
--- a/lisp/org-fold.el
+++ b/lisp/org-fold.el
@@ -189,7 +189,10 @@ (defcustom org-fold-catch-invisible-edits 'smart
                  Never delete a previously invisible character or add in the
                  middle or right after an invisible region.  Basically, this
                  allows insertion and backward-delete right before ellipses.
-                 FIXME: maybe in this case we should not even show?"
+                 FIXME: maybe in this case we should not even show?
+
+This variable only affects commands listed in
+`org-fold-catch-invisible-edits-commands'."
   :group 'org-edit-structure
   :version "24.1"
   :type '(choice
@@ -199,6 +202,33 @@ (defcustom org-fold-catch-invisible-edits 'smart
 	  (const :tag "Show invisible part and do the edit" show)
 	  (const :tag "Be smart and do the right thing" smart)))
 
+(defcustom org-fold-catch-invisible-edits-commands
+  ;; We do not add non-Org commands here by default to avoid advising
+  ;; globally.  See `org-fold--advice-edit-commands'.
+  '((org-self-insert-command . insert)
+    (org-delete-backward-char . delete-backward)
+    (org-delete-char . delete)
+    (org-meta-return . insert)
+    (org-return . insert))
+  "Alist of commands where Org checks for invisible edits.
+Each element is (COMMAND . KIND), where COMMAND is symbol representing
+command as stored in `this-command' and KIND is symbol `insert',
+symbol `delete', or symbol `delete-backward'.
+
+The checks are performed around `point'.
+
+This variable must be set before loading Org in order to take effect.
+
+Also, see `org-fold-catch-invisible-edits'."
+  :group 'org-edit-structure
+  :package-version '("Org" . "9.7")
+  :type '(alist
+          :key-type symbol
+          :value-type (choice
+                       (const insert)
+                       (const delete)
+                       (const delete-backward))))
+
 ;;; Core functionality
 
 ;;; API
@@ -901,6 +931,19 @@ (defun org-fold-check-before-invisible-edit (kind)
 	    ;; Don't do the edit, make the user repeat it in full visibility
 	    (user-error "Edit in invisible region aborted, repeat to confirm with text visible"))))))))
 
+(defun org-fold-check-before-invisible-edit-maybe (&rest _)
+  "Check before invisible command by `this-command'."
+  (when (derived-mode-p 'org-mode)
+    (pcase (alist-get this-command org-fold-catch-invisible-edits-commands)
+      ((pred null) nil)
+      (kind (org-fold-check-before-invisible-edit kind)))))
+
+(defun org-fold--advice-edit-commands ()
+  "Advice editing commands according to `org-fold-catch-invisible-edits-commands'.
+The advices are installed in current buffer."
+  (dolist (command (mapcar #'car org-fold-catch-invisible-edits-commands))
+    (advice-add command :before #'org-fold-check-before-invisible-edit-maybe)))
+
 (provide 'org-fold)
 
 ;;; org-fold.el ends here
diff --git a/lisp/org.el b/lisp/org.el
index 4d12084d9..8d88f83e7 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -4839,6 +4839,8 @@ (define-derived-mode org-mode outline-mode "Org"
   (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)
+  ;; Check for invisible edits.
+  (org-fold--advice-edit-commands)
   ;; Initialize cache.
   (org-element-cache-reset)
   (when (and org-element-cache-persistent
@@ -16440,7 +16442,6 @@ (defun org-self-insert-command (N)
 If the cursor is in a table looking at whitespace, the whitespace is
 overwritten, and the table is not marked as requiring realignment."
   (interactive "p")
-  (org-fold-check-before-invisible-edit 'insert)
   (cond
    ((and org-use-speed-commands
 	 (let ((kv (this-command-keys-vector)))
@@ -16510,7 +16511,6 @@ (defun org-delete-backward-char (N)
 because, in this case the deletion might narrow the column."
   (interactive "p")
   (save-match-data
-    (org-fold-check-before-invisible-edit 'delete-backward)
     (if (and (= N 1)
 	     (not overwrite-mode)
 	     (not (org-region-active-p))
@@ -16530,7 +16530,6 @@ (defun org-delete-char (N)
 because, in this case the deletion might narrow the column."
   (interactive "p")
   (save-match-data
-    (org-fold-check-before-invisible-edit 'delete)
     (cond
      ((or (/= N 1)
 	  (eq (char-after) ?|)
@@ -17944,7 +17943,6 @@ (defun org-meta-return (&optional arg)
 `org-table-wrap-region', depending on context.  When called with
 an argument, unconditionally call `org-insert-heading'."
   (interactive "P")
-  (org-fold-check-before-invisible-edit 'insert)
   (or (run-hook-with-args-until-success 'org-metareturn-hook)
       (call-interactively (cond (arg #'org-insert-heading)
 				((org-at-table-p) #'org-table-wrap-region)
-- 
2.39.1


[-- Attachment #3: Type: text/plain, Size: 224 bytes --]


-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [PATCH] Allow customizing commands affected by `org-fold-catch-invisible-edits' (was: Should we extend org-catch-invisible-edits to more interactive commands? (was: Catching invisible edits: problem understanding doc))

[Alain.Cochard]

Ihor Radchenko writes on Sun 12 Feb 2023 15:23:

 > [...] I want to implement more generic feature.
 > See the attached patch.

This sounds very promising.  I find the doc and the two relevant
docstrings clear.

However, I could not have it work for my case so far.  I tried to add
"(undo . insert)", "(undo . delete-backward)" and "(undo . delete)" to
'org-fold-catch-invisible-edits-commands', one by one and together,
and all values of 'org-fold-catch-invisible-edits'.  Any clue of what
I am doing wrong?  Thanks.

-- 
EOST (École et Observatoire des Sciences de la Terre) 
ITE (Institut Terre & Environnement) | alain.cochard@unistra.fr
5 rue René Descartes   [bureau 110]  | Phone: +33 (0)3 68 85 50 44 
F-67084 Strasbourg Cedex, France     | [ slot available for rent ]

Re: [PATCH] Allow customizing commands affected by `org-fold-catch-invisible-edits' (was: Should we extend org-catch-invisible-edits to more interactive commands? (was: Catching invisible edits: problem understanding doc))

[Ihor Radchenko]

Alain.Cochard@unistra.fr writes:

> However, I could not have it work for my case so far.  I tried to add
> "(undo . insert)", "(undo . delete-backward)" and "(undo . delete)" to
> 'org-fold-catch-invisible-edits-commands', one by one and together,
> and all values of 'org-fold-catch-invisible-edits'.  Any clue of what
> I am doing wrong?  Thanks.

As the docstring states, the check is performed at point.
Checking the actual modifications would be rather tricky. It might be
done, but there are so many caveats that the code will be fragile.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [PATCH] Allow customizing commands affected by `org-fold-catch-invisible-edits' (was: Should we extend org-catch-invisible-edits to more interactive commands? (was: Catching invisible edits: problem understanding doc))

[Alain.Cochard]

Ihor Radchenko writes on Thu 16 Feb 2023 15:06:

 > As the docstring states, the check is performed at point.  Checking
 > the actual modifications would be rather tricky. It might be done,
 > but there are so many caveats that the code will be fragile.

OK, too bad.  Thanks for trying.  

-- 
EOST (École et Observatoire des Sciences de la Terre) 
ITE (Institut Terre & Environnement) | alain.cochard@unistra.fr
5 rue René Descartes   [bureau 110]  | Phone: +33 (0)3 68 85 50 44 
F-67084 Strasbourg Cedex, France     | [ slot available for rent ]

Re: [PATCH] Allow customizing commands affected by `org-fold-catch-invisible-edits' (was: Should we extend org-catch-invisible-edits to more interactive commands? (was: Catching invisible edits: problem understanding doc))

[Ihor Radchenko]

Alain.Cochard@unistra.fr writes:

> However, I could not have it work for my case so far.  I tried to add
> "(undo . insert)", "(undo . delete-backward)" and "(undo . delete)" to
> 'org-fold-catch-invisible-edits-commands', one by one and together,
> and all values of 'org-fold-catch-invisible-edits'.  Any clue of what
> I am doing wrong?  Thanks.

What can be reasonably done here is revealing edits, when they are done
inside folded text. Do you think that adding such an option to Org would
be useful?

Note that we cannot easily prevent edits inside invisible region because
some edits may start in visible text and then continue inside invisible
- aborting only invisible part of the edit will cause partial edited
state, which is terrible idea.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [PATCH] Allow customizing commands affected by `org-fold-catch-invisible-edits' (was: Should we extend org-catch-invisible-edits to more interactive commands? (was: Catching invisible edits: problem understanding doc))

[Ihor Radchenko]

Ihor Radchenko <yantar92@posteo.net> writes:

> Subject: [PATCH] org-fold: Allow customizing commands where invisible edits
>  are checked

Applied, onto main.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>