[PATCH 1/6] test: run emacs inside screen

[PATCH 1/6] test: run emacs inside screen

[Dmitry Kurochkin]

Before the change, emacs run in daemon mode without any visible
buffers.  Turns out that this affects emacs behavior in some
cases.  In particular, `window-end' function returns `point-max'
instead of the last visible position.  That makes it hard or
impossible to implement some tests.  The patch runs emacs in a
detached screen(1) session.  So that it works exactly as if it
has a visible window.

Note: screen terminates when emacs exits.  So the patch does not
introduce new "running processes left behind" issues.
---
 test/test-lib.sh |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 079d7db..8d7c98d 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -873,10 +873,16 @@ EOF
 test_emacs () {
 	if [ -z "$EMACS_SERVER" ]; then
 		EMACS_SERVER="notmuch-test-suite-$$"
-		"$TMP_DIRECTORY/run_emacs" \
-			--daemon \
+		# start a detached screen session with an emacs server
+		screen -S "$EMACS_SERVER" -d -m "$TMP_DIRECTORY/run_emacs" \
+			--no-window-system \
 			--eval "(setq server-name \"$EMACS_SERVER\")" \
+			--eval '(server-start)' \
 			--eval "(orphan-watchdog $$)" || return
+		# wait until the emacs server is up
+		until test_emacs '()'; do
+			sleep 1
+		done
 	fi
 
 	emacsclient --socket-name="$EMACS_SERVER" --eval "(progn $@)"
-- 
1.7.5.4

[PATCH 2/6] test: do not set frame width in emacs

[Dmitry Kurochkin]

No need for `set-frame-width' in emacs tests since it runs in
screen now.
---
 test/test-lib.el |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/test/test-lib.el b/test/test-lib.el
index a783936..97ae593 100644
--- a/test/test-lib.el
+++ b/test/test-lib.el
@@ -20,9 +20,6 @@
 ;;
 ;; Authors: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>
 
-;; avoid crazy 10-column default of --batch
-(set-frame-width (window-frame (get-buffer-window)) 80)
-
 ;; `read-file-name' by default uses `completing-read' function to read
 ;; user input.  It does not respect `standard-input' variable which we
 ;; use in tests to provide user input.  So replace it with a plain
-- 
1.7.5.4

[PATCH 3/6] test: `notmuch-show-advance-and-archive' with invisible signature

[Dmitry Kurochkin]

Add Emacs test to check that `notmuch-show-advance-and-archive'
works for the last message in thread with invisible signature.
---
 test/emacs |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/test/emacs b/test/emacs
index e59de47..bad1122 100755
--- a/test/emacs
+++ b/test/emacs
@@ -347,4 +347,18 @@ 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 'notmuch-show-advance-and-archive with invisible signature'
+message1='id:20091118010116.GC25380@dottiness.seas.harvard.edu'
+message2='id:1258491078-29658-1-git-send-email-dottedmag@dottedmag.net'
+test_emacs "(notmuch-search \"$message1 or $message2\")
+	    (notmuch-test-wait)
+	    (notmuch-search-show-thread)
+	    (goto-char (point-max))
+	    (redisplay)
+	    (notmuch-show-advance-and-archive)
+	    (test-output)"
+test_emacs "(notmuch-show \"$message2\")
+	    (test-output \"EXPECTED\")"
+test_expect_equal_file OUTPUT EXPECTED
+
 test_done
-- 
1.7.5.4

[PATCH 4/6] emacs: improve hidden signatures handling in notmuch-show-advance-and-archive

[Dmitry Kurochkin]

Use `previous-single-char-property-change' instead of going
through each character by hand and testing it's visibility.  This
fixes `notmuch-show-advance-and-archive' to work for the last
message in thread with hidden signature.
---
 emacs/notmuch-show.el |   17 +++++++++--------
 1 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 6685717..ad3cc7b 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1113,17 +1113,18 @@ thread, (remove the \"inbox\" tag from each message). Also kill
 this buffer, and display the next thread from the search from
 which this thread was originally shown."
   (interactive)
-  (let ((end-of-this-message (notmuch-show-message-bottom)))
+  (let* ((end-of-this-message (notmuch-show-message-bottom))
+	 (visible-end-of-this-message (1- end-of-this-message)))
+    (while (invisible-p visible-end-of-this-message)
+      (setq visible-end-of-this-message
+	    (previous-single-char-property-change visible-end-of-this-message
+						  'invisible)))
     (cond
      ;; Ideally we would test `end-of-this-message' against the result
      ;; of `window-end', but that doesn't account for the fact that
-     ;; the end of the message might be hidden, so we have to actually
-     ;; go to the end, walk back over invisible text and then see if
-     ;; point is visible.
-     ((save-excursion
-	(goto-char (- end-of-this-message 1))
-	(notmuch-show-move-past-invisible-backward)
-	(> (point) (window-end)))
+     ;; the end of the message might be hidden.
+     ((and visible-end-of-this-message
+	   (> visible-end-of-this-message (window-end)))
       ;; The bottom of this message is not visible - scroll.
       (scroll-up nil))
 
-- 
1.7.5.4

[PATCH 5/6] emacs: remove no longer used functions from notmuch-show.el

[Dmitry Kurochkin]

Remove `notmuch-show-move-past-invisible-backward' and
`notmuch-show-move-past-invisible-forward' functions which are
unused.
---
 emacs/notmuch-show.el |    8 --------
 1 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index ad3cc7b..dcaea65 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -977,14 +977,6 @@ All currently available key bindings:
     (notmuch-show-move-to-message-top)
     t))
 
-(defun notmuch-show-move-past-invisible-forward ()
-  (while (point-invisible-p)
-    (forward-char)))
-
-(defun notmuch-show-move-past-invisible-backward ()
-  (while (point-invisible-p)
-    (backward-char)))
-
 ;; Functions relating to the visibility of messages and their
 ;; components.
 
-- 
1.7.5.4

[PATCH 6/6] emacs: remove unused `point-invisible-p' function

[Dmitry Kurochkin]

`point-invisible-p' does not work correctly when `invisible'
property is a list.  There are standard `invisible-p' and related
functions that should be used instead.
---
 emacs/notmuch-lib.el |   15 ---------------
 1 files changed, 0 insertions(+), 15 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index f93c957..0f856bf 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -105,21 +105,6 @@ the user hasn't set this variable with the old or new value."
 
 ;;
 
-;; XXX: This should be a generic function in emacs somewhere, not
-;; here.
-(defun point-invisible-p ()
-  "Return whether the character at point is invisible.
-
-Here visibility is determined by `buffer-invisibility-spec' and
-the invisible property of any overlays for point. It doesn't have
-anything to do with whether point is currently being displayed
-within the current window."
-  (let ((prop (get-char-property (point) 'invisible)))
-    (if (eq buffer-invisibility-spec t)
-	prop
-      (or (memq prop buffer-invisibility-spec)
-	  (assq prop buffer-invisibility-spec)))))
-
 (defun notmuch-remove-if-not (predicate list)
   "Return a copy of LIST with all items not satisfying PREDICATE removed."
   (let (out)
-- 
1.7.5.4

Re: [PATCH 1/6] test: run emacs inside screen

[Jim Paris]

Dmitry Kurochkin wrote:
> Before the change, emacs run in daemon mode without any visible
> buffers.  Turns out that this affects emacs behavior in some
> cases.  In particular, `window-end' function returns `point-max'
> instead of the last visible position.  That makes it hard or
> impossible to implement some tests.  The patch runs emacs in a
> detached screen(1) session.  So that it works exactly as if it
> has a visible window.
> 
> Note: screen terminates when emacs exits.  So the patch does not
> introduce new "running processes left behind" issues.
> ---
>  test/test-lib.sh |   10 ++++++++--
>  1 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index 079d7db..8d7c98d 100755
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -873,10 +873,16 @@ EOF
>  test_emacs () {
>  	if [ -z "$EMACS_SERVER" ]; then
>  		EMACS_SERVER="notmuch-test-suite-$$"
> -		"$TMP_DIRECTORY/run_emacs" \
> -			--daemon \
> +		# start a detached screen session with an emacs server
> +		screen -S "$EMACS_SERVER" -d -m "$TMP_DIRECTORY/run_emacs" \

To avoid problems with ~/.screenrc or /etc/screenrc, you might want e.g.
  env SYSSCREENRC=/dev/null SCREENRC=/dev/null screen -S ...

-jim

> +			--no-window-system \
>  			--eval "(setq server-name \"$EMACS_SERVER\")" \
> +			--eval '(server-start)' \
>  			--eval "(orphan-watchdog $$)" || return
> +		# wait until the emacs server is up
> +		until test_emacs '()'; do
> +			sleep 1
> +		done
>  	fi
>  
>  	emacsclient --socket-name="$EMACS_SERVER" --eval "(progn $@)"
> -- 
> 1.7.5.4
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 1/6] test: run emacs inside screen

[Dmitry Kurochkin]

On Wed, 29 Jun 2011 16:16:40 -0400, Jim Paris <jim@jtan.com> wrote:
> Dmitry Kurochkin wrote:
> > Before the change, emacs run in daemon mode without any visible
> > buffers.  Turns out that this affects emacs behavior in some
> > cases.  In particular, `window-end' function returns `point-max'
> > instead of the last visible position.  That makes it hard or
> > impossible to implement some tests.  The patch runs emacs in a
> > detached screen(1) session.  So that it works exactly as if it
> > has a visible window.
> > 
> > Note: screen terminates when emacs exits.  So the patch does not
> > introduce new "running processes left behind" issues.
> > ---
> >  test/test-lib.sh |   10 ++++++++--
> >  1 files changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/test/test-lib.sh b/test/test-lib.sh
> > index 079d7db..8d7c98d 100755
> > --- a/test/test-lib.sh
> > +++ b/test/test-lib.sh
> > @@ -873,10 +873,16 @@ EOF
> >  test_emacs () {
> >  	if [ -z "$EMACS_SERVER" ]; then
> >  		EMACS_SERVER="notmuch-test-suite-$$"
> > -		"$TMP_DIRECTORY/run_emacs" \
> > -			--daemon \
> > +		# start a detached screen session with an emacs server
> > +		screen -S "$EMACS_SERVER" -d -m "$TMP_DIRECTORY/run_emacs" \
> 
> To avoid problems with ~/.screenrc or /etc/screenrc, you might want e.g.
>   env SYSSCREENRC=/dev/null SCREENRC=/dev/null screen -S ...
> 

Thanks for the suggestion, Jim.  I sent a patch to fix that.

Regards,
  Dmitry

> -jim
> 
> > +			--no-window-system \
> >  			--eval "(setq server-name \"$EMACS_SERVER\")" \
> > +			--eval '(server-start)' \
> >  			--eval "(orphan-watchdog $$)" || return
> > +		# wait until the emacs server is up
> > +		until test_emacs '()'; do
> > +			sleep 1
> > +		done
> >  	fi
> >  
> >  	emacsclient --socket-name="$EMACS_SERVER" --eval "(progn $@)"
> > -- 
> > 1.7.5.4
> > 
> > _______________________________________________
> > notmuch mailing list
> > notmuch@notmuchmail.org
> > http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH] test: avoid using screen(1) configuration files

[Dmitry Kurochkin]

Set SCREENRC and SYSSCREENRC environment variables to "/dev/null"
as suggested by Jim Paris to avoid potential problems with
screen(1) configuration files.
---
 test/test-lib.sh |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 8d7c98d..7cd4832 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -50,6 +50,8 @@ TZ=UTC
 TERM=dumb
 export LANG LC_ALL PAGER TERM TZ
 GIT_TEST_CMP=${GIT_TEST_CMP:-diff -u}
+export SCREENRC=/dev/null
+export SYSSCREENRC=/dev/null
 
 # Protect ourselves from common misconfiguration to export
 # CDPATH into the environment
-- 
1.7.5.4

[PATCH v2 0/7] advance-and-archive bugfix, run emacs inside screen

[Dmitry Kurochkin]

Amended patch series.  Only minor changes:

* redirect errors from test_emacs in the emacs server wait loop to /dev/null
* reordered patches

Regards,
  Dmitry

[PATCH v2 1/7] test: run emacs inside screen

[Dmitry Kurochkin]

Before the change, emacs run in daemon mode without any visible
buffers.  Turns out that this affects emacs behavior in some
cases.  In particular, `window-end' function returns `point-max'
instead of the last visible position.  That makes it hard or
impossible to implement some tests.  The patch runs emacs in a
detached screen(1) session.  So that it works exactly as if it
has a visible window.

Note: screen terminates when emacs exits.  So the patch does not
introduce new "running processes left behind" issues.
---
 test/test-lib.sh |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 22e387e..688598c 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -858,10 +858,16 @@ EOF
 test_emacs () {
 	if [ -z "$EMACS_SERVER" ]; then
 		EMACS_SERVER="notmuch-test-suite-$$"
-		"$TMP_DIRECTORY/run_emacs" \
-			--daemon \
+		# start a detached screen session with an emacs server
+		screen -S "$EMACS_SERVER" -d -m "$TMP_DIRECTORY/run_emacs" \
+			--no-window-system \
 			--eval "(setq server-name \"$EMACS_SERVER\")" \
+			--eval '(server-start)' \
 			--eval "(orphan-watchdog $$)" || return
+		# wait until the emacs server is up
+		until test_emacs '()' 2>/dev/null; do
+			sleep 1
+		done
 	fi
 
 	emacsclient --socket-name="$EMACS_SERVER" --eval "(progn $@)"
-- 
1.7.5.4

[PATCH] test: run emacs inside tmux if screen is not available

[Thomas Jost]

Before this change, the test suite reported many failed tests on machines where
screen is not installed (which is the case of many *BSD systems). This patch
makes the test suite try to use tmux, another terminal multiplexer, if screen is
not available.
---
 test/test-lib.sh |   13 +++++++++++--
 1 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 0608e42..5851b3d 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -860,12 +860,21 @@ EOF
 test_emacs () {
 	if [ -z "$EMACS_SERVER" ]; then
 		EMACS_SERVER="notmuch-test-suite-$$"
-		# start a detached screen session with an emacs server
+		# start a detached screen or tmux session with an emacs server
 		screen -S "$EMACS_SERVER" -d -m "$TMP_DIRECTORY/run_emacs" \
 			--no-window-system \
 			--eval "(setq server-name \"$EMACS_SERVER\")" \
 			--eval '(server-start)' \
-			--eval "(orphan-watchdog $$)" || return
+			--eval "(orphan-watchdog $$)"
+		if [ "$?" != 0 ]
+		then
+			tmux -f /dev/null -L "$EMACS_SERVER" new-session -d "$TMP_DIRECTORY/run_emacs \
+				--no-window-system \
+				--eval \"(setq server-name \\\"$EMACS_SERVER\\\")\" \
+				--eval '(server-start)' \
+				--eval \"(orphan-watchdog $$)\"" || return
+		fi
+
 		# wait until the emacs server is up
 		until test_emacs '()' 2>/dev/null; do
 			sleep 1
-- 
1.7.6

Re: [PATCH] test: run emacs inside tmux if screen is not available

[Dmitry Kurochkin]

Hi Thomas.

On Wed, 20 Jul 2011 14:36:36 +0200, Thomas Jost <schnouki@schnouki.net> wrote:
> Before this change, the test suite reported many failed tests on machines where
> screen is not installed (which is the case of many *BSD systems). This patch
> makes the test suite try to use tmux, another terminal multiplexer, if screen is
> not available.

This is a nice improvement.  But I think we should make it even better :)
There are more terminal emulators besides screen and tmux (dtach comes
to mind).  We need a more general mechanism for trying them.  There
should be a list of commands for running terminal emulators in the order
of preference (I think that is dtach, tmux, screen), and we should try
each one in a loop (this would also avoid repeating the long emacs
command).

Regards,
  Dmitry

> ---
>  test/test-lib.sh |   13 +++++++++++--
>  1 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index 0608e42..5851b3d 100755
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -860,12 +860,21 @@ EOF
>  test_emacs () {
>  	if [ -z "$EMACS_SERVER" ]; then
>  		EMACS_SERVER="notmuch-test-suite-$$"
> -		# start a detached screen session with an emacs server
> +		# start a detached screen or tmux session with an emacs server
>  		screen -S "$EMACS_SERVER" -d -m "$TMP_DIRECTORY/run_emacs" \
>  			--no-window-system \
>  			--eval "(setq server-name \"$EMACS_SERVER\")" \
>  			--eval '(server-start)' \
> -			--eval "(orphan-watchdog $$)" || return
> +			--eval "(orphan-watchdog $$)"
> +		if [ "$?" != 0 ]
> +		then
> +			tmux -f /dev/null -L "$EMACS_SERVER" new-session -d "$TMP_DIRECTORY/run_emacs \
> +				--no-window-system \
> +				--eval \"(setq server-name \\\"$EMACS_SERVER\\\")\" \
> +				--eval '(server-start)' \
> +				--eval \"(orphan-watchdog $$)\"" || return
> +		fi
> +
>  		# wait until the emacs server is up
>  		until test_emacs '()' 2>/dev/null; do
>  			sleep 1
> -- 
> 1.7.6
> 

Re: [PATCH] test: run emacs inside tmux if screen is not available

[Jameson Graef Rollins]

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

On Wed, 20 Jul 2011 17:37:35 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> On Wed, 20 Jul 2011 14:36:36 +0200, Thomas Jost <schnouki@schnouki.net> wrote:
> > Before this change, the test suite reported many failed tests on machines where
> > screen is not installed (which is the case of many *BSD systems). This patch
> > makes the test suite try to use tmux, another terminal multiplexer, if screen is
> > not available.
> 
> This is a nice improvement.  But I think we should make it even better :)
> There are more terminal emulators besides screen and tmux (dtach comes
> to mind).  We need a more general mechanism for trying them.  There
> should be a list of commands for running terminal emulators in the order
> of preference (I think that is dtach, tmux, screen), and we should try
> each one in a loop (this would also avoid repeating the long emacs
> command).

Hi, guys.  I suppose you can go down this route if you want, but I worry
that it just adds a lot of extra code to the test suite that will
ultimately make it less robust.  What's wrong with just depending on
screen for the tests?  I'm sure screen is available on every operating
system that we currently support.

It makes me wary that we would be starting a precedent for making a
bunch of conditionals for all of the possible alternative tools we use
in the test suite.  Maybe that's an overblown concern, though.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH] test: run emacs inside tmux if screen is not available

[Thomas Jost]

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

On Wed, 20 Jul 2011 10:28:38 -0700, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
Non-text part: multipart/signed
> On Wed, 20 Jul 2011 17:37:35 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > On Wed, 20 Jul 2011 14:36:36 +0200, Thomas Jost <schnouki@schnouki.net> wrote:
> > > Before this change, the test suite reported many failed tests on machines where
> > > screen is not installed (which is the case of many *BSD systems). This patch
> > > makes the test suite try to use tmux, another terminal multiplexer, if screen is
> > > not available.
> > 
> > This is a nice improvement.  But I think we should make it even better :)
> > There are more terminal emulators besides screen and tmux (dtach comes
> > to mind).  We need a more general mechanism for trying them.  There
> > should be a list of commands for running terminal emulators in the order
> > of preference (I think that is dtach, tmux, screen), and we should try
> > each one in a loop (this would also avoid repeating the long emacs
> > command).
> 
> Hi, guys.  I suppose you can go down this route if you want, but I worry
> that it just adds a lot of extra code to the test suite that will
> ultimately make it less robust.  What's wrong with just depending on
> screen for the tests?  I'm sure screen is available on every operating
> system that we currently support.
> 
> It makes me wary that we would be starting a precedent for making a
> bunch of conditionals for all of the possible alternative tools we use
> in the test suite.  Maybe that's an overblown concern, though.
> 
> jamie.

You're right, just depending on screen would be enough (the package is
just 864 kB on Arch Linux...), and supporting alternative tools
would make the test suite more complicated and less robust. But the
dependency on screen should be more *explicit*: if it's not available,
the test suite should skip all the emacs tests and display a message
about screen.

Apparently test-lib.sh includes some things about prerequisites. I'll
try to use these to implement the explicit dependency on screen.

Regards,

-- 
Thomas/Schnouki

[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]

Re: [PATCH] test: run emacs inside tmux if screen is not available

[Dmitry Kurochkin]

On Wed, 20 Jul 2011 20:12:55 +0200, Thomas Jost <schnouki@schnouki.net> wrote:
> On Wed, 20 Jul 2011 10:28:38 -0700, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> Non-text part: multipart/signed
> > On Wed, 20 Jul 2011 17:37:35 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > > On Wed, 20 Jul 2011 14:36:36 +0200, Thomas Jost <schnouki@schnouki.net> wrote:
> > > > Before this change, the test suite reported many failed tests on machines where
> > > > screen is not installed (which is the case of many *BSD systems). This patch
> > > > makes the test suite try to use tmux, another terminal multiplexer, if screen is
> > > > not available.
> > > 
> > > This is a nice improvement.  But I think we should make it even better :)
> > > There are more terminal emulators besides screen and tmux (dtach comes
> > > to mind).  We need a more general mechanism for trying them.  There
> > > should be a list of commands for running terminal emulators in the order
> > > of preference (I think that is dtach, tmux, screen), and we should try
> > > each one in a loop (this would also avoid repeating the long emacs
> > > command).
> > 
> > Hi, guys.  I suppose you can go down this route if you want, but I worry
> > that it just adds a lot of extra code to the test suite that will
> > ultimately make it less robust.  What's wrong with just depending on
> > screen for the tests?  I'm sure screen is available on every operating
> > system that we currently support.
> > 
> > It makes me wary that we would be starting a precedent for making a
> > bunch of conditionals for all of the possible alternative tools we use
> > in the test suite.  Maybe that's an overblown concern, though.
> > 
> > jamie.
> 
> You're right, just depending on screen would be enough (the package is
> just 864 kB on Arch Linux...), and supporting alternative tools
> would make the test suite more complicated and less robust. But the
> dependency on screen should be more *explicit*: if it's not available,
> the test suite should skip all the emacs tests and display a message
> about screen.
> 
> Apparently test-lib.sh includes some things about prerequisites. I'll
> try to use these to implement the explicit dependency on screen.
> 

Full ack.  The only reason I did not implement it in the original patch
is because there is an existing patch series [1] that adds proper
prerequisites for gdb, emacs and gpg.  So instead of reimplementing the
same thing, I decided to wait until that series is merged and add screen
prerequisite later.

Regards,
  Dmitry

[1] id:"1307016220-17509-1-git-send-email-pieter@praet.org"

> Regards,
> 
> -- 
> Thomas/Schnouki
Non-text part: application/pgp-signature

[PATCH v2 2/7] test: avoid using screen(1) configuration files

[Dmitry Kurochkin]

Set SCREENRC and SYSSCREENRC environment variables to "/dev/null"
as suggested by Jim Paris to avoid potential problems with
screen(1) configuration files.
---
 test/test-lib.sh |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 688598c..0608e42 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -50,6 +50,8 @@ TZ=UTC
 TERM=dumb
 export LANG LC_ALL PAGER TERM TZ
 GIT_TEST_CMP=${GIT_TEST_CMP:-diff -u}
+export SCREENRC=/dev/null
+export SYSSCREENRC=/dev/null
 
 # Protect ourselves from common misconfiguration to export
 # CDPATH into the environment
-- 
1.7.5.4

[PATCH v2 3/7] test: do not set frame width in emacs

[Dmitry Kurochkin]

No need for `set-frame-width' in emacs tests since it runs in
screen now.
---
 test/test-lib.el |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/test/test-lib.el b/test/test-lib.el
index a783936..97ae593 100644
--- a/test/test-lib.el
+++ b/test/test-lib.el
@@ -20,9 +20,6 @@
 ;;
 ;; Authors: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>
 
-;; avoid crazy 10-column default of --batch
-(set-frame-width (window-frame (get-buffer-window)) 80)
-
 ;; `read-file-name' by default uses `completing-read' function to read
 ;; user input.  It does not respect `standard-input' variable which we
 ;; use in tests to provide user input.  So replace it with a plain
-- 
1.7.5.4

[PATCH v2 4/7] test: `notmuch-show-advance-and-archive' with invisible signature

[Dmitry Kurochkin]

Add Emacs test to check that `notmuch-show-advance-and-archive'
works for the last message in thread with invisible signature.
---
 test/emacs |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/test/emacs b/test/emacs
index 53f455a..708c504 100755
--- a/test/emacs
+++ b/test/emacs
@@ -347,4 +347,18 @@ 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 'notmuch-show-advance-and-archive with invisible signature'
+message1='id:20091118010116.GC25380@dottiness.seas.harvard.edu'
+message2='id:1258491078-29658-1-git-send-email-dottedmag@dottedmag.net'
+test_emacs "(notmuch-search \"$message1 or $message2\")
+	    (notmuch-test-wait)
+	    (notmuch-search-show-thread)
+	    (goto-char (point-max))
+	    (redisplay)
+	    (notmuch-show-advance-and-archive)
+	    (test-output)"
+test_emacs "(notmuch-show \"$message2\")
+	    (test-output \"EXPECTED\")"
+test_expect_equal_file OUTPUT EXPECTED
+
 test_done
-- 
1.7.5.4

Re: [PATCH v2 4/7] test: `notmuch-show-advance-and-archive' with invisible signature

[David Bremner]

On Fri,  1 Jul 2011 08:55:19 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> Add Emacs test to check that `notmuch-show-advance-and-archive'
> works for the last message in thread with invisible signature.

I pushed the first 4 patches in the series. The 5th gave me conflicts
(because it lingered so long no doubt) which I have not had a chance to
track down.

d

Re: [PATCH v2 4/7] test: `notmuch-show-advance-and-archive' with invisible signature

[Dmitry Kurochkin]

Hi David.

On Sat, 05 Nov 2011 22:06:47 -0300, David Bremner <david@tethera.net> wrote:
> On Fri,  1 Jul 2011 08:55:19 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > Add Emacs test to check that `notmuch-show-advance-and-archive'
> > works for the last message in thread with invisible signature.
> 
> I pushed the first 4 patches in the series. The 5th gave me conflicts
> (because it lingered so long no doubt) which I have not had a chance to
> track down.
> 

Thanks.  Following is a rebased series of the remaining 4 patches.

Regards,
  Dmitry

> d

[PATCH v3 1/4] test: `notmuch-show-advance-and-archive' with invisible signature

[Dmitry Kurochkin]

Add Emacs test to check that `notmuch-show-advance-and-archive'
works for the last message in thread with invisible signature.
---
 test/emacs |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/test/emacs b/test/emacs
index 0303d7d..1aa5299 100755
--- a/test/emacs
+++ b/test/emacs
@@ -368,6 +368,20 @@ test_emacs "(notmuch-show \"id:$id\") \
             (test-visible-output \"EXPECTED\")"
 test_expect_equal_file OUTPUT EXPECTED
 
+test_begin_subtest 'notmuch-show-advance-and-archive with invisible signature'
+message1='id:20091118010116.GC25380@dottiness.seas.harvard.edu'
+message2='id:1258491078-29658-1-git-send-email-dottedmag@dottedmag.net'
+test_emacs "(notmuch-search \"$message1 or $message2\")
+	    (notmuch-test-wait)
+	    (notmuch-search-show-thread)
+	    (goto-char (point-max))
+	    (redisplay)
+	    (notmuch-show-advance-and-archive)
+	    (test-output)"
+test_emacs "(notmuch-show \"$message2\")
+	    (test-output \"EXPECTED\")"
+test_expect_equal_file OUTPUT EXPECTED
+
 test_begin_subtest "Refresh show buffer"
 test_emacs '(notmuch-show "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail.gmail.com")
 	    (test-visible-output "EXPECTED")
-- 
1.7.7.1

[PATCH v3 2/4] emacs: improve hidden signatures handling in notmuch-show-advance-and-archive

[Dmitry Kurochkin]

Use `previous-single-char-property-change' instead of going
through each character by hand and testing it's visibility.  This
fixes `notmuch-show-advance-and-archive' to work for the last
message in thread with hidden signature.
---
 emacs/notmuch-show.el |   17 +++++++++--------
 1 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 440e089..b70dbfb 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1152,17 +1152,18 @@ thread, (remove the \"inbox\" tag from each message). Also kill
 this buffer, and display the next thread from the search from
 which this thread was originally shown."
   (interactive)
-  (let ((end-of-this-message (notmuch-show-message-bottom)))
+  (let* ((end-of-this-message (notmuch-show-message-bottom))
+	 (visible-end-of-this-message (1- end-of-this-message)))
+    (while (invisible-p visible-end-of-this-message)
+      (setq visible-end-of-this-message
+	    (previous-single-char-property-change visible-end-of-this-message
+						  'invisible)))
     (cond
      ;; Ideally we would test `end-of-this-message' against the result
      ;; of `window-end', but that doesn't account for the fact that
-     ;; the end of the message might be hidden, so we have to actually
-     ;; go to the end, walk back over invisible text and then see if
-     ;; point is visible.
-     ((save-excursion
-	(goto-char (- end-of-this-message 1))
-	(notmuch-show-move-past-invisible-backward)
-	(> (point) (window-end)))
+     ;; the end of the message might be hidden.
+     ((and visible-end-of-this-message
+	   (> visible-end-of-this-message (window-end)))
       ;; The bottom of this message is not visible - scroll.
       (scroll-up nil))
 
-- 
1.7.7.1

[PATCH v3 3/4] emacs: remove no longer used functions from notmuch-show.el

[Dmitry Kurochkin]

Remove `notmuch-show-move-past-invisible-backward' and
`notmuch-show-move-past-invisible-forward' functions which are
unused.
---
 emacs/notmuch-show.el |    8 --------
 1 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index b70dbfb..aaf69f5 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1016,14 +1016,6 @@ All currently available key bindings:
     (notmuch-show-move-to-message-top)
     t))
 
-(defun notmuch-show-move-past-invisible-forward ()
-  (while (point-invisible-p)
-    (forward-char)))
-
-(defun notmuch-show-move-past-invisible-backward ()
-  (while (point-invisible-p)
-    (backward-char)))
-
 ;; Functions relating to the visibility of messages and their
 ;; components.
 
-- 
1.7.7.1

[PATCH v3 4/4] emacs: remove unused `point-invisible-p' function

[Dmitry Kurochkin]

`point-invisible-p' does not work correctly when `invisible'
property is a list.  There are standard `invisible-p' and related
functions that should be used instead.
---
 emacs/notmuch-lib.el |   15 ---------------
 1 files changed, 0 insertions(+), 15 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index f93c957..0f856bf 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -105,21 +105,6 @@ the user hasn't set this variable with the old or new value."
 
 ;;
 
-;; XXX: This should be a generic function in emacs somewhere, not
-;; here.
-(defun point-invisible-p ()
-  "Return whether the character at point is invisible.
-
-Here visibility is determined by `buffer-invisibility-spec' and
-the invisible property of any overlays for point. It doesn't have
-anything to do with whether point is currently being displayed
-within the current window."
-  (let ((prop (get-char-property (point) 'invisible)))
-    (if (eq buffer-invisibility-spec t)
-	prop
-      (or (memq prop buffer-invisibility-spec)
-	  (assq prop buffer-invisibility-spec)))))
-
 (defun notmuch-remove-if-not (predicate list)
   "Return a copy of LIST with all items not satisfying PREDICATE removed."
   (let (out)
-- 
1.7.7.1

Re: [PATCH v2 4/7] test: `notmuch-show-advance-and-archive' with invisible signature

[David Bremner]

On Sun, 06 Nov 2011 13:02:36 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > I pushed the first 4 patches in the series. The 5th gave me conflicts
> > (because it lingered so long no doubt) which I have not had a chance to
> > track down.

> Thanks.  Following is a rebased series of the remaining 4 patches.

I pushed all four, with the modification of marking the test in patch 1
broken initially, and removing that in the next commit.

d

[PATCH v2 5/7] emacs: improve hidden signatures handling in notmuch-show-advance-and-archive

[Dmitry Kurochkin]

Use `previous-single-char-property-change' instead of going
through each character by hand and testing it's visibility.  This
fixes `notmuch-show-advance-and-archive' to work for the last
message in thread with hidden signature.
---
 emacs/notmuch-show.el |   17 +++++++++--------
 1 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 6685717..ad3cc7b 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1113,17 +1113,18 @@ thread, (remove the \"inbox\" tag from each message). Also kill
 this buffer, and display the next thread from the search from
 which this thread was originally shown."
   (interactive)
-  (let ((end-of-this-message (notmuch-show-message-bottom)))
+  (let* ((end-of-this-message (notmuch-show-message-bottom))
+	 (visible-end-of-this-message (1- end-of-this-message)))
+    (while (invisible-p visible-end-of-this-message)
+      (setq visible-end-of-this-message
+	    (previous-single-char-property-change visible-end-of-this-message
+						  'invisible)))
     (cond
      ;; Ideally we would test `end-of-this-message' against the result
      ;; of `window-end', but that doesn't account for the fact that
-     ;; the end of the message might be hidden, so we have to actually
-     ;; go to the end, walk back over invisible text and then see if
-     ;; point is visible.
-     ((save-excursion
-	(goto-char (- end-of-this-message 1))
-	(notmuch-show-move-past-invisible-backward)
-	(> (point) (window-end)))
+     ;; the end of the message might be hidden.
+     ((and visible-end-of-this-message
+	   (> visible-end-of-this-message (window-end)))
       ;; The bottom of this message is not visible - scroll.
       (scroll-up nil))
 
-- 
1.7.5.4

Re: [PATCH v2 5/7] emacs: improve hidden signatures handling in notmuch-show-advance-and-archive

[Jani Nikula]

On Fri,  1 Jul 2011 08:55:20 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> Use `previous-single-char-property-change' instead of going
> through each character by hand and testing it's visibility.  This
> fixes `notmuch-show-advance-and-archive' to work for the last
> message in thread with hidden signature.

Hi, this is still broken in master, did this patch fall between the
cracks or something?

BR,
Jani.

Re: [PATCH v2 5/7] emacs: improve hidden signatures handling in notmuch-show-advance-and-archive

[Dmitry Kurochkin]

On Wed, 07 Sep 2011 09:13:22 +0000, Jani Nikula <jani@nikula.org> wrote:
> On Fri,  1 Jul 2011 08:55:20 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > Use `previous-single-char-property-change' instead of going
> > through each character by hand and testing it's visibility.  This
> > fixes `notmuch-show-advance-and-archive' to work for the last
> > message in thread with hidden signature.
> 
> Hi, this is still broken in master, did this patch fall between the
> cracks or something?
> 

It was not applied to master.  Just like many other patches.

Regards,
  Dmitry

> BR,
> Jani.

[PATCH v2 6/7] emacs: remove no longer used functions from notmuch-show.el

[Dmitry Kurochkin]

Remove `notmuch-show-move-past-invisible-backward' and
`notmuch-show-move-past-invisible-forward' functions which are
unused.
---
 emacs/notmuch-show.el |    8 --------
 1 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index ad3cc7b..dcaea65 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -977,14 +977,6 @@ All currently available key bindings:
     (notmuch-show-move-to-message-top)
     t))
 
-(defun notmuch-show-move-past-invisible-forward ()
-  (while (point-invisible-p)
-    (forward-char)))
-
-(defun notmuch-show-move-past-invisible-backward ()
-  (while (point-invisible-p)
-    (backward-char)))
-
 ;; Functions relating to the visibility of messages and their
 ;; components.
 
-- 
1.7.5.4

[PATCH v2 7/7] emacs: remove unused `point-invisible-p' function

[Dmitry Kurochkin]

`point-invisible-p' does not work correctly when `invisible'
property is a list.  There are standard `invisible-p' and related
functions that should be used instead.
---
 emacs/notmuch-lib.el |   15 ---------------
 1 files changed, 0 insertions(+), 15 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index f93c957..0f856bf 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -105,21 +105,6 @@ the user hasn't set this variable with the old or new value."
 
 ;;
 
-;; XXX: This should be a generic function in emacs somewhere, not
-;; here.
-(defun point-invisible-p ()
-  "Return whether the character at point is invisible.
-
-Here visibility is determined by `buffer-invisibility-spec' and
-the invisible property of any overlays for point. It doesn't have
-anything to do with whether point is currently being displayed
-within the current window."
-  (let ((prop (get-char-property (point) 'invisible)))
-    (if (eq buffer-invisibility-spec t)
-	prop
-      (or (memq prop buffer-invisibility-spec)
-	  (assq prop buffer-invisibility-spec)))))
-
 (defun notmuch-remove-if-not (predicate list)
   "Return a copy of LIST with all items not satisfying PREDICATE removed."
   (let (out)
-- 
1.7.5.4