unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH 1/2] test: add emacs test for hiding message when point is at invisible text
@ 2011-07-03  4:28 Dmitry Kurochkin
  2011-07-03  4:28 ` [PATCH 2/2] emacs: skip forward to visible text in notmuch-show-message-extent Dmitry Kurochkin
  0 siblings, 1 reply; 4+ messages in thread
From: Dmitry Kurochkin @ 2011-07-03  4:28 UTC (permalink / raw)
  To: notmuch

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

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

* [PATCH 2/2] emacs: skip forward to visible text in notmuch-show-message-extent
  2011-07-03  4:28 [PATCH 1/2] test: add emacs test for hiding message when point is at invisible text Dmitry Kurochkin
@ 2011-07-03  4:28 ` Dmitry Kurochkin
  2011-07-04  8:25   ` Pieter Praet
  2011-07-05  9:38   ` Dmitry Kurochkin
  0 siblings, 2 replies; 4+ messages in thread
From: Dmitry Kurochkin @ 2011-07-03  4:28 UTC (permalink / raw)
  To: notmuch

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

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

* Re: [PATCH 2/2] emacs: skip forward to visible text in notmuch-show-message-extent
  2011-07-03  4:28 ` [PATCH 2/2] emacs: skip forward to visible text in notmuch-show-message-extent Dmitry Kurochkin
@ 2011-07-04  8:25   ` Pieter Praet
  2011-07-05  9:38   ` Dmitry Kurochkin
  1 sibling, 0 replies; 4+ messages in thread
From: Pieter Praet @ 2011-07-04  8:25 UTC (permalink / raw)
  To: Dmitry Kurochkin, notmuch

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"

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

* Re: [PATCH 2/2] emacs: skip forward to visible text in notmuch-show-message-extent
  2011-07-03  4:28 ` [PATCH 2/2] emacs: skip forward to visible text in notmuch-show-message-extent Dmitry Kurochkin
  2011-07-04  8:25   ` Pieter Praet
@ 2011-07-05  9:38   ` Dmitry Kurochkin
  1 sibling, 0 replies; 4+ messages in thread
From: Dmitry Kurochkin @ 2011-07-05  9:38 UTC (permalink / raw)
  To: notmuch

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
> 

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

end of thread, other threads:[~2011-07-05  9:38 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-07-03  4:28 [PATCH 1/2] test: add emacs test for hiding message when point is at invisible text Dmitry Kurochkin
2011-07-03  4:28 ` [PATCH 2/2] emacs: skip forward to visible text in notmuch-show-message-extent Dmitry Kurochkin
2011-07-04  8:25   ` Pieter Praet
2011-07-05  9:38   ` Dmitry Kurochkin

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

	https://yhetil.org/notmuch.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).