emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* Outline and org-mode don't insert text into folded sections logically
@ 2011-09-03 19:14 Kelly Dean
  2011-09-05  7:52 ` Giovanni Ridolfi
  0 siblings, 1 reply; 25+ messages in thread
From: Kelly Dean @ 2011-09-03 19:14 UTC (permalink / raw)
  To: emacs-orgmode

Using Emacs 23.2.1, make a buffer in outline mode and enter this:
*1A
Body of 1A
**2A
Body of 2A
***3A
Body of 3A
*1B
Body of 1B

Now, collapse all except the first item, so you have:
*1A
Body of 1A...
*1B...

Put the point at the beginning of the line "*1B...", type "test" and press enter. Now you have:
*1A
Body of 1A...
test
*1B...

Which item is the new line "test" part of? It looks like it should be part of 1A, but that's impossible, due to the use of implicit item termination and the existence of items 2A and 3A.
Expanding the text gives:
*1A
Body of 1A
**2A
Body of 2A
***3A
Body of 3A
test
*1B
Body of 1B

So, the new line is part of 3A, which can be confirmed by hiding 3A, which will hide both the line "Body of 3A" and "test".

Why is this a problem? Because it's illogical for the new line to appear on screen but go into an item which is folded away (besides that, there's not even any indication on screen of which item the line is going into). If the new line is part of the body of 3A, then shouldn't the entirety of that body (including the line "Body of 3A") be shown, rather than just part of it (the line "test")? The problem could logically be solved by _not_ printing the line onto the screen as it's typed, since it's going into a body which is folded away, but of course this would make for a very unfriendly user interface. Alternatively, upon typing the first character of the new line, the body of 3A could be automatically unfolded (and the heading of 2A unfolded above it) to make it obvious where the text is 
 going, but this is also not very friendly, because it modifies the folding state without the user's explicit request, and causes a potentially large and disorienting
 change in the on-screen display when the user was just trying to type in a little bit of text.
The most reasonable solution seems to be to reject the text entry attempt, and treat it as though the user is attempting to modify read-only text (which Emacs is already capable of handling); just display a message explaining that in order to enter text at the current position, the outline must be unfolded. This is reasonable (assuming implicitly terminating items by the beginning of the next item is even an appropriate model for the outline in the first place, as opposed to using an explicit termination symbol) because entering the new line "test" under the visual line "Body of 1A..." is most likely not an actual attempt by the user to enter the line into the body of 3A; it's much more likely a mistake, with the intended destination for the text being somewhere else, perhaps even the body
  of 1A but following the fold, due to the user failing to realize that implicit item termination makes that impossible, or forgetting that the outline mode is using
 implicit item termination.

Org-mode exhibits the same problem which outline mode does. Put spaces after the asterisks as org-mode requires, switch to org-mode, and cycle visibility to _overview_ to get:
* 1A...
* 1B...

Put the point at the beginning of the line "* 1B...", type "test2" and press enter. Now you have:
* 1A...
test2
* 1B...

Cycle visibility to _show all_, and you see that "test2" has been entered into the body of 3A.

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

* Re: Outline and org-mode don't insert text into folded sections logically
  2011-09-03 19:14 Outline and org-mode don't insert text into folded sections logically Kelly Dean
@ 2011-09-05  7:52 ` Giovanni Ridolfi
  2011-09-05  8:01   ` suvayu ali
  0 siblings, 1 reply; 25+ messages in thread
From: Giovanni Ridolfi @ 2011-09-05  7:52 UTC (permalink / raw)
  To: Kelly Dean; +Cc: emacs-orgmode

Kelly Dean <kellydeanch@yahoo.com> writes:

> Using Emacs 23.2.1, make a buffer in outline mode and enter this:
> *1A
> Body of 1A
> **2A
> Body of 2A
> ***3A
> Body of 3A

> Org-mode exhibits the same problem which outline mode does. Put spaces
> after the asterisks as org-mode requires, switch to org-mode, and
> cycle visibility to _overview_ to get:  
> * 1A...
> * 1B...
>
> Put the point at the beginning of the line "* 1B...", type "test2" and press enter. Now you have:
> * 1A...
> test2
> * 1B...
>
> Cycle visibility to _show all_, and you see that "test2" has been
> entered into the body of 3A.

<joking>

                  thou shalt not edit folded subtrees.

</joking>

Hi, Kelly,

I can understand that this seems to be illogical from your point of
view, but  folded subtrees are meant for a faster and cleaner
organization of the text, not for editing.

Once you've decided to edit a subtree you should expand it.

Hope that explains,

Giovanni

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

* Re: Outline and org-mode don't insert text into folded sections logically
  2011-09-05  7:52 ` Giovanni Ridolfi
@ 2011-09-05  8:01   ` suvayu ali
  2011-10-22 10:13     ` Bastien
  0 siblings, 1 reply; 25+ messages in thread
From: suvayu ali @ 2011-09-05  8:01 UTC (permalink / raw)
  To: Giovanni Ridolfi; +Cc: emacs-orgmode, Kelly Dean

Hi,

On Mon, Sep 5, 2011 at 9:52 AM, Giovanni Ridolfi
<giovanni.ridolfi@yahoo.it> wrote:
> Hi, Kelly,
>
> I can understand that this seems to be illogical from your point of
> view, but  folded subtrees are meant for a faster and cleaner
> organization of the text, not for editing.
>
> Once you've decided to edit a subtree you should expand it.
>

I think this might be the right time to ask for a feature request,
unfold a tree if someone tries to edit it. This will prevent many
"mishaps".

> Hope that explains,
>
> Giovanni

-- 
Suvayu

Open source is the future. It sets us free.

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

* Re: Outline and org-mode don't insert text into folded sections logically
  2011-09-05  8:01   ` suvayu ali
@ 2011-10-22 10:13     ` Bastien
  2011-10-22 10:17       ` suvayu ali
  2011-10-22 10:21       ` Jambunathan K
  0 siblings, 2 replies; 25+ messages in thread
From: Bastien @ 2011-10-22 10:13 UTC (permalink / raw)
  To: suvayu ali; +Cc: Kelly Dean, emacs-orgmode

Hi Suvayu,

suvayu ali <fatkasuvayu+linux@gmail.com> writes:

> I think this might be the right time to ask for a feature request,
> unfold a tree if someone tries to edit it. This will prevent many
> "mishaps".

Good idea.  There is `post-self-insert-hook' but there is no
`pre-self-insert-hook' that would check whether the point is in 
an invisible area of the buffer, and send a warning about this.

Any idea on how to implement this?

-- 
 Bastien

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

* Re: Outline and org-mode don't insert text into folded sections logically
  2011-10-22 10:13     ` Bastien
@ 2011-10-22 10:17       ` suvayu ali
  2011-10-22 10:21       ` Jambunathan K
  1 sibling, 0 replies; 25+ messages in thread
From: suvayu ali @ 2011-10-22 10:17 UTC (permalink / raw)
  To: Bastien; +Cc: Kelly Dean, emacs-orgmode

Hi Bastien,

On Sat, Oct 22, 2011 at 12:13 PM, Bastien <bzg@altern.org> wrote:
>
> suvayu ali <fatkasuvayu+linux@gmail.com> writes:
>
>> I think this might be the right time to ask for a feature request,
>> unfold a tree if someone tries to edit it. This will prevent many
>> "mishaps".
>
> Good idea.  There is `post-self-insert-hook' but there is no
> `pre-self-insert-hook' that would check whether the point is in
> an invisible area of the buffer, and send a warning about this.
>
> Any idea on how to implement this?
>

Sorry, I had been meaning to take a look at this for a long time but
haven't had the time so far. :(

-- 
Suvayu

Open source is the future. It sets us free.

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

* Re: Outline and org-mode don't insert text into folded sections logically
  2011-10-22 10:13     ` Bastien
  2011-10-22 10:17       ` suvayu ali
@ 2011-10-22 10:21       ` Jambunathan K
  2011-10-22 13:53         ` Bastien
  1 sibling, 1 reply; 25+ messages in thread
From: Jambunathan K @ 2011-10-22 10:21 UTC (permalink / raw)
  To: Bastien; +Cc: emacs-orgmode, Kelly Dean

Bastien <bzg@altern.org> writes:

> Hi Suvayu,
>
> suvayu ali <fatkasuvayu+linux@gmail.com> writes:
>
>> I think this might be the right time to ask for a feature request,
>> unfold a tree if someone tries to edit it. This will prevent many
>> "mishaps".
>
> Good idea.  There is `post-self-insert-hook' but there is no
> `pre-self-insert-hook' that would check whether the point is in 
> an invisible area of the buffer, and send a warning about this.

In org-self-insert-command check for visibility at point and take the
required action.

> Any idea on how to implement this?

-- 

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

* Re: Outline and org-mode don't insert text into folded sections logically
  2011-10-22 10:21       ` Jambunathan K
@ 2011-10-22 13:53         ` Bastien
  2011-10-23  2:18           ` suvayu ali
  0 siblings, 1 reply; 25+ messages in thread
From: Bastien @ 2011-10-22 13:53 UTC (permalink / raw)
  To: suvayu ali; +Cc: emacs-orgmode, Kelly Dean

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

Jambunathan K <kjambunathan@gmail.com> writes:

>> Good idea.  There is `post-self-insert-hook' but there is no
>> `pre-self-insert-hook' that would check whether the point is in 
>> an invisible area of the buffer, and send a warning about this.
>
> In org-self-insert-command check for visibility at point and take the
> required action.

That's it.  Thanks!

Here is a dummy patch that prevents the user from editing invisible
parts of the buffer.  It doesn't prevent query-and-replace commands.

Can people test it and comment it?

Maybe throwing an error is a bit too much.  Maybe preventing all 
kind of edition in invisible parts of the buffer is too much as 
well -- looking forward reading comments on this.

Thanks,


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Prevent-self-insert-command-in-invisible-regions.patch --]
[-- Type: text/x-patch, Size: 981 bytes --]

From 442f30b74f3f4eb888b63cb5d2cd04542952f84a Mon Sep 17 00:00:00 2001
From: Bastien Guerry <bzg@altern.org>
Date: Sat, 22 Oct 2011 15:50:52 +0200
Subject: [PATCH] Prevent `self-insert-command' in invisible regions.

* org.el (org-self-insert-command): Throw an error when the
user is trying to use self-insert-command in invisible regions.
---
 lisp/org.el |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index d82ae0c..627faa3 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -17328,6 +17328,9 @@ 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")
   (cond
+   ((eq (car (get-char-property-and-overlay
+	      (point) 'invisible)) 'outline)
+    (error "Attempt to edit an invisible part of the buffer"))
    ((and org-use-speed-commands
 	 (setq org-speed-command
 	       (run-hook-with-args-until-success
-- 
1.7.6.1


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


-- 
 Bastien

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

* Re: Outline and org-mode don't insert text into folded sections logically
  2011-10-22 13:53         ` Bastien
@ 2011-10-23  2:18           ` suvayu ali
  2011-10-29 14:10             ` Bastien
  0 siblings, 1 reply; 25+ messages in thread
From: suvayu ali @ 2011-10-23  2:18 UTC (permalink / raw)
  To: Bastien; +Cc: emacs-orgmode, Kelly Dean

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

Hi Bastien,

A few comments. I have also attached a small test file.

On Sat, Oct 22, 2011 at 15:53, Bastien <bzg@altern.org> wrote:
>
> Here is a dummy patch that prevents the user from editing invisible
> parts of the buffer.  It doesn't prevent query-and-replace commands.
>
> Can people test it and comment it?
>
> Maybe throwing an error is a bit too much.  Maybe preventing all
> kind of edition in invisible parts of the buffer is too much as
> well -- looking forward reading comments on this.
>

Using org-reveal might be better. I see that org-reveal works for lists
but not for headlines or folded src_blocks (is that a regression?). If
not, something with similar functionality for headlines and src_blocks
would be great.

Now about something more subtle and open to subjective opinion. In the
example file, if you go to the beginning of a folded headline / list /
src_block and hit C-e, this should take you to the end of the ellipsis.
Now attempting to edit should either throw an error or unfold the item
(depending on whether you have the patch as is or with my proposal).
However if you press the cursor key to the right once after the C-e in
the last step, then you end up after the ellipsis and inserting text is
possible. The inserted text is placed right above the next entry. So in
the attached test file, if you do this with the first headline the
inserted text will be right before the second headline.

In my opinion this means the problem would still exist. I don't
understand overlays very well, but I was thinking if it could be
resolved by adding a new line to the ellipsis. I customised org-ellipsis
accordingly, although this didn't solve the problem it made it more
obvious when I edited the folded entry inadvertently.

I am not sure what would be an appropriate solution here. If there was a
way to determine if you are inside a folded entity instead of testing
for invisibility of `point' it might be better (maybe test if there is an
overlayed ellipsis?).

> Thanks,

Hope these comments help.

-- 
Suvayu

Open source is the future. It sets us free.

[-- Attachment #2: invisible-test.org --]
[-- Type: text/x-org, Size: 318 bytes --]

#+STARTUP: overview

* This is 1st headline
** Some sub-heading
Followed by some text

+ Maybe a list some multiline content, lets say a code block.
  #+begin_src sh :eval no
    echo "This is $USER testing org"
  #+end_src
+ Second item in the list

* Text inserted before the 2nd headline
With some text underneath


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

* Re: Outline and org-mode don't insert text into folded sections logically
  2011-10-23  2:18           ` suvayu ali
@ 2011-10-29 14:10             ` Bastien
  2011-10-29 14:57               ` suvayu ali
  0 siblings, 1 reply; 25+ messages in thread
From: Bastien @ 2011-10-29 14:10 UTC (permalink / raw)
  To: suvayu ali; +Cc: emacs-orgmode, Kelly Dean

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

Hi Suvayu,

suvayu ali <fatkasuvayu+linux@gmail.com> writes:

> A few comments. I have also attached a small test file.

thanks for the comments.   Can you try the updated patch?
It will take care of unfold the invisible part of the buffer
when trying to edit _in_ it and _right after it_.

As for `org-reveal', please report a bug if it doesn't work
as expected in specific parts of the buffer.

Thanks for your useful comments!


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Prevent-self-insert-command-in-invisible-regions.patch --]
[-- Type: text/x-patch, Size: 981 bytes --]

From 442f30b74f3f4eb888b63cb5d2cd04542952f84a Mon Sep 17 00:00:00 2001
From: Bastien Guerry <bzg@altern.org>
Date: Sat, 22 Oct 2011 15:50:52 +0200
Subject: [PATCH] Prevent `self-insert-command' in invisible regions.

