From: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>
To: David Edmondson <dme@dme.org>, notmuch@notmuchmail.org
Subject: Re: [PATCH 4/4 v2] emacs: Take more care when hiding regions with
	buttons.
In-Reply-To: <1327506328-22126-5-git-send-email-dme@dme.org>
References: <1327503908-17226-1-git-send-email-dme@dme.org>
	<1327506328-22126-1-git-send-email-dme@dme.org>
	<1327506328-22126-5-git-send-email-dme@dme.org>
User-Agent: Notmuch/0.11+116~ge6e10b8 (http://notmuchmail.org) Emacs/23.3.1
	(x86_64-pc-linux-gnu)
Date: Wed, 25 Jan 2012 21:53:54 +0400
Message-ID: <87ipjzhdn1.fsf@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list

On Wed, 25 Jan 2012 15:45:28 +0000, David Edmondson <dme@dme.org> wrote:
> If the region to be hidden with a button by
> `notmuch-wash-region-to-button' starts at the beginning of the buffer,
> the invisible region will include the inserted button. This is
> unfortunate, as it means that it is not possible to see the button to
> be pressed.
> 
> Make a little space at the start of the buffer before inserting the
> button to avoid this, not forgetting to remove the inserted space upon
> completion.
> 
> Mark the relevant test as no longer broken.
> ---

I will try to find some time to play with the test and look for
alternative solutions.

Regards,
  Dmitry

>  emacs/notmuch-wash.el                 |   57 ++++++++++++++++++++++-----------
>  test/emacs-original-message-hiding.sh |    1 -
>  2 files changed, 38 insertions(+), 20 deletions(-)
> 
> diff --git a/emacs/notmuch-wash.el b/emacs/notmuch-wash.el
> index 5c1e830..4afd3b3 100644
> --- a/emacs/notmuch-wash.el
> +++ b/emacs/notmuch-wash.el
> @@ -147,25 +147,44 @@ insert before the button, probably for indentation."
>    ;; symbols because of the way the button code works. Note that
>    ;; replacing intern-soft with make-symbol will cause this to fail,
>    ;; since the newly created symbol has no plist.
> -
> -  (let ((overlay (make-overlay beg end))
> -	(message-invis-spec (plist-get msg :message-invis-spec))
> -	(invis-spec (make-symbol (concat "notmuch-" type "-region")))
> -	(button-type (intern-soft (concat "notmuch-wash-button-"
> -					  type "-toggle-type"))))
> -    (add-to-invisibility-spec invis-spec)
> -    (overlay-put overlay 'invisible (list invis-spec message-invis-spec))
> -    (overlay-put overlay 'isearch-open-invisible #'notmuch-wash-region-isearch-show)
> -    (overlay-put overlay 'priority 10)
> -    (overlay-put overlay 'type type)
> -    (goto-char (1+ end))
> -    (save-excursion
> -      (goto-char (1- beg))
> -      (insert prefix)
> -      (insert-button (notmuch-wash-button-label overlay)
> -		     'invisibility-spec invis-spec
> -		     'overlay overlay
> -		     :type button-type))))
> +  (save-excursion
> +    ;; If the beginning of the region to be converted to a button is the
> +    ;; beginning of the buffer we must move forward a little to avoid
> +    ;; creating an overlay that will hide the button intended to be used
> +    ;; to reveal the hidden region.
> +    (let (scene-of-crime)
> +      (when (eq beg (point-min))
> +	(goto-char (point-min))
> +	(insert "\n")
> +	(setq scene-of-crime (point-min)
> +	      beg (point)))
> +
> +      ;; This uses some slightly tricky conversions between strings and
> +      ;; symbols because of the way the button code works. Note that
> +      ;; replacing intern-soft with make-symbol will cause this to fail,
> +      ;; since the newly created symbol has no plist.
> +
> +      (let ((overlay (make-overlay beg end))
> +	    (message-invis-spec (plist-get msg :message-invis-spec))
> +	    (invis-spec (make-symbol (concat "notmuch-" type "-region")))
> +	    (button-type (intern-soft (concat "notmuch-wash-button-"
> +					      type "-toggle-type"))))
> +	(add-to-invisibility-spec invis-spec)
> +	(overlay-put overlay 'invisible (list invis-spec message-invis-spec))
> +	(overlay-put overlay 'isearch-open-invisible #'notmuch-wash-region-isearch-show)
> +	(overlay-put overlay 'priority 10)
> +	(overlay-put overlay 'type type)
> +
> +	(goto-char (1- beg))
> +	(insert prefix)
> +	(insert-button (notmuch-wash-button-label overlay)
> +		       'invisibility-spec invis-spec
> +		       'overlay overlay
> +		       :type button-type))
> +
> +      (when scene-of-crime
> +      	(goto-char scene-of-crime)
> +      	(delete-char 1)))))
>  
>  (defun notmuch-wash-excerpt-citations (msg depth)
>    "Excerpt citations and up to one signature."
> diff --git a/test/emacs-original-message-hiding.sh b/test/emacs-original-message-hiding.sh
> index 01cf98d..038ef4f 100755
> --- a/test/emacs-original-message-hiding.sh
> +++ b/test/emacs-original-message-hiding.sh
> @@ -13,7 +13,6 @@ add_message \
>      '[body]="-----Original Message-----
>  Text here.
>  "'
> -test_subtest_known_broken
>  test_emacs_expect_t \
>      '(load "emacs-original-message-hiding.el") (notmuch-test-original-message-hiding)'
>  
> -- 
> 1.7.8.3
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch