unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: Stephen Berman <stephen.berman@gmx.net>
Cc: pent@aparamon.msk.ru, 2749@debbugs.gnu.org
Subject: bug#2749: 23.0.91; Visual Line Mode: incorrect line wrapping in corner case
Date: Fri, 10 May 2013 20:37:08 +0300	[thread overview]
Message-ID: <83ehdebut7.fsf@gnu.org> (raw)
In-Reply-To: <87wqr66dij.fsf@rosalinde.fritz.box>

> From: Stephen Berman <stephen.berman@gmx.net>
> Cc: monnier@iro.umontreal.ca,  pent@aparamon.msk.ru,  2749@debbugs.gnu.org
> Date: Fri, 10 May 2013 17:49:40 +0200
> 
> > Does the patch below fix the problem (and whatever original problem
> > that led you to this recipe)?
> >
> > --- src/xdisp.c~0	2013-05-10 17:56:44.338000000 +0300
> > +++ src/xdisp.c	2013-05-10 17:57:07.197375000 +0300
> > @@ -8466,7 +8466,8 @@
> >  				   && it->bidi_it.paragraph_dir == R2L)
> >  				  ? WINDOW_LEFT_FRINGE_WIDTH (it->w)
> >  				  : WINDOW_RIGHT_FRINGE_WIDTH (it->w)) == 0
> > -			      || IT_OVERFLOW_NEWLINE_INTO_FRINGE (it))
> > +			      || (IT_OVERFLOW_NEWLINE_INTO_FRINGE (it)
> > +				  && it->line_wrap != WORD_WRAP))
> >  			    {
> >  			      if (!get_next_display_element (it))
> >  				{
> 
> Yes, it fixes the "confusing results" with the patch in my previous
> mail.  Thanks very much!

You are welcome.

> (I wish I understood why this problem occurred and how your patch
> fixes it...)

I didn't really debug it, I just applied Sherlock Holmes's principle:
when you have eliminated all the other possible explanations, the one
that stays is the correct one.

IT_OVERFLOW_NEWLINE_INTO_FRINGE is used in a small number of places.
Most of them are related to actually displaying text, which couldn't
be the reason here, because no text was displayed, only the cursor.
Of the 2 that are left, one was conditioned by it->line_wrap == TRUNCATE,
so again couldn't be the reason.  What's left was the one I fixed, and
the fix just added back the condition you removed from
IT_OVERFLOW_NEWLINE_INTO_FRINGE.

I can give you a vague idea of why the problem occurred.  When you
click the mouse, Emacs needs to determine what buffer position is at
that place, so it could move point there.  The code which determines
buffer position from pixel coordinates simulates the display, starting
at the window beginning and producing glyphs for display (and
discarding them) until it gets to the desired pixel coordinates.  The
code where I made the change is part of that display simulation.  It
assumed without testing that word-wrap can never be on when
IT_OVERFLOW_NEWLINE_INTO_FRINGE is true, which means that you cannot
click outside of text, because text goes to the very end of the
window.  But with the change in IT_OVERFLOW_NEWLINE_INTO_FRINGE, that
assumption was no longer true.

> Unless someone knows of another problem with allowing
> overflow-newline-into-fringe in Visual Line mode, I would like to
> request that both my patch and yours be committed to the trunk

Do you use Org mode or any other modes (e.g., magit) that are heavy
users of display strings?  If not, then I'm not prepared to believe
that the above is the only problem with this change.  I'd like to see
much more reports from users of those modes that this change doesn't
cause any trouble, before I'd agree to installing it.  Just look how
much time it took until this problem popped up.  Since I knew the only
change was in a single macro that is used in a small number of places,
fixing it was a 5-min job.  But imagine that the same would happen 3
years after committing this change, with gazillions of unrelated
changes in the display code since then.

Thanks.





  reply	other threads:[~2013-05-10 17:37 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-03-24 18:04 bug#2749: 23.0.91; Visual Line Mode: incorrect line wrapping in corner case Chong Yidong
2009-03-25  1:35 ` Stefan Monnier
2009-03-22 19:13   ` pent
2013-02-18 12:03     ` Stephen Berman
2013-05-10 14:01       ` Stephen Berman
2013-05-10 14:59         ` Eli Zaretskii
2013-05-10 15:49           ` Stephen Berman
2013-05-10 17:37             ` Eli Zaretskii [this message]
2013-05-10 19:06               ` Stephen Berman
2013-05-10 19:31                 ` Eli Zaretskii
2013-05-18 20:30                   ` Stephen Berman
2013-05-19 15:29                     ` Eli Zaretskii
2013-05-19 16:28                       ` Stephen Berman
2013-05-19 17:38                         ` Eli Zaretskii
2013-05-28 22:26                     ` Stefan Monnier
2013-07-02 15:54                       ` Eli Zaretskii
2013-07-03  9:15                         ` Stefan Monnier
2013-07-03  9:52                         ` Stephen Berman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=83ehdebut7.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=2749@debbugs.gnu.org \
    --cc=pent@aparamon.msk.ru \
    --cc=stephen.berman@gmx.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.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).