GNU bug report logs - #76538
31.0.50; 31.0.50; 31.0.50; feature/igc: using magit-section-cycle-global (S-TAB) and magit-section-toggle (TAB) in some random ways blocks GNU Emacs.

Previous Next

Full log

Message #149 received at 76538 <at> debbugs.gnu.org (full text, mbox):

From: Pip Cet <pipcet <at> protonmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: yantar92 <at> posteo.net, eller.helmut <at> gmail.com, gerd.moellmann <at> gmail.com,
 monnier <at> iro.umontreal.ca, joaomoreira <at> gmx.se, 76538 <at> debbugs.gnu.org
Subject: Re: bug#76538: 31.0.50; 31.0.50; 31.0.50;
 feature/igc: using magit-section-cycle-global (S-TAB) and
 magit-section-toggle (TAB) in some random ways blocks GNU Emacs.
Date: Mon, 03 Mar 2025 15:54:02 +0000

"Eli Zaretskii" <eliz <at> gnu.org> writes:

>> Date: Mon, 03 Mar 2025 14:46:55 +0000
>> From: Pip Cet <pipcet <at> protonmail.com>
>> Cc: gerd.moellmann <at> gmail.com, eller.helmut <at> gmail.com, monnier <at> iro.umontreal.ca, yantar92 <at> posteo.net, joaomoreira <at> gmx.se, 76538 <at> debbugs.gnu.org
>>
>> Should this combination ever be valid?
>>
>> it->what = IT_CHARACTER
>> it->c = 10
>> it->len = 0
>>
>> >> It seems to me like a bug in the xdisp.c code, but it's xdisp.c, so
>> >> who knows?
>> >
>> > I suspect it->len is not used when the character is composed, because
>>
>> The character isn't composed, is it? It's a plain newline.
>
> AFAIR, the value of it->len for a newline is never used, because we
> are at the end of a line, and will then jump to the next visible line
> anyway (or stop iteration altogether).

The way I read set_iterator_to_next:

set_iterator_to_next (struct it *it, bool reseat_p)
{

  if (max_redisplay_ticks > 0)
    update_redisplay_ticks (1, it->w);

  switch (it->method)
    {
    case GET_FROM_BUFFER:
      /* The current display element of IT is a character from
	 current_buffer.  Advance in the buffer, and maybe skip over
	 invisible lines that are so because of selective display.  */
      if (ITERATOR_AT_END_OF_LINE_P (it) && reseat_p)
	reseat_at_next_visible_line_start (it, false);
        ...
      else
        {
	  eassert (it->len != 0);

it depends on the value of reseat_p, which is false in my case, where
it's called from here:

  if (it->what == IT_CHARACTER
      && it->c == '\n'
      && CHARPOS (it->position) == IT_CHARPOS (*it))
    {
      if (it->bidi_p && bidi_it_prev)
	*bidi_it_prev = it->bidi_it;
      set_iterator_to_next (it, false);
      it->c = 0;
      return true;
    }

> Which is not to say I'm against the changes, I'm just trying to
> explain why it never caused us any assertion violations till now.

Thank you very much, I appreciate that!  I too wonder how creating fewer
cache markers somehow causes us to run into a problem that the master
branch (with one marker per 5000 chars) doesn't, and we should try to
understand that before considering whether to skip over long Qinvisible
sections in back_to_previous_line_start etc, IMHO (it makes magit more
usable but I'm not really comfortable suggesting changes to that code
given my current (lack of) understanding).

Pip

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.