* org.el (org-self-insert-command): Throw an error when the
user is trying to use self-insert-command in invisible regions.
---
 lisp/org.el |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index d82ae0c..627faa3 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -17328,6 +17328,9 @@ 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")
   (cond
+   ((eq (car (get-char-property-and-overlay
+	      (point) 'invisible)) 'outline)
+    (error "Attempt to edit an invisible part of the buffer"))
    ((and org-use-speed-commands
 	 (setq org-speed-command
 	       (run-hook-with-args-until-success
-- 
1.7.6.1


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


-- 
 Bastien

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

* Re: Outline and org-mode don't insert text into folded sections logically
  2011-10-29 14:10             ` Bastien
@ 2011-10-29 14:57               ` suvayu ali
  2011-10-29 15:40                 ` Bastien
  0 siblings, 1 reply; 25+ messages in thread
From: suvayu ali @ 2011-10-29 14:57 UTC (permalink / raw)
  To: Bastien; +Cc: emacs-orgmode, Kelly Dean

Hi Bastien,

On Sat, Oct 29, 2011 at 16:10, Bastien <bzg@altern.org> wrote:
> Hi Suvayu,
>
> suvayu ali <fatkasuvayu+linux@gmail.com> writes:
>
>> A few comments. I have also attached a small test file.
>
> thanks for the comments.   Can you try the updated patch?
> It will take care of unfold the invisible part of the buffer
> when trying to edit _in_ it and _right after it_.
>

Did you by any chance attach the old patch again? ;)

> As for `org-reveal', please report a bug if it doesn't work
> as expected in specific parts of the buffer.
>

I will try to make a more complete analysis and report it.

-- 
Suvayu

Open source is the future. It sets us free.

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

* Re: Outline and org-mode don't insert text into folded sections logically
  2011-10-29 14:57               ` suvayu ali
@ 2011-10-29 15:40                 ` Bastien
  2011-10-29 15:53                   ` suvayu ali
  0 siblings, 1 reply; 25+ messages in thread
From: Bastien @ 2011-10-29 15:40 UTC (permalink / raw)
  To: suvayu ali; +Cc: emacs-orgmode, Kelly Dean

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

Hi Suvayu,

suvayu ali <fatkasuvayu+linux@gmail.com> writes:

> Did you by any chance attach the old patch again? ;)

Er, yes :)

Here is the fresh one -- thanks for testing it.

>> As for `org-reveal', please report a bug if it doesn't work
>> as expected in specific parts of the buffer.
>
> I will try to make a more complete analysis and report it.

Thanks!


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org.el-Unfold-invisible-region-at-point-or-right-bef.patch --]
[-- Type: text/x-patch, Size: 1693 bytes --]

From 23ef0209d53b0acdf2158da9dd8d4532b437f97e Mon Sep 17 00:00:00 2001
From: Bastien Guerry <bzg@altern.org>
Date: Sat, 29 Oct 2011 16:08:27 +0200
Subject: [PATCH]  org.el: Unfold invisible region at point or right before
 point when editing.

* org.el (org-self-insert-command): Unfold invisible region at
point or right before point when editing.

Thanks to Suvayu Ali for discussing this.
---
 lisp/org.el |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index fe45cf7..6c809be 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -17393,6 +17393,23 @@ hook.  The default setting is `org-speed-command-default-hook'."
 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")
+  (let ((invisible-at-point 
+	 (car (get-char-property-and-overlay (point) 'invisible)))
+	(invisible-before-point 
+	 (car (get-char-property-and-overlay (1- (point)) 'invisible))))
+    (when (or (eq invisible-at-point 'outline)
+	    (eq invisible-at-point 'org-hide-block)
+	    (eq invisible-before-point 'outline)
+	    (eq invisible-before-point 'org-hide-block))
+      (if (or (eq invisible-before-point 'outline)
+	      (eq invisible-before-point 'org-hide-block))
+	  (goto-char (previous-overlay-change (point))))
+      (org-cycle)
+      (if (or (eq invisible-before-point 'outline)
+	      (eq invisible-before-point 'org-hide-block))
+	  (forward-char 1))
+      (message "Unfolding invisible region around point before editing")
+      (sit-for 1)))
   (cond
    ((and org-use-speed-commands
 	 (setq org-speed-command
-- 
1.7.7.1


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


-- 
 Bastien

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

* Re: Outline and org-mode don't insert text into folded sections logically
  2011-10-29 15:40                 ` Bastien
@ 2011-10-29 15:53                   ` suvayu ali
  2011-10-29 16:15                     ` Bastien
  0 siblings, 1 reply; 25+ messages in thread
From: suvayu ali @ 2011-10-29 15:53 UTC (permalink / raw)
  To: Bastien; +Cc: emacs-orgmode, Kelly Dean

Hi Bastien,

This works beautifully! I noticed that there is a tiny lag (maybe
about half a second) between the revealing and inserting the
character. However there is no such issue if there is no need to
reveal the surrounding text.

Thanks a lot for implementing this feature. :)



On Sat, Oct 29, 2011 at 17:40, Bastien <bzg@altern.org> wrote:
> Hi Suvayu,
>
> suvayu ali <fatkasuvayu+linux@gmail.com> writes:
>
>> Did you by any chance attach the old patch again? ;)
>
> Er, yes :)
>
> Here is the fresh one -- thanks for testing it.
>
>>> As for `org-reveal', please report a bug if it doesn't work
>>> as expected in specific parts of the buffer.
>>
>> I will try to make a more complete analysis and report it.
>
> Thanks!
>
>
>
> --
>  Bastien
>
>



-- 
Suvayu

Open source is the future. It sets us free.

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

* Re: Outline and org-mode don't insert text into folded sections logically
  2011-10-29 15:53                   ` suvayu ali
@ 2011-10-29 16:15                     ` Bastien
  2011-10-29 16:22                       ` suvayu ali
  0 siblings, 1 reply; 25+ messages in thread
From: Bastien @ 2011-10-29 16:15 UTC (permalink / raw)
  To: suvayu ali; +Cc: Kelly Dean, emacs-orgmode

Hi Suvayu,

suvayu ali <fatkasuvayu+linux@gmail.com> writes:

> This works beautifully! I noticed that there is a tiny lag (maybe
> about half a second) between the revealing and inserting the
> character. However there is no such issue if there is no need to
> reveal the surrounding text.

The lag is intentional -- just to make sure people read the message 
and know what they do...  If you and others find this is useless, I'll
remove it.

Thanks for testing!

-- 
 Bastien

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

* Re: Outline and org-mode don't insert text into folded sections logically
  2011-10-29 16:15                     ` Bastien
@ 2011-10-29 16:22                       ` suvayu ali
  2011-10-30  1:07                         ` Bastien
  0 siblings, 1 reply; 25+ messages in thread
From: suvayu ali @ 2011-10-29 16:22 UTC (permalink / raw)
  To: Bastien; +Cc: Kelly Dean, emacs-orgmode

On Sat, Oct 29, 2011 at 18:15, Bastien <bzg@altern.org> wrote:
> Hi Suvayu,
>
> suvayu ali <fatkasuvayu+linux@gmail.com> writes:
>
>> This works beautifully! I noticed that there is a tiny lag (maybe
>> about half a second) between the revealing and inserting the
>> character. However there is no such issue if there is no need to
>> reveal the surrounding text.
>
> The lag is intentional -- just to make sure people read the message
> and know what they do...  If you and others find this is useless, I'll
> remove it.
>
> Thanks for testing!

Ah! Okay that seems reasonable, lets see what others think. :)

-- 
Suvayu

Open source is the future. It sets us free.

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

* Re: Outline and org-mode don't insert text into folded sections logically
  2011-10-29 16:22                       ` suvayu ali
@ 2011-10-30  1:07                         ` Bastien
  2011-10-30  6:28                           ` Carsten Dominik
  0 siblings, 1 reply; 25+ messages in thread
From: Bastien @ 2011-10-30  1:07 UTC (permalink / raw)
  To: suvayu ali; +Cc: Kelly Dean, emacs-orgmode

Hi Suvayu,

suvayu ali <fatkasuvayu+linux@gmail.com> writes:

> Ah! Okay that seems reasonable, lets see what others think. :)

I've now pushed this commit so that more people can test it.

-- 
 Bastien

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

* Re: Outline and org-mode don't insert text into folded sections logically
  2011-10-30  1:07                         ` Bastien
@ 2011-10-30  6:28                           ` Carsten Dominik
  2011-10-30  7:30                             ` Jambunathan K
  2011-10-30  9:04                             ` Bastien
  0 siblings, 2 replies; 25+ messages in thread
From: Carsten Dominik @ 2011-10-30  6:28 UTC (permalink / raw)
  To: Bastien; +Cc: emacs-orgmode, Kelly Dean


On 30.10.2011, at 02:07, Bastien wrote:

> Hi Suvayu,
> 
> suvayu ali <fatkasuvayu+linux@gmail.com> writes:
> 
>> Ah! Okay that seems reasonable, lets see what others think. :)
> 
> I've now pushed this commit so that more people can test it.

While I think that this is a potentially useful idea, 
I would like to point out a few things:

- This patch covers only one of many ways to make unwanted changes
  in an invisible area.  Others would be delete, backspace,
  kill-region, yank, kill-line, and an arbitrarily long list of
  less obvious other commands.  Full protection could only be
  done with pre-change-hooks or so, but would then prevent
  also programmed changes - something that would not be useful.
  `org-self-insert-command' is probably only ever used in an
  interactive way, so the patch as you have written it may very
  well function correctly.

- All the code in org-self-insert-command is executed for each
  keypress, so one needs to be careful to have this function
  carry as little overhead as possible.

- Currently this chokes at the beginning of the buffer because
  the invisibility test is also run at (1- (point)).

- I am not sure if I understand the positioning code:
> (if (or (eq invisible-before-point 'outline)
> 	(eq invisible-before-point 'org-hide-block))
>     (goto-char (previous-overlay-change (point))))
> (org-cycle)
> (if (or (eq invisible-before-point 'outline)
> 	(eq invisible-before-point 'org-hide-block))
>     (forward-char 1))

  So when I happen to be somewhere in the middle of invisible
  text and press a character, it seems to me that the character
  will be inserted at the beginning of the invisible text, and
  not where the cursor was.

  Maybe a better solution would be to save point, unfold,
  go back to point, throw and error and not insert the pressed
  character.  I am not sure, though.  Maybe you can explain
  your reasoning?

Regards

- Carsten

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

* Re: Outline and org-mode don't insert text into folded sections logically
  2011-10-30  6:28                           ` Carsten Dominik
@ 2011-10-30  7:30                             ` Jambunathan K
  2011-10-30  7:47                               ` Carsten Dominik
  2011-10-30  9:04                             ` Bastien
  1 sibling, 1 reply; 25+ messages in thread
From: Jambunathan K @ 2011-10-30  7:30 UTC (permalink / raw)
  To: emacs-orgmode


> - This patch covers only one of many ways to make unwanted changes
>   in an invisible area.  Others would be delete, backspace,
>   kill-region, yank, kill-line, and an arbitrarily long list of
>   less obvious other commands.  Full protection could only be
>   done with pre-change-hooks or so, but would then prevent
              ^^^^^^^^^^^^^^^
              ^^^^^^^^^^^^^^^
May be you were referring to `before-change-functions' here?

I see the use of the above hook already in Org. (Strangely this hook
ends with "functions" and has no has no hook in it.)

,----
|   (org-add-hook 'before-change-functions 'org-before-change-function nil
| 		'local)
`----

>   also programmed changes - something that would not be useful.
>   `org-self-insert-command' is probably only ever used in an
>   interactive way, so the patch as you have written it may very
>   well function correctly.

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

* Re: Outline and org-mode don't insert text into folded sections logically
  2011-10-30  7:30                             ` Jambunathan K
@ 2011-10-30  7:47                               ` Carsten Dominik
  0 siblings, 0 replies; 25+ messages in thread
From: Carsten Dominik @ 2011-10-30  7:47 UTC (permalink / raw)
  To: Jambunathan K; +Cc: emacs-orgmode


On 30.10.2011, at 08:30, Jambunathan K wrote:

> 
>> - This patch covers only one of many ways to make unwanted changes
>>  in an invisible area.  Others would be delete, backspace,
>>  kill-region, yank, kill-line, and an arbitrarily long list of
>>  less obvious other commands.  Full protection could only be
>>  done with pre-change-hooks or so, but would then prevent
>              ^^^^^^^^^^^^^^^
>              ^^^^^^^^^^^^^^^
> May be you were referring to `before-change-functions' here?

Yes, my mistake.

- Carsten

> 
> I see the use of the above hook already in Org. (Strangely this hook
> ends with "functions" and has no has no hook in it.)
> 
> ,----
> |   (org-add-hook 'before-change-functions 'org-before-change-function nil
> | 		'local)
> `----
> 
>>  also programmed changes - something that would not be useful.
>>  `org-self-insert-command' is probably only ever used in an
>>  interactive way, so the patch as you have written it may very
>>  well function correctly.
> 

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

* Re: Outline and org-mode don't insert text into folded sections logically
  2011-10-30  6:28                           ` Carsten Dominik
  2011-10-30  7:30                             ` Jambunathan K
@ 2011-10-30  9:04                             ` Bastien
  2011-10-30 15:37                               ` Carsten Dominik
  2011-11-02  7:12                               ` Carsten Dominik
  1 sibling, 2 replies; 25+ messages in thread
From: Bastien @ 2011-10-30  9:04 UTC (permalink / raw)
  To: Carsten Dominik; +Cc: emacs-orgmode, Kelly Dean

Hi Carsten,

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

> - This patch covers only one of many ways to make unwanted changes
>   in an invisible area.  Others would be delete, backspace,
>   kill-region, yank, kill-line, and an arbitrarily long list of
>   less obvious other commands.  Full protection could only be
>   done with pre-change-hooks or so, but would then prevent
>   also programmed changes - something that would not be useful.

Yes, I don't want programmed changes to be affected by this feature. 

But having such a warning for `org-delete' would also be useful IMHO.

>   `org-self-insert-command' is probably only ever used in an
>   interactive way, so the patch as you have written it may very
>   well function correctly.
>
> - All the code in org-self-insert-command is executed for each
>   keypress, so one needs to be careful to have this function
>   carry as little overhead as possible.

I actually think there should be a user option
`org-edit-invisible-send-warning' defaulting to nil.

The request "don't let me shoot in my foot" is a common
one, and this option would let people set this to `t'.

> - Currently this chokes at the beginning of the buffer because
>   the invisibility test is also run at (1- (point)).

Fixed, thanks.

> - I am not sure if I understand the positioning code:
>> (if (or (eq invisible-before-point 'outline)
>> 	(eq invisible-before-point 'org-hide-block))
>>     (goto-char (previous-overlay-change (point))))
>> (org-cycle)
>> (if (or (eq invisible-before-point 'outline)
>> 	(eq invisible-before-point 'org-hide-block))
>>     (forward-char 1))
>
>   So when I happen to be somewhere in the middle of invisible
>   text and press a character, it seems to me that the character
>   will be inserted at the beginning of the invisible text, and
>   not where the cursor was.
>
>   Maybe a better solution would be to save point, unfold,
>   go back to point, throw and error and not insert the pressed
>   character.  I am not sure, though.  

Throwing an error and not inserting the text was what my first
patch did.  I thought it was too restrictive, though.

With an option `org-edit-invisible-send-warning', we could have both:
`t' would just throw a warning, 'prevent would throw an error.

> Maybe you can explain your reasoning?

My reasoning is that, when in the "middle" of an invisible region,
the user does not know where the point is, hence he doesn't really 
know where he wants to insert characters.  In this case, I assume 
insert at the beginning of the invisible region is a reasonable
default.

Thanks for the feedback -- let's continue brainstorming, I think
this feature is important.

Best,

-- 
 Bastien

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

* Re: Outline and org-mode don't insert text into folded sections logically
  2011-10-30  9:04                             ` Bastien
@ 2011-10-30 15:37                               ` Carsten Dominik
  2011-10-31 20:58                                 ` Carsten Dominik
  2011-11-02  7:12                               ` Carsten Dominik
  1 sibling, 1 reply; 25+ messages in thread
From: Carsten Dominik @ 2011-10-30 15:37 UTC (permalink / raw)
  To: Bastien; +Cc: emacs-orgmode, Kelly Dean


On 30.10.2011, at 10:04, Bastien wrote:

> Hi Carsten,
> 
> Carsten Dominik <carsten.dominik@gmail.com> writes:
> 
>> - This patch covers only one of many ways to make unwanted changes
>>  in an invisible area.  Others would be delete, backspace,
>>  kill-region, yank, kill-line, and an arbitrarily long list of
>>  less obvious other commands.  Full protection could only be
>>  done with pre-change-hooks or so, but would then prevent
>>  also programmed changes - something that would not be useful.
> 
> Yes, I don't want programmed changes to be affected by this feature. 
> 
> But having such a warning for `org-delete' would also be useful IMHO.

I guess you mean, org-delete-char and org-delete-backward-char

> 
>>  `org-self-insert-command' is probably only ever used in an
>>  interactive way, so the patch as you have written it may very
>>  well function correctly.
>> 
>> - All the code in org-self-insert-command is executed for each
>>  keypress, so one needs to be careful to have this function
>>  carry as little overhead as possible.
> 
> I actually think there should be a user option
> `org-edit-invisible-send-warning' defaulting to nil.

+1

> 
> The request "don't let me shoot in my foot" is a common
> one, and this option would let people set this to `t'.
> 
>> - Currently this chokes at the beginning of the buffer because
>>  the invisibility test is also run at (1- (point)).
> 
> Fixed, thanks.
> 
>> - I am not sure if I understand the positioning code:
>>> (if (or (eq invisible-before-point 'outline)
>>> 	(eq invisible-before-point 'org-hide-block))
>>>    (goto-char (previous-overlay-change (point))))
>>> (org-cycle)
>>> (if (or (eq invisible-before-point 'outline)
>>> 	(eq invisible-before-point 'org-hide-block))
>>>    (forward-char 1))
>> 
>>  So when I happen to be somewhere in the middle of invisible
>>  text and press a character, it seems to me that the character
>>  will be inserted at the beginning of the invisible text, and
>>  not where the cursor was.
>> 
>>  Maybe a better solution would be to save point, unfold,
>>  go back to point, throw and error and not insert the pressed
>>  character.  I am not sure, though.  
> 
> Throwing an error and not inserting the text was what my first
> patch did.  I thought it was too restrictive, though.
> 
> With an option `org-edit-invisible-send-warning', we could have both:
> `t' would just throw a warning, 'prevent would throw an error.

I like that.

> 
>> Maybe you can explain your reasoning?
> 
> My reasoning is that, when in the "middle" of an invisible region,
> the user does not know where the point is, hence he doesn't really 
> know where he wants to insert characters.  In this case, I assume 
> insert at the beginning of the invisible region is a reasonable
> default.

I have to admit that it does work well at the end of a folded headline,
and delete-backward-char there would also work fine.

I would think, when the cursor is in the middle of an invisble regions,
the change should always be denied.

Cheers

- Carsten

> 
> Thanks for the feedback -- let's continue brainstorming, I think
> this feature is important.
> 
> Best,
> 
> -- 
> Bastien

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

* Re: Outline and org-mode don't insert text into folded sections logically
  2011-10-30 15:37                               ` Carsten Dominik
@ 2011-10-31 20:58                                 ` Carsten Dominik
  0 siblings, 0 replies; 25+ messages in thread
From: Carsten Dominik @ 2011-10-31 20:58 UTC (permalink / raw)
  To: Carsten Dominik; +Cc: Bastien, Kelly Dean, emacs-orgmode

Hi Bastien,

more feedback:

I just got bitten by this while editing a file with
visible-mode turned on.  I was trying to add some text, and the code
would continuously jump to some other position.  Took me a while
to figure out.

So I suggest to add a test for

  (or (not (boundp 'visible-mode)) (not visible-mode))

before doing anything.

Also, this made me again think that moving point is not a good idea.

- Carsten

On 30.10.2011, at 16:37, Carsten Dominik wrote:

> 
> On 30.10.2011, at 10:04, Bastien wrote:
> 
>> Hi Carsten,
>> 
>> Carsten Dominik <carsten.dominik@gmail.com> writes:
>> 
>>> - This patch covers only one of many ways to make unwanted changes
>>> in an invisible area.  Others would be delete, backspace,
>>> kill-region, yank, kill-line, and an arbitrarily long list of
>>> less obvious other commands.  Full protection could only be
>>> done with pre-change-hooks or so, but would then prevent
>>> also programmed changes - something that would not be useful.
>> 
>> Yes, I don't want programmed changes to be affected by this feature. 
>> 
>> But having such a warning for `org-delete' would also be useful IMHO.
> 
> I guess you mean, org-delete-char and org-delete-backward-char
> 
>> 
>>> `org-self-insert-command' is probably only ever used in an
>>> interactive way, so the patch as you have written it may very
>>> well function correctly.
>>> 
>>> - All the code in org-self-insert-command is executed for each
>>> keypress, so one needs to be careful to have this function
>>> carry as little overhead as possible.
>> 
>> I actually think there should be a user option
>> `org-edit-invisible-send-warning' defaulting to nil.
> 
> +1
> 
>> 
>> The request "don't let me shoot in my foot" is a common
>> one, and this option would let people set this to `t'.
>> 
>>> - Currently this chokes at the beginning of the buffer because
>>> the invisibility test is also run at (1- (point)).
>> 
>> Fixed, thanks.
>> 
>>> - I am not sure if I understand the positioning code:
>>>> (if (or (eq invisible-before-point 'outline)
>>>> 	(eq invisible-before-point 'org-hide-block))
>>>>   (goto-char (previous-overlay-change (point))))
>>>> (org-cycle)
>>>> (if (or (eq invisible-before-point 'outline)
>>>> 	(eq invisible-before-point 'org-hide-block))
>>>>   (forward-char 1))
>>> 
>>> So when I happen to be somewhere in the middle of invisible
>>> text and press a character, it seems to me that the character
>>> will be inserted at the beginning of the invisible text, and
>>> not where the cursor was.
>>> 
>>> Maybe a better solution would be to save point, unfold,
>>> go back to point, throw and error and not insert the pressed
>>> character.  I am not sure, though.  
>> 
>> Throwing an error and not inserting the text was what my first
>> patch did.  I thought it was too restrictive, though.
>> 
>> With an option `org-edit-invisible-send-warning', we could have both:
>> `t' would just throw a warning, 'prevent would throw an error.
> 
> I like that.
> 
>> 
>>> Maybe you can explain your reasoning?
>> 
>> My reasoning is that, when in the "middle" of an invisible region,
>> the user does not know where the point is, hence he doesn't really 
>> know where he wants to insert characters.  In this case, I assume 
>> insert at the beginning of the invisible region is a reasonable
>> default.
> 
> I have to admit that it does work well at the end of a folded headline,
> and delete-backward-char there would also work fine.
> 
> I would think, when the cursor is in the middle of an invisble regions,
> the change should always be denied.
> 
> Cheers
> 
> - Carsten
> 
>> 
>> Thanks for the feedback -- let's continue brainstorming, I think
>> this feature is important.
>> 
>> Best,
>> 
>> -- 
>> Bastien
> 

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

* Re: Outline and org-mode don't insert text into folded sections logically
  2011-10-30  9:04                             ` Bastien
  2011-10-30 15:37                               ` Carsten Dominik
@ 2011-11-02  7:12                               ` Carsten Dominik
  2011-11-02 10:10                                 ` Bastien
  1 sibling, 1 reply; 25+ messages in thread
From: Carsten Dominik @ 2011-11-02  7:12 UTC (permalink / raw)
  To: Bastien; +Cc: emacs-orgmode, Kelly Dean

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

Hi Bastien,

please find enclosed a patch which is my proposal for rounding
off this feature.

It introduces a variable, applies the check to org-delete-char
and org-delete-backward-char, and removes the (sit-for 1), 
and it never moves the cursor.

I have been unsing it for a day or two, and I like the `smart'
setting of the variable.  Even though, I do agree with your
earlier post that the default should be nil.

- Carsten


[-- Attachment #2: checking-for-invisible-edits.patch --]
[-- Type: application/octet-stream, Size: 6145 bytes --]

diff --git a/lisp/org.el b/lisp/org.el
index 318ccfd..28b9f1c 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -1065,6 +1065,28 @@ OK to kill that hidden subtree.  When nil, kill without remorse."
 	  (const :tag "Protect hidden subtrees with a security query" t)
 	  (const :tag "Never kill a hidden subtree with C-k" error)))
 
+(defcustom org-catch-invisible-edits nil
+  "Check if in invisible region before inserting or deleting a character.
+Valid values are:
+
+nil              Do not check, so just do invisible edits.
+error            Throw an error and do nothing.
+show             Make point visible, and do the requested edit.
+show-and-error   Make point visible, then throw an error and abort the edit.
+smart            Make point visible, and do insertion/deletion if it is
+                 adjacent to visible text and the change feels predictable.
+                 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?"
+  :group 'org-edit-structure
+  :type '(choice
+	  (const :tag "Do not check" nil)
+	  (const :tag "Throw error when trying to edit" error)
+	  (const :tag "Unhide, but do not do the edit" show-and-error)
+	  (const :tag "Show invisible part and do the edit" show)
+	  (const :tag "Be smart and do the right thing" smart)))
+
 (defcustom org-yank-folded-subtrees t
   "Non-nil means when yanking subtrees, fold them.
 If the kill is a single subtree, or a sequence of subtrees, i.e. if
@@ -17401,24 +17423,7 @@ hook.  The default setting is `org-speed-command-default-hook'."
 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")
-  (let ((invisible-at-point
-	 (car (get-char-property-and-overlay (point) 'invisible)))
-	(invisible-before-point
-	 (or (bobp) (car (get-char-property-and-overlay 
-			  (1- (point)) 'invisible)))))
-    (when (or (eq invisible-at-point 'outline)
-	    (eq invisible-at-point 'org-hide-block)
-	    (eq invisible-before-point 'outline)
-	    (eq invisible-before-point 'org-hide-block))
-      (if (or (eq invisible-before-point 'outline)
-	      (eq invisible-before-point 'org-hide-block))
-	  (goto-char (previous-overlay-change (point))))
-      (org-cycle)
-      (if (or (eq invisible-before-point 'outline)
-	      (eq invisible-before-point 'org-hide-block))
-	  (forward-char 1))
-      (message "Unfolding invisible region around point before editing")
-      (sit-for 1)))
+  (org-check-before-invisible-edit 'insert)
   (cond
    ((and org-use-speed-commands
 	 (setq org-speed-command
@@ -17470,6 +17475,53 @@ overwritten, and the table is not marked as requiring realignment."
 	    (setq org-self-insert-command-undo-counter
 		  (1+ org-self-insert-command-undo-counter))))))))
 
+(defun org-check-before-invisible-edit (kind)
+  "Check is editing if kind KIND would be dangerous with invisible text around.
+The detailed reaction depends on the user option `org-catch-invisible-edits'."
+  ;; First, try to get out of here as quickly as possible, to reduce overhead
+  (if (and org-catch-invisible-edits
+	   (or (not (boundp 'visible-mode)) (not visible-mode))
+	   (or (get-char-property (point) 'invisible)
+	       (get-char-property (max (point-min) (1- (point))) 'invisible)))
+      ;; OK, we need to take a closer look
+      (let ((invisible-at-point (get-char-property (point) 'invisible))
+	    (invisible-before-point (if (bobp) nil  (get-char-property 
+						     (1- (point)) 'invisible)))
+	    (border-and-ok-direction
+	     (or
+	      ;; Check if we are acting predictably before invisible text
+	      (and invisible-at-point (not invisible-before-point)
+		   (memq kind '(insert delete-backward)))
+	      ;; Check if we are acting predictably after invisible text
+	      ;; This works not well, and I have turned it off.  It seems
+	      ;; better to always show and stop after invisible text.
+	      ;; (and (not invisible-at-point) invisible-before-point
+	      ;;  (memq kind '(insert delete)))
+	      )))
+
+	(when (or (memq invisible-at-point '(outline org-hide-block))
+		  (memq invisible-before-point '(outline org-hide-block)))
+	  (if (eq org-catch-invisible-edits 'error)
+	      (error "Editing in invisible areas is prohibited - make visible first"))
+	  ;; Make the area visible
+	  (save-excursion
+	    (if invisible-before-point
+		(goto-char (previous-single-char-property-change
+			    (point) 'invisible)))
+	    (org-cycle))
+	  (cond
+	   ((eq org-catch-invisible-edits 'show)
+	    ;; That's it, we do the edit after showing
+	    (message
+	     "Unfolding invisible region around point before editing")
+	    (sit-for 1))
+	   ((and (eq org-catch-invisible-edits 'smart)
+		 border-and-ok-direction)
+	    (message "Unfolding invisible region around point before editing"))
+	   (t
+	    ;; Don't do the edit, make the user repeat it in full visibility
+	    (error "Edit in invisible region aborted, repeat to confirm with text visible")))))))
+
 (defun org-fix-tags-on-the-fly ()
   (when (and (equal (char-after (point-at-bol)) ?*)
 	     (org-on-heading-p))
@@ -17482,6 +17534,7 @@ front of the next \"|\" separator, to keep the table aligned.  The table will
 still be marked for re-alignment if the field did fill the entire column,
 because, in this case the deletion might narrow the column."
   (interactive "p")
+  (org-check-before-invisible-edit 'delete-backward)
   (if (and (org-table-p)
 	   (eq N 1)
 	   (string-match "|" (buffer-substring (point-at-bol) (point)))
@@ -17508,6 +17561,7 @@ front of the next \"|\" separator, to keep the table aligned.  The table will
 still be marked for re-alignment if the field did fill the entire column,
 because, in this case the deletion might narrow the column."
   (interactive "p")
+  (org-check-before-invisible-edit 'delete)
   (if (and (org-table-p)
 	   (not (bolp))
 	   (not (= (char-after) ?|))

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




On 30.10.2011, at 10:04, Bastien wrote:

> Hi Carsten,
> 
> Carsten Dominik <carsten.dominik@gmail.com> writes:
> 
>> - This patch covers only one of many ways to make unwanted changes
>>  in an invisible area.  Others would be delete, backspace,
>>  kill-region, yank, kill-line, and an arbitrarily long list of
>>  less obvious other commands.  Full protection could only be
>>  done with pre-change-hooks or so, but would then prevent
>>  also programmed changes - something that would not be useful.
> 
> Yes, I don't want programmed changes to be affected by this feature. 
> 
> But having such a warning for `org-delete' would also be useful IMHO.
> 
>>  `org-self-insert-command' is probably only ever used in an
>>  interactive way, so the patch as you have written it may very
>>  well function correctly.
>> 
>> - All the code in org-self-insert-command is executed for each
>>  keypress, so one needs to be careful to have this function
>>  carry as little overhead as possible.
> 
> I actually think there should be a user option
> `org-edit-invisible-send-warning' defaulting to nil.
> 
> The request "don't let me shoot in my foot" is a common
> one, and this option would let people set this to `t'.
> 
>> - Currently this chokes at the beginning of the buffer because
>>  the invisibility test is also run at (1- (point)).
> 
> Fixed, thanks.
> 
>> - I am not sure if I understand the positioning code:
>>> (if (or (eq invisible-before-point 'outline)
>>> 	(eq invisible-before-point 'org-hide-block))
>>>    (goto-char (previous-overlay-change (point))))
>>> (org-cycle)
>>> (if (or (eq invisible-before-point 'outline)
>>> 	(eq invisible-before-point 'org-hide-block))
>>>    (forward-char 1))
>> 
>>  So when I happen to be somewhere in the middle of invisible
>>  text and press a character, it seems to me that the character
>>  will be inserted at the beginning of the invisible text, and
>>  not where the cursor was.
>> 
>>  Maybe a better solution would be to save point, unfold,
>>  go back to point, throw and error and not insert the pressed
>>  character.  I am not sure, though.  
> 
> Throwing an error and not inserting the text was what my first
> patch did.  I thought it was too restrictive, though.
> 
> With an option `org-edit-invisible-send-warning', we could have both:
> `t' would just throw a warning, 'prevent would throw an error.
> 
>> Maybe you can explain your reasoning?
> 
> My reasoning is that, when in the "middle" of an invisible region,
> the user does not know where the point is, hence he doesn't really 
> know where he wants to insert characters.  In this case, I assume 
> insert at the beginning of the invisible region is a reasonable
> default.
> 
> Thanks for the feedback -- let's continue brainstorming, I think
> this feature is important.
> 
> Best,
> 
> -- 
> Bastien


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

* Re: Outline and org-mode don't insert text into folded sections logically
  2011-11-02  7:12                               ` Carsten Dominik
@ 2011-11-02 10:10                                 ` Bastien
  2011-11-02 12:53                                   ` Carsten Dominik
  0 siblings, 1 reply; 25+ messages in thread
From: Bastien @ 2011-11-02 10:10 UTC (permalink / raw)
  To: Carsten Dominik; +Cc: emacs-orgmode, Kelly Dean

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

Hi Carsten,

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

> please find enclosed a patch which is my proposal for rounding
> off this feature.

thanks for the clean implementation.

There is a small typo: the second (let ...) should be (let* ...),
see attached updated patch.

> It introduces a variable, applies the check to org-delete-char
> and org-delete-backward-char, and removes the (sit-for 1), 
> and it never moves the cursor.

Okay.  It also applies the check for `org-self-insert-command'.

I like the variable's name.

Nitpicking:

- (setq org-catch-invisible-edits 'show-and-error) is the same than
  (setq org-catch-invisible-edits t), right?  Can we document this
  somewhere, so that users know what to expect from setting the variable
  to t?

- From the docstring: "[...] or add in the middle or right after an
  invisible region" -- I agree inserting while in the middle of the
  invisible region would feel too unpredictable, but I'd argue that
  inserting *right after* the invisible region when point is at the end
  of it feels okay.  Nevermind, maybe this request will come up later,
  I'm fine with your current solution.

> I have been unsing it for a day or two, and I like the `smart'
> setting of the variable.  Even though, I do agree with your
> earlier post that the default should be nil.

Okay.  Please feel free to apply a patch when you want.

Thanks!


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: checking-for-invisible-edits_bzg.patch --]
[-- Type: text/x-patch, Size: 6158 bytes --]

diff --git a/lisp/org.el b/lisp/org.el
index 318ccfd..d9a8db9 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -1065,6 +1065,28 @@ OK to kill that hidden subtree.  When nil, kill without remorse."
 	  (const :tag "Protect hidden subtrees with a security query" t)
 	  (const :tag "Never kill a hidden subtree with C-k" error)))
 
+(defcustom org-catch-invisible-edits nil
+  "Check if in invisible region before inserting or deleting a character.
+Valid values are:
+
+nil              Do not check, so just do invisible edits.
+error            Throw an error and do nothing.
+show             Make point visible, and do the requested edit.
+show-and-error   Make point visible, then throw an error and abort the edit.
+smart            Make point visible, and do insertion/deletion if it is
+                 adjacent to visible text and the change feels predictable.
+                 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?"
+  :group 'org-edit-structure
+  :type '(choice
+	  (const :tag "Do not check" nil)
+	  (const :tag "Throw error when trying to edit" error)
+	  (const :tag "Unhide, but do not do the edit" show-and-error)
+	  (const :tag "Show invisible part and do the edit" show)
+	  (const :tag "Be smart and do the right thing" smart)))
+
 (defcustom org-yank-folded-subtrees t
   "Non-nil means when yanking subtrees, fold them.
 If the kill is a single subtree, or a sequence of subtrees, i.e. if
@@ -17401,24 +17423,7 @@ hook.  The default setting is `org-speed-command-default-hook'."
 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")
-  (let ((invisible-at-point
-	 (car (get-char-property-and-overlay (point) 'invisible)))
-	(invisible-before-point
-	 (or (bobp) (car (get-char-property-and-overlay 
-			  (1- (point)) 'invisible)))))
-    (when (or (eq invisible-at-point 'outline)
-	    (eq invisible-at-point 'org-hide-block)
-	    (eq invisible-before-point 'outline)
-	    (eq invisible-before-point 'org-hide-block))
-      (if (or (eq invisible-before-point 'outline)
-	      (eq invisible-before-point 'org-hide-block))
-	  (goto-char (previous-overlay-change (point))))
-      (org-cycle)
-      (if (or (eq invisible-before-point 'outline)
-	      (eq invisible-before-point 'org-hide-block))
-	  (forward-char 1))
-      (message "Unfolding invisible region around point before editing")
-      (sit-for 1)))
+  (org-check-before-invisible-edit 'insert)
   (cond
    ((and org-use-speed-commands
 	 (setq org-speed-command
@@ -17470,6 +17475,53 @@ overwritten, and the table is not marked as requiring realignment."
 	    (setq org-self-insert-command-undo-counter
 		  (1+ org-self-insert-command-undo-counter))))))))
 
+(defun org-check-before-invisible-edit (kind)
+  "Check is editing if kind KIND would be dangerous with invisible text around.
+The detailed reaction depends on the user option `org-catch-invisible-edits'."
+  ;; First, try to get out of here as quickly as possible, to reduce overhead
+  (if (and org-catch-invisible-edits
+	   (or (not (boundp 'visible-mode)) (not visible-mode))
+	   (or (get-char-property (point) 'invisible)
+	       (get-char-property (max (point-min) (1- (point))) 'invisible)))
+      ;; OK, we need to take a closer look
+      (let* ((invisible-at-point (get-char-property (point) 'invisible))
+	     (invisible-before-point (if (bobp) nil  (get-char-property
+						      (1- (point)) 'invisible)))
+	     (border-and-ok-direction
+	      (or
+	       ;; Check if we are acting predictably before invisible text
+	       (and invisible-at-point (not invisible-before-point)
+		    (memq kind '(insert delete-backward)))
+	       ;; Check if we are acting predictably after invisible text
+	       ;; This works not well, and I have turned it off.  It seems
+	       ;; better to always show and stop after invisible text.
+	       ;; (and (not invisible-at-point) invisible-before-point
+	       ;;  (memq kind '(insert delete)))
+	       )))
+
+	(when (or (memq invisible-at-point '(outline org-hide-block))
+		  (memq invisible-before-point '(outline org-hide-block)))
+	  (if (eq org-catch-invisible-edits 'error)
+	      (error "Editing in invisible areas is prohibited - make visible first"))
+	  ;; Make the area visible
+	  (save-excursion
+	    (if invisible-before-point
+		(goto-char (previous-single-char-property-change
+			    (point) 'invisible)))
+	    (org-cycle))
+	  (cond
+	   ((eq org-catch-invisible-edits 'show)
+	    ;; That's it, we do the edit after showing
+	    (message
+	     "Unfolding invisible region around point before editing")
+	    (sit-for 1))
+	   ((and (eq org-catch-invisible-edits 'smart)
+		 border-and-ok-direction)
+	    (message "Unfolding invisible region around point before editing"))
+	   (t
+	    ;; Don't do the edit, make the user repeat it in full visibility
+	    (error "Edit in invisible region aborted, repeat to confirm with text visible")))))))
+
 (defun org-fix-tags-on-the-fly ()
   (when (and (equal (char-after (point-at-bol)) ?*)
 	     (org-on-heading-p))
@@ -17482,6 +17534,7 @@ front of the next \"|\" separator, to keep the table aligned.  The table will
 still be marked for re-alignment if the field did fill the entire column,
 because, in this case the deletion might narrow the column."
   (interactive "p")
+  (org-check-before-invisible-edit 'delete-backward)
   (if (and (org-table-p)
 	   (eq N 1)
 	   (string-match "|" (buffer-substring (point-at-bol) (point)))
@@ -17508,6 +17561,7 @@ front of the next \"|\" separator, to keep the table aligned.  The table will
 still be marked for re-alignment if the field did fill the entire column,
 because, in this case the deletion might narrow the column."
   (interactive "p")
+  (org-check-before-invisible-edit 'delete)
   (if (and (org-table-p)
 	   (not (bolp))
 	   (not (= (char-after) ?|))

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


-- 
 Bastien

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

* Re: Outline and org-mode don't insert text into folded sections logically
  2011-11-02 10:10                                 ` Bastien
@ 2011-11-02 12:53                                   ` Carsten Dominik
  2011-11-03  1:30                                     ` Bastien
  0 siblings, 1 reply; 25+ messages in thread
From: Carsten Dominik @ 2011-11-02 12:53 UTC (permalink / raw)
  To: Bastien; +Cc: emacs-orgmode, Kelly Dean


On Nov 2, 2011, at 11:10 AM, Bastien wrote:

> Hi Carsten,
> 
> Carsten Dominik <carsten.dominik@gmail.com> writes:
> 
>> please find enclosed a patch which is my proposal for rounding
>> off this feature.
> 
> thanks for the clean implementation.
> 
> There is a small typo: the second (let ...) should be (let* ...),
> see attached updated patch.

OK, I will take a look at it and then push.

> 
>> It introduces a variable, applies the check to org-delete-char
>> and org-delete-backward-char, and removes the (sit-for 1), 
>> and it never moves the cursor.
> 
> Okay.  It also applies the check for `org-self-insert-command'.
> 
> I like the variable's name.
> 
> Nitpicking:
> 
> - (setq org-catch-invisible-edits 'show-and-error) is the same than
>  (setq org-catch-invisible-edits t), right?  Can we document this
>  somewhere, so that users know what to expect from setting the variable
>  to t?


Will do.

> 
> - From the docstring: "[...] or add in the middle or right after an
>  invisible region" -- I agree inserting while in the middle of the
>  invisible region would feel too unpredictable, but I'd argue that
>  inserting *right after* the invisible region when point is at the end
>  of it feels okay.  Nevermind, maybe this request will come up later,
>  I'm fine with your current solution.


I tried, and it confused the hell out of me... :D

> 
>> I have been unsing it for a day or two, and I like the `smart'
>> setting of the variable.  Even though, I do agree with your
>> earlier post that the default should be nil.
> 
> Okay.  Please feel free to apply a patch when you want.
> 
> Thanks!

Cheers

- Carsten

> 
> <checking-for-invisible-edits_bzg.patch>
> -- 
> Bastien

- Carsten

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

* Re: Outline and org-mode don't insert text into folded sections logically
  2011-11-02 12:53                                   ` Carsten Dominik
@ 2011-11-03  1:30                                     ` Bastien
  0 siblings, 0 replies; 25+ messages in thread
From: Bastien @ 2011-11-03  1:30 UTC (permalink / raw)
  To: Carsten Dominik; +Cc: emacs-orgmode, Kelly Dean

Hi Carsten,

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

> OK, I will take a look at it and then push.

Great, thanks!

>> - From the docstring: "[...] or add in the middle or right after an
>>  invisible region" -- I agree inserting while in the middle of the
>>  invisible region would feel too unpredictable, but I'd argue that
>>  inserting *right after* the invisible region when point is at the end
>>  of it feels okay.  Nevermind, maybe this request will come up later,
>>  I'm fine with your current solution.
>
> I tried, and it confused the hell out of me... :D

When I did only try a few hours, so your UX is surely more stable than
mine!  :)

Best,

-- 
 Bastien

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

end of thread, other threads:[~2011-11-03  1:29 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-03 19:14 Outline and org-mode don't insert text into folded sections logically Kelly Dean
2011-09-05  7:52 ` Giovanni Ridolfi
2011-09-05  8:01   ` suvayu ali
2011-10-22 10:13     ` Bastien
2011-10-22 10:17       ` suvayu ali
2011-10-22 10:21       ` Jambunathan K
2011-10-22 13:53         ` Bastien
2011-10-23  2:18           ` suvayu ali
2011-10-29 14:10             ` Bastien
2011-10-29 14:57               ` suvayu ali
2011-10-29 15:40                 ` Bastien
2011-10-29 15:53                   ` suvayu ali
2011-10-29 16:15                     ` Bastien
2011-10-29 16:22                       ` suvayu ali
2011-10-30  1:07                         ` Bastien
2011-10-30  6:28                           ` Carsten Dominik
2011-10-30  7:30                             ` Jambunathan K
2011-10-30  7:47                               ` Carsten Dominik
2011-10-30  9:04                             ` Bastien
2011-10-30 15:37                               ` Carsten Dominik
2011-10-31 20:58                                 ` Carsten Dominik
2011-11-02  7:12                               ` Carsten Dominik
2011-11-02 10:10                                 ` Bastien
2011-11-02 12:53                                   ` Carsten Dominik
2011-11-03  1:30                                     ` Bastien

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).