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: Eli Zaretskii <[email-masked]>
Date: Mon, 03 Mar 2025 18:43:40 +0200

> Date: Mon, 03 Mar 2025 15:54:02 +0000
> From: Pip Cet <[email-masked]>
> Cc: [email-masked], [email-masked], [email-masked], [email-masked], [email-masked], [email-masked]
> 
> "Eli Zaretskii" <[email-masked]> writes:
> 
> > 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;
>     }

I meant this place in display_line, which is where we handle display
iteration that reaches EOL:

      if (ITERATOR_AT_END_OF_LINE_P (it))
	{
	  int used_before = row->used[TEXT_AREA];
	  [...]
	  /* Consume the line end.  This skips over invisible lines.  */
	  set_iterator_to_next (it, true); <<<<<<<<<<<<<<<<<<<<
	  it->continuation_lines_width = 0;
	  break;
	}

As for the call with 'false' as 2nd arg: First, when it->bidi_p is
true, we don't use it->len for moving to the next display element.
And second, this place (and another similar to it) are in functions
that are used to jump far away, so they are always followed by a call
to 'reseat'.