Date: Thu, 10 Jan 2013 17:39:13 -0500
From: Austin Clements <amdragon@MIT.EDU>
To: notmuch@notmuchmail.org
Subject: Re: [PATCH] emacs: Fix point motion in `beginning-of-visual-line'
Message-ID: <20130110223836.GV17581@mit.edu>
References: <1357852215-16788-1-git-send-email-markwalters1009@gmail.com>
	<1357855176-31653-1-git-send-email-amdragon@mit.edu>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1357855176-31653-1-git-send-email-amdragon@mit.edu>
User-Agent: Mutt/1.5.21 (2010-09-15)
Precedence: list

After some IRC discussion, Mark and I have found that my patch doesn't
fix everything related to w3m that Mark's patch does (though my patch
does fix other things not related to w3m).

Quoth myself on Jan 10 at  4:59 pm:
> `beginning-of-visual-line' interacts poorly with our use of invisible
> overlays and often moves point into the previous message or part even
> though it looks like it's in the next message or part (e.g., if you
> press C-a on a header line and the previous message is invisible,
> point will move into the previous message, even though it appears to
> still be on the header line).  This advises `beginning-of-visual-line'
> to address this behavior.
> ---
> 
> This is a completely different approach that should fix the HTML
> problem, as well as other problems.  I also have an alternate approach
> to this that sets 'field properties on the header line and part
> buttons, which has the same effect, but seems to have more
> side-effects (e.g., if point is at the end of a part button, C-a won't
> go to the beginning).  This patch, OTOH, strikes directly at the
> strange (maybe even buggy) behavior of beginning-of-visual-line.  The
> downside is that it's advice, and advice always feels a little
> sketchy.
> 
>  emacs/notmuch-show.el |   22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 5751d98..6f5c53a 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -1120,6 +1120,28 @@ function is used."
>      (notmuch-show-goto-first-wanted-message)
>      (current-buffer)))
>  
> +(defadvice beginning-of-visual-line (around constrain-invisible activate)
> +  "Fix point motion around invisible overlays.
> +
> +In `notmuch-show-mode', we often hide overlays that end with a
> +newline (e.g., messages, parts, etc).  This has the effect of
> +collapsing the overlayed text into the beginning of the next
> +line.  Unfortunately, this causes `visual-motion' and hence
> +`beginning-of-visual-line' to move to the first character of the
> +invisible overlay rather than the more obvious first character of
> +the visible line.  Visually, these two points are
> +indistinguishable, but this tends to make actions mysteriously
> +apply to the previous message when it looks like they should
> +apply to the next message.
> +
> +This advice fixes this behavior in `notmuch-show-mode' by
> +restricting the motion of `beginning-of-visual-line'."
> +  (if (eq major-mode 'notmuch-show-mode)
> +      (save-restriction
> +	(narrow-to-region (line-beginning-position) (point-max))
> +	ad-do-it)
> +    ad-do-it))
> +
>  (defun notmuch-show-build-buffer ()
>    (let ((inhibit-read-only t))
>