bug#24633: highlight-region func using (window-hscroll) in :align-to spec can cause inf loop

bug#24633: highlight-region func using (window-hscroll) in :align-to spec can cause inf loop

[npostavs]

[-- Attachment #1: Type: text/plain, Size: 1002 bytes --]

Version: 25.1 24.5

This is sort of a continuation from Bug #21468 (see
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=21468#46 and the
following).  The overlay with newline approach did indeed turn up some
more problems, so we tried moving to the :overline :underline text
method for Emacs 25, but it turns out this causes Emacs to freeze in
certain circumstances (see
https://github.com/magit/magit/issues/2758#issuecomment-250838301).

There is some sentiment to abandon the horizontal bar effect and settle
for just applying a face, but I think this is a bug in Emacs that should
be fixed regardless.  Or maybe it's just a case of "if it hurts, dont do
it"? (using (window-hscroll) in :align-to specs, that is)

-----------------------

With the code below as overlay-bars.el, run

    emacs -Q overlay-bars.el -l overlay-bars.el

then move point to the end of the long line with all the semicolons, hit
C-SPC, and then C-n.  Emacs gets stuck in an infinite loop that cannot
be interrupted by C-g.


[-- Attachment #2: demo code --]
[-- Type: text/plain, Size: 1338 bytes --]

(require 'cl-lib)

(defvar-local 21468-region-overlays nil)

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;;

(defun 21468-update-hunk-region (start end window rol)
  (mapc #'delete-overlay 21468-region-overlays)
  (cl-flet ((ov (start end &rest args)
                (let ((ov (make-overlay start end nil t)))
                  (while args (overlay-put ov (pop args) (pop args)))
                  (push ov 21468-region-overlays)
                  ov)))
    (let* ((align (list 'space :align-to
                        `(+ ,(window-hscroll) (0 . right))))
           (rend-line (save-excursion (goto-char end)
                                      (cons (line-beginning-position)
                                            (line-end-position))))
           (face (list :underline (face-background 'highlight nil t))))
      (message "align = %S" align)
      (ov (car rend-line) (cdr rend-line) 'face face
          'after-string (propertize "\s" 'face face 'display align 'cursor t)))))

(setq-local redisplay-highlight-region-function '21468-update-hunk-region)

(setq-local redisplay-unhighlight-region-function
            (lambda (rol) (mapc #'delete-overlay 21468-region-overlays)))

(setq-local truncate-lines t)

[-- Attachment #3: Type: text/plain, Size: 2648 bytes --]


If the frame is split vertically with C-x 2 first, then Emacs doesn't
get stuck, but does the equivalent of (scroll-left 4) about once per
second; this can be interrupted by C-g.

Modifying the :align-to expression to `(+ ,(1- (window-hscroll)) (0
. right)) causes this assertion failure after some looping:

xdisp.c:19474: Emacs fatal error: assertion failed: row->pixel_width >= 0

Thread 1 "emacs" hit Breakpoint 1, terminate_due_to_signal (sig=6, backtrace_limit=2147483647)
    at emacs.c:354
354	  signal (sig, SIG_DFL);
(gdb) bt
#0  terminate_due_to_signal (sig=6, backtrace_limit=2147483647) at emacs.c:354
#1  0x00000000005fb0aa in die (msg=0x6f64c2 "row->pixel_width >= 0", file=0x6f3450 "xdisp.c", 
    line=19474) at alloc.c:7223
#2  0x0000000000479b64 in compute_line_metrics (it=0x7fffffff9110) at xdisp.c:19474
#3  0x0000000000481e07 in display_line (it=0x7fffffff9110) at xdisp.c:21234
#4  0x0000000000471b56 in try_window (window=21548085, pos=..., flags=1) at xdisp.c:17246
#5  0x000000000046e6ae in redisplay_window (window=21548085, just_this_one_p=true) at xdisp.c:16695
#6  0x00000000004665a3 in redisplay_window_1 (window=21548085) at xdisp.c:14494
#7  0x000000000061b092 in internal_condition_case_1 (bfun=0x466561 <redisplay_window_1>, 
    arg=21548085, handlers=14582547, hfun=0x4664db <redisplay_window_error>) at eval.c:1338
#8  0x00000000004657c2 in redisplay_internal () at xdisp.c:14119
#9  0x0000000000462ca2 in redisplay () at xdisp.c:13254
#10 0x00000000005768ea in read_char (commandflag=1, map=26212659, prev_event=0, 
    used_mouse_menu=0x7fffffffe36f, end_time=0x0) at keyboard.c:2477
#11 0x00000000005866f7 in read_key_sequence (keybuf=0x7fffffffe520, bufsize=30, prompt=0, 
    dont_downcase_last=false, can_return_switch_frame=true, fix_current_buffer=true, 
    prevent_redisplay=false) at keyboard.c:9063
#12 0x000000000057386d in command_loop_1 () at keyboard.c:1365
#13 0x000000000061aff8 in internal_condition_case (bfun=0x573431 <command_loop_1>, handlers=19056, 
    hfun=0x572ac3 <cmd_error>) at eval.c:1314
#14 0x0000000000573073 in command_loop_2 (ignore=0) at keyboard.c:1107
#15 0x000000000061a5c7 in internal_catch (tag=45888, func=0x57304a <command_loop_2>, arg=0)
    at eval.c:1079
#16 0x0000000000573015 in command_loop () at keyboard.c:1086
#17 0x00000000005725b3 in recursive_edit_1 () at keyboard.c:692
#18 0x00000000005727b3 in Frecursive_edit () at keyboard.c:763
#19 0x0000000000570560 in main (argc=5, argv=0x7fffffffe9b8) at emacs.c:1626

Lisp Backtrace:
"redisplay_internal (C function)" (0x0)

I see the same failure *without* modifying the :align-to expression on
Emacs 24.5.

bug#24633: highlight-region func using (window-hscroll) in :align-to spec can cause inf loop

[Eli Zaretskii]

> From: npostavs@users.sourceforge.net
> Date: Thu, 06 Oct 2016 21:01:32 -0400
> 
> With the code below as overlay-bars.el, run
> 
>     emacs -Q overlay-bars.el -l overlay-bars.el
> 
> then move point to the end of the long line with all the semicolons, hit
> C-SPC, and then C-n.  Emacs gets stuck in an infinite loop that cannot
> be interrupted by C-g.
> 
> (require 'cl-lib)
> 
> (defvar-local 21468-region-overlays nil)
> 
> ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
> ;;
> 
> (defun 21468-update-hunk-region (start end window rol)
>   (mapc #'delete-overlay 21468-region-overlays)
>   (cl-flet ((ov (start end &rest args)
>                 (let ((ov (make-overlay start end nil t)))
>                   (while args (overlay-put ov (pop args) (pop args)))
>                   (push ov 21468-region-overlays)
>                   ov)))
>     (let* ((align (list 'space :align-to
>                         `(+ ,(window-hscroll) (0 . right))))
>            (rend-line (save-excursion (goto-char end)
>                                       (cons (line-beginning-position)
>                                             (line-end-position))))
>            (face (list :underline (face-background 'highlight nil t))))
>       (message "align = %S" align)
>       (ov (car rend-line) (cdr rend-line) 'face face
>           'after-string (propertize "\s" 'face face 'display align 'cursor t)))))
> 
> (setq-local redisplay-highlight-region-function '21468-update-hunk-region)
> 
> (setq-local redisplay-unhighlight-region-function
>             (lambda (rol) (mapc #'delete-overlay 21468-region-overlays)))
> 
> (setq-local truncate-lines t)

Can you explain what do you expect this to do, and how/why?  It's hard
to reverse-engineer this to glean the intent.  In particular, I don't
understand the align-to expression: e.g., window-hscroll returns its
value in columns, while align-to needs pixels, AFAIU.  (But this is
not the only thing I don't understand about this, so please provide a
higher-level overview as well.)

What I see in the debugger is that the display engine loops
indefinitely, each time increasing the window's hscroll by 4 columns.
IOW, the redisplay cycle never stops.

Thanks.

bug#24633: highlight-region func using (window-hscroll) in :align-to spec can cause inf loop

[npostavs]

Eli Zaretskii <eliz@gnu.org> writes:

>
> Can you explain what do you expect this to do, and how/why?  It's hard
> to reverse-engineer this to glean the intent.

The high-level idea is to have a line from going from the left to the
right edge of the window.  Most of the line is composed of an overlay
specifying the :underline text property.  The last bit of it, from the
end of the text to the right edge of the screen, is created by an
underlined after-string space which uses :align-to to reach the right
edge of the screen.

Using :align-to 'right or :align-to `((,(window-body-width nil t))) does
not account for horizontal scrolling (i.e., the line ends at 80 columns,
even when the 80th column is scrolled to the left).  So I tried

    `(+ ,(window-hscroll) (0 . right))
    `(+ ,(window-hscroll) (,(window-body-width nil t))) ; initially this

to make up for it, which turns out to cause this strange looping.

>  In particular, I don't
> understand the align-to expression: e.g., window-hscroll returns its
> value in columns, while align-to needs pixels, AFAIU.

According to `(elisp) Pixel Specification',

       The form NUM specifies a fraction of the default frame font height
    or width.  The form `(NUM)' specifies an absolute number of pixels.

I interpreted "fraction of the default frame font" to mean that the
result of window-hscroll would be multiplied by the font width (it's a
plain number outside a list).

I tried now also `(+ (,(window-hscroll) . width) (0 . right)), which
gives the same result.

    The `width' and `height' units correspond to the default width and
    height of the current face.
    [...]
    A value of the form `(NUM . EXPR)' stands for the product of the
    values of NUM and EXPR.

By the way, I also tried `(+ (,(window-hscroll) . width)
(,(window-body-width nil t))) which again gives the same result.

>
> What I see in the debugger is that the display engine loops
> indefinitely, each time increasing the window's hscroll by 4 columns.
> IOW, the redisplay cycle never stops.
>
> Thanks.

bug#24633: highlight-region func using (window-hscroll) in :align-to spec can cause inf loop

[Eli Zaretskii]

> From: npostavs@users.sourceforge.net
> Cc: 24633@debbugs.gnu.org
> Date: Sat, 08 Oct 2016 13:17:37 -0400
> 
> The high-level idea is to have a line from going from the left to the
> right edge of the window.  Most of the line is composed of an overlay
> specifying the :underline text property.  The last bit of it, from the
> end of the text to the right edge of the screen, is created by an
> underlined after-string space which uses :align-to to reach the right
> edge of the screen.
> 
> Using :align-to 'right or :align-to `((,(window-body-width nil t))) does
> not account for horizontal scrolling (i.e., the line ends at 80 columns,
> even when the 80th column is scrolled to the left).  So I tried
> 
>     `(+ ,(window-hscroll) (0 . right))
>     `(+ ,(window-hscroll) (,(window-body-width nil t))) ; initially this
> 
> to make up for it, which turns out to cause this strange looping.

Automatic hscrolling is based on length of point's line at the end of
redisplaying a window.  Here, "line length" means the horizontal space
required for displaying all the glyphs of the line, which includes the
glyphs that come from the after-string.  Since you extend your
after-string each time the window is hscrolled, you in effect enlarge
the line each time the display engine hscrolls the window, which
causes an endless loop.

Can you restrict the after-string's length so it always ends at the
last character of the line, or some fixed number of columns afterward?

> >  In particular, I don't
> > understand the align-to expression: e.g., window-hscroll returns its
> > value in columns, while align-to needs pixels, AFAIU.
> 
> According to `(elisp) Pixel Specification',
> 
>        The form NUM specifies a fraction of the default frame font height
>     or width.  The form `(NUM)' specifies an absolute number of pixels.

I admire your courage in reading that documentation and then writing
stuff like the above, which the documentation doesn't mention even
remotely.

bug#24633: highlight-region func using (window-hscroll) in :align-to spec can cause inf loop

[npostavs]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: npostavs@users.sourceforge.net
>> Cc: 24633@debbugs.gnu.org
>> Date: Sat, 08 Oct 2016 13:17:37 -0400
>> 
>> The high-level idea is to have a line from going from the left to the
>> right edge of the window.  Most of the line is composed of an overlay
>> specifying the :underline text property.  The last bit of it, from the
>> end of the text to the right edge of the screen, is created by an
>> underlined after-string space which uses :align-to to reach the right
>> edge of the screen.
>> 
>> Using :align-to 'right or :align-to `((,(window-body-width nil t))) does
>> not account for horizontal scrolling (i.e., the line ends at 80 columns,
>> even when the 80th column is scrolled to the left).  So I tried
>> 
>>     `(+ ,(window-hscroll) (0 . right))
>>     `(+ ,(window-hscroll) (,(window-body-width nil t))) ; initially this
>> 
>> to make up for it, which turns out to cause this strange looping.
>
> Automatic hscrolling is based on length of point's line at the end of
> redisplaying a window.  Here, "line length" means the horizontal space
> required for displaying all the glyphs of the line, which includes the
> glyphs that come from the after-string.  Since you extend your
> after-string each time the window is hscrolled, you in effect enlarge
> the line each time the display engine hscrolls the window, which
> causes an endless loop.
>
> Can you restrict the after-string's length so it always ends at the
> last character of the line, or some fixed number of columns afterward?

Then it won't reach the edge of the screen when the buffer is scrolled.
I guess what I would expect is that the automatic hscrolling only goes
as far as the cursor is rendered, which should be the end of text where
I put the `cursor' property.  If you think this would be too
complex/slow to implement, we can mark this bug as wontfix.

>
>> >  In particular, I don't
>> > understand the align-to expression: e.g., window-hscroll returns its
>> > value in columns, while align-to needs pixels, AFAIU.
>> 
>> According to `(elisp) Pixel Specification',
>> 
>>        The form NUM specifies a fraction of the default frame font height
>>     or width.  The form `(NUM)' specifies an absolute number of pixels.
>
> I admire your courage in reading that documentation and then writing
> stuff like the above, which the documentation doesn't mention even
> remotely.

Uh, not sure how to read this, is it irony?

bug#24633: highlight-region func using (window-hscroll) in :align-to spec can cause inf loop

[Eli Zaretskii]

> From: npostavs@users.sourceforge.net
> Cc: 24633@debbugs.gnu.org
> Date: Sat, 08 Oct 2016 15:18:26 -0400
> 
> > Automatic hscrolling is based on length of point's line at the end of
> > redisplaying a window.  Here, "line length" means the horizontal space
> > required for displaying all the glyphs of the line, which includes the
> > glyphs that come from the after-string.  Since you extend your
> > after-string each time the window is hscrolled, you in effect enlarge
> > the line each time the display engine hscrolls the window, which
> > causes an endless loop.
> >
> > Can you restrict the after-string's length so it always ends at the
> > last character of the line, or some fixed number of columns afterward?
> 
> Then it won't reach the edge of the screen when the buffer is scrolled.

Yes, but is it so bad?

> I guess what I would expect is that the automatic hscrolling only goes
> as far as the cursor is rendered, which should be the end of text where
> I put the `cursor' property.

The implementation doesn't actually use the cursor position (it can't:
by the time hscroll_windows runs the cursor is not yet in its final
horizontal position, only its vertical position is correct, because
the horizontal scrolling was not yet done).  The implementation uses a
function that simulates redisplay, and that one doesn't pay attention
to the 'cursor' property.  So it stops only when the after-string is
exhausted.

> If you think this would be too complex/slow to implement, we can
> mark this bug as wontfix.

It'd be messy, yes.  We'd need to search for before- and after-strings
that begin/end right at point, then augment the call to the display
emulating function accordingly, deal with complications like strings
that include newlines, etc.

> >> >  In particular, I don't
> >> > understand the align-to expression: e.g., window-hscroll returns its
> >> > value in columns, while align-to needs pixels, AFAIU.
> >> 
> >> According to `(elisp) Pixel Specification',
> >> 
> >>        The form NUM specifies a fraction of the default frame font height
> >>     or width.  The form `(NUM)' specifies an absolute number of pixels.
> >
> > I admire your courage in reading that documentation and then writing
> > stuff like the above, which the documentation doesn't mention even
> > remotely.
> 
> Uh, not sure how to read this, is it irony?

Only a little.  I find this area severely under-documented.

bug#24633: highlight-region func using (window-hscroll) in :align-to spec can cause inf loop

[npostavs]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: npostavs@users.sourceforge.net
>> Cc: 24633@debbugs.gnu.org
>> Date: Sat, 08 Oct 2016 15:18:26 -0400
>> 
>> > Automatic hscrolling is based on length of point's line at the end of
>> > redisplaying a window.  Here, "line length" means the horizontal space
>> > required for displaying all the glyphs of the line, which includes the
>> > glyphs that come from the after-string.  Since you extend your
>> > after-string each time the window is hscrolled, you in effect enlarge
>> > the line each time the display engine hscrolls the window, which
>> > causes an endless loop.
>> >
>> > Can you restrict the after-string's length so it always ends at the
>> > last character of the line, or some fixed number of columns afterward?
>> 
>> Then it won't reach the edge of the screen when the buffer is scrolled.
>
> Yes, but is it so bad?

Hmm, it's not quite the effect I was going for, but actually maybe it
isn't that bad.

> It'd be messy, yes.  We'd need to search for before- and after-strings
> that begin/end right at point, then augment the call to the display
> emulating function accordingly, deal with complications like strings
> that include newlines, etc.

Anyway, it doesn't seem worth going through this complexity.  I just
wonder if there is some way to stop bad lisp code from triggering a hard
lockup.  Can the display engine notice if it's looping and throw some
kind of error?  Maybe unset pre-redisplay-functions?

>
>> >> >  In particular, I don't
>> >> > understand the align-to expression: e.g., window-hscroll returns its
>> >> > value in columns, while align-to needs pixels, AFAIU.
>> >> 
>> >> According to `(elisp) Pixel Specification',
>> >> 
>> >>        The form NUM specifies a fraction of the default frame font height
>> >>     or width.  The form `(NUM)' specifies an absolute number of pixels.
>> >
>> > I admire your courage in reading that documentation and then writing
>> > stuff like the above, which the documentation doesn't mention even
>> > remotely.
>> 
>> Uh, not sure how to read this, is it irony?
>
> Only a little.  I find this area severely under-documented.

The grammar in the doc seems complete to me.  Although using "fraction
of" to mean "gets multiplied by" is perhaps a bit unintuitive.

bug#24633: highlight-region func using (window-hscroll) in :align-to spec can cause inf loop

[Eli Zaretskii]

> From: npostavs@users.sourceforge.net
> Cc: 24633@debbugs.gnu.org
> Date: Sun, 09 Oct 2016 08:29:51 -0400
> 
> Anyway, it doesn't seem worth going through this complexity.  I just
> wonder if there is some way to stop bad lisp code from triggering a hard
> lockup.  Can the display engine notice if it's looping and throw some
> kind of error?  Maybe unset pre-redisplay-functions?

I don't see how we could detect loops in general.  But for the
particular case of infinite hscrolling, we could perhaps count the
number of times hscroll_windows was called and returned a non-zero
value, and forcibly stop the loop after some reasonable number of
iterations.

Patches welcome.  Could be a nice small project for someone who wants
to gain practice with hacking the display engine.

> >> >> According to `(elisp) Pixel Specification',
> >> >> 
> >> >>        The form NUM specifies a fraction of the default frame font height
> >> >>     or width.  The form `(NUM)' specifies an absolute number of pixels.
> >> >
> >> > I admire your courage in reading that documentation and then writing
> >> > stuff like the above, which the documentation doesn't mention even
> >> > remotely.
> >> 
> >> Uh, not sure how to read this, is it irony?
> >
> > Only a little.  I find this area severely under-documented.
> 
> The grammar in the doc seems complete to me.

Do you really think that a formal grammar, whether accurate/complete
or not, is a good way of describing a feature?

bug#24633: highlight-region func using (window-hscroll) in :align-to spec can cause inf loop

[npostavs]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: npostavs@users.sourceforge.net
>> Cc: 24633@debbugs.gnu.org
>> Date: Sun, 09 Oct 2016 08:29:51 -0400
>> 
>> Anyway, it doesn't seem worth going through this complexity.  I just
>> wonder if there is some way to stop bad lisp code from triggering a hard
>> lockup.  Can the display engine notice if it's looping and throw some
>> kind of error?  Maybe unset pre-redisplay-functions?
>
> I don't see how we could detect loops in general.  But for the
> particular case of infinite hscrolling, we could perhaps count the
> number of times hscroll_windows was called and returned a non-zero
> value, and forcibly stop the loop after some reasonable number of
> iterations.

I tried adding a counter in redisplay_internal, it prevents having a
tight loop in C.  Actually, the behaviour becomes like the split window
case: there is additional hscrolling each time the cursor blinks, but it
can be interrupted with C-g.  That seems adequate behaviour to me, what
do you think?

diff --git c/src/xdisp.c i/src/xdisp.c
index ba6518b..3bf3fd8 100644
--- c/src/xdisp.c
+++ i/src/xdisp.c
@@ -13528,6 +13528,9 @@ redisplay_internal (void)
   bool polling_stopped_here = false;
   Lisp_Object tail, frame;
 
+  enum { MAX_HSCROLL_RETRIES = 16 };
+  int hscroll_retries = 0;
+
   /* True means redisplay has to consider all windows on all
      frames.  False, only selected_window is considered.  */
   bool consider_all_windows_p;
@@ -14044,8 +14047,12 @@ redisplay_internal (void)
 		  if (!f->already_hscrolled_p)
 		    {
 		      f->already_hscrolled_p = true;
-		      if (hscroll_windows (f->root_window))
-			goto retry_frame;
+                      if (hscroll_retries <= MAX_HSCROLL_RETRIES
+                          && hscroll_windows (f->root_window))
+                        {
+                          hscroll_retries++;
+                          goto retry_frame;
+                        }
 		    }
 
 		  /* If the frame's redisplay flag was not set before
@@ -14143,8 +14150,12 @@ redisplay_internal (void)
 
       if (FRAME_VISIBLE_P (sf) && !FRAME_OBSCURED_P (sf))
 	{
-	  if (hscroll_windows (selected_window))
-	    goto retry;
+          if (hscroll_retries <= MAX_HSCROLL_RETRIES
+              && hscroll_windows (selected_window))
+            {
+              hscroll_retries++;
+              goto retry;
+            }
 
 	  XWINDOW (selected_window)->must_be_updated_p = true;
 	  pending = update_frame (sf, false, false);
@@ -14164,8 +14175,12 @@ redisplay_internal (void)
 	  XWINDOW (mini_window)->must_be_updated_p = true;
 	  pending |= update_frame (mini_frame, false, false);
 	  mini_frame->cursor_type_changed = false;
-	  if (!pending && hscroll_windows (mini_window))
-	    goto retry;
+          if (!pending && hscroll_retries <= MAX_HSCROLL_RETRIES
+              && hscroll_windows (mini_window))
+            {
+              hscroll_retries++;
+              goto retry;
+            }
 	}
     }



>> >> >> According to `(elisp) Pixel Specification',
>> >> >> 
>> >> >>        The form NUM specifies a fraction of the default frame font height
>> >> >>     or width.  The form `(NUM)' specifies an absolute number of pixels.
>> >> >
>> >> > I admire your courage in reading that documentation and then writing
>> >> > stuff like the above, which the documentation doesn't mention even
>> >> > remotely.
>> >> 
>> >> Uh, not sure how to read this, is it irony?
>> >
>> > Only a little.  I find this area severely under-documented.
>> 
>> The grammar in the doc seems complete to me.
>
> Do you really think that a formal grammar, whether accurate/complete
> or not, is a good way of describing a feature?

The formal grammar plus the informal description of what the parts mean
seems a perfectly fine description for _this_ feature.

bug#24633: highlight-region func using (window-hscroll) in :align-to spec can cause inf loop

[Eli Zaretskii]

> From: npostavs@users.sourceforge.net
> Cc: 24633@debbugs.gnu.org
> Date: Sat, 22 Oct 2016 15:27:42 -0400
> 
> I tried adding a counter in redisplay_internal, it prevents having a
> tight loop in C.  Actually, the behaviour becomes like the split window
> case: there is additional hscrolling each time the cursor blinks, but it
> can be interrupted with C-g.  That seems adequate behaviour to me, what
> do you think?

Did you see what happens when blink-cursor-mode is turned off?  (If
that's what you meant by "each time the cursor blinks; if not, what
caused it to blink?)

The patch looks OK to me, so if it gives better results than locking
up Emacs with the current code, let's push it to master.

> > Do you really think that a formal grammar, whether accurate/complete
> > or not, is a good way of describing a feature?
> 
> The formal grammar plus the informal description of what the parts mean
> seems a perfectly fine description for _this_ feature.

Not to me, not anywhere close to being fine.  For starters, Emacs Lisp
programmers don't have to be proficient in reading formal grammars.
But that's me.

Thanks.

bug#24633: highlight-region func using (window-hscroll) in :align-to spec can cause inf loop

[npostavs]

tags 24633 fixed
close 24633 26
quit

Eli Zaretskii <eliz@gnu.org> writes:

>> From: npostavs@users.sourceforge.net
>> Cc: 24633@debbugs.gnu.org
>> Date: Sat, 22 Oct 2016 15:27:42 -0400
>> 
>> I tried adding a counter in redisplay_internal, it prevents having a
>> tight loop in C.  Actually, the behaviour becomes like the split window
>> case: there is additional hscrolling each time the cursor blinks, but it
>> can be interrupted with C-g.  That seems adequate behaviour to me, what
>> do you think?
>
> Did you see what happens when blink-cursor-mode is turned off?  (If
> that's what you meant by "each time the cursor blinks; if not, what
> caused it to blink?)

If I turn blink-cursor-mode off, then it stops scrolling after what
seems like immediately (though I can see from the message calls that the
highlight-region function was called several times).  So the counter is
limiting redisplay looping as expected.

>
> The patch looks OK to me, so if it gives better results than locking
> up Emacs with the current code, let's push it to master.

Okay, pushed as 241ae7a1 "Avoid infinite hscrolling in redisplay", and
closing the bug as fixed.