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.

From: Pip Cet <[email-masked]>
Date: Mon, 03 Mar 2025 15:54:02 +0000

"Eli Zaretskii" <[email-masked]> writes:

>> Date: Mon, 03 Mar 2025 14:46:55 +0000
>> From: Pip Cet <[email-masked]>
>> Cc: [email-masked], [email-masked], [email-masked], [email-masked], [email-masked], [email-masked]
>>
>> 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