[PATCH 1/2] test: add emacs test for hiding message when point is at invisible text

[PATCH 1/2] test: add emacs test for hiding message when point is at invisible text

[Dmitry Kurochkin]

Human-friendly scenario:

* open a thread which has at least 2 messages in notmuch-show view
* hide the first message
* move to the first line of the second message
* press C-a (bound to `beginning-of-visual-line')
* press RET (bound to `notmuch-show-toggle-message')

Result: the first message is shown
Expected result: the second message is hidden or shown

Currently the test is failing.  The bug will be fixed by a subsequent
patch.
---
 test/emacs |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/test/emacs b/test/emacs
index 53f455a..173d371 100755
--- a/test/emacs
+++ b/test/emacs
@@ -347,4 +347,15 @@ test_emacs '(notmuch-show "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail.
 	    (test-visible-output)'
 test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-with-hidden-messages
 
+test_begin_subtest 'Hiding message when point is at invisible text'
+test_emacs '(notmuch-show "id:1258471718-6781-2-git-send-email-dottedmag@dottedmag.net")
+	    (notmuch-show-toggle-message)
+	    (test-visible-output "EXPECTED")
+	    (notmuch-show-toggle-message)
+	    (goto-char (previous-single-char-property-change
+			(point) '\''invisible))
+	    (notmuch-show-toggle-message)
+	    (test-visible-output)'
+test_expect_equal_file OUTPUT EXPECTED
+
 test_done
-- 
1.7.5.4

[PATCH 2/2] emacs: skip forward to visible text in notmuch-show-message-extent

[Dmitry Kurochkin]

The patch rewrites `notmuch-show-message-extent' to be more
robust.  The main goal is to make it work as expected if point is
invisible.  Besides, there are no more point movements and
property search functions are used instead manual loops.  The
comment regarding properties strangeness is removed since there
is no strangeness here: property ranges (as well as overlay, and
many others, I believe) are given as [begin, end), not [begin,
end].
---
 emacs/notmuch-show.el |   19 +++++++++++--------
 1 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index f96743b..cf8b405 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -933,15 +933,18 @@ All currently available key bindings:
 
 ;; Movement related functions.
 
-;; There's some strangeness here where a text property applied to a
-;; region a->b is not found when point is at b. We walk backwards
-;; until finding the property.
 (defun notmuch-show-message-extent ()
-  (let (r)
-    (save-excursion
-      (while (not (setq r (get-text-property (point) :notmuch-message-extent)))
-	(backward-char)))
-    r))
+  (let ((p (point)))
+    ;; if point is invisible, skip forward to visible text
+    (while (invisible-p p)
+      (setq p (next-single-char-property-change p 'invisible)))
+    ;; if no visible text found, use the point
+    (or p (setq p (point)))
+    (or (get-text-property p :notmuch-message-extent)
+	;; if there is no text property, skip to the previous message
+	(and (setq p (previous-single-char-property-change
+		      p :notmuch-message-extent))
+	     (get-text-property p :notmuch-message-extent)))))
 
 (defun notmuch-show-message-top ()
   (car (notmuch-show-message-extent)))
-- 
1.7.5.4

Re: [PATCH 2/2] emacs: skip forward to visible text in notmuch-show-message-extent

[Pieter Praet]

On Sun,  3 Jul 2011 08:28:06 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> The patch rewrites `notmuch-show-message-extent' to be more
> robust.  The main goal is to make it work as expected if point is
> invisible.  Besides, there are no more point movements and
> property search functions are used instead manual loops.  The
> comment regarding properties strangeness is removed since there
> is no strangeness here: property ranges (as well as overlay, and
> many others, I believe) are given as [begin, end), not [begin,
> end].
> ---
>  emacs/notmuch-show.el |   19 +++++++++++--------
>  1 files changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index f96743b..cf8b405 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -933,15 +933,18 @@ All currently available key bindings:
>  
>  ;; Movement related functions.
>  
> -;; There's some strangeness here where a text property applied to a
> -;; region a->b is not found when point is at b. We walk backwards
> -;; until finding the property.
>  (defun notmuch-show-message-extent ()
> -  (let (r)
> -    (save-excursion
> -      (while (not (setq r (get-text-property (point) :notmuch-message-extent)))
> -	(backward-char)))
> -    r))
> +  (let ((p (point)))
> +    ;; if point is invisible, skip forward to visible text
> +    (while (invisible-p p)
> +      (setq p (next-single-char-property-change p 'invisible)))
> +    ;; if no visible text found, use the point
> +    (or p (setq p (point)))
> +    (or (get-text-property p :notmuch-message-extent)
> +	;; if there is no text property, skip to the previous message
> +	(and (setq p (previous-single-char-property-change
> +		      p :notmuch-message-extent))
> +	     (get-text-property p :notmuch-message-extent)))))
>  
>  (defun notmuch-show-message-top ()
>    (car (notmuch-show-message-extent)))
> -- 
> 1.7.5.4
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Signed-off-by: Pieter Praet <pieter@praet.org>

Though it's worth noting that (as a previous test of yours [1] demonstrates),
it doesn't work consistently when the previous message has a text/html part:
complains with "No URL at point".

Peace


-- 
Pieter

[1] id:"1309752441-10651-4-git-send-email-dmitry.kurochkin@gmail.com"

Re: [PATCH 2/2] emacs: skip forward to visible text in notmuch-show-message-extent

[Dmitry Kurochkin]

On Sun,  3 Jul 2011 08:28:06 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> The patch rewrites `notmuch-show-message-extent' to be more
> robust.  The main goal is to make it work as expected if point is
> invisible.  Besides, there are no more point movements and
> property search functions are used instead manual loops.  The
> comment regarding properties strangeness is removed since there
> is no strangeness here: property ranges (as well as overlay, and
> many others, I believe) are given as [begin, end), not [begin,
> end].

Please do not apply this patch.  I am considering a different solution:
instead of making the trailing newline invisible, make the preceding
newline invisible.  This way a message would always have a visible
newline at the end and it would prevent `beginning-of-visual-line' going
beyond the beginning of message.

FYI there is a discussion on emacs-devel [1] regarding point adjustment
inside invisible regions.

Regards,
  Dmitry

[1] http://thread.gmane.org/gmane.emacs.devel/141479

> ---
>  emacs/notmuch-show.el |   19 +++++++++++--------
>  1 files changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index f96743b..cf8b405 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -933,15 +933,18 @@ All currently available key bindings:
>  
>  ;; Movement related functions.
>  
> -;; There's some strangeness here where a text property applied to a
> -;; region a->b is not found when point is at b. We walk backwards
> -;; until finding the property.
>  (defun notmuch-show-message-extent ()
> -  (let (r)
> -    (save-excursion
> -      (while (not (setq r (get-text-property (point) :notmuch-message-extent)))
> -	(backward-char)))
> -    r))
> +  (let ((p (point)))
> +    ;; if point is invisible, skip forward to visible text
> +    (while (invisible-p p)
> +      (setq p (next-single-char-property-change p 'invisible)))
> +    ;; if no visible text found, use the point
> +    (or p (setq p (point)))
> +    (or (get-text-property p :notmuch-message-extent)
> +	;; if there is no text property, skip to the previous message
> +	(and (setq p (previous-single-char-property-change
> +		      p :notmuch-message-extent))
> +	     (get-text-property p :notmuch-message-extent)))))
>  
>  (defun notmuch-show-message-top ()
>    (car (notmuch-show-message-extent)))
> -- 
> 1.7.5.4
>