APPROVE COMMIT NOTE: This patch has been committed # HG changeset patch # User Aidan Kehoe <kehoea(a)parhasard.net&gt; # Date 1552223383 0 # Sun Mar 10 13:09:43 2019 +0000 # Node ID 93f9adafefb31b803e6ed8d5062aa3ce70b46ff4 # Parent cc27eb6f808b9fe3055bd866e521f2646b2556b2 Make last_window_start a zero-length extent; ditto saved window points. src/ChangeLog addition: 2019-03-10 Aidan Kehoe <kehoea(a)parhasard.net&gt; Move the buffer last_window_start to being a zero-length extent, not an int. Move the window saved point to being a zero-length extent also, to give better behaviour with editing and deleting as we saw previously with http://mid.xemacs.org/539b.674c.c8912.0ca7＠parhasard.net . * buffer.c (finish_init_buffer): * buffer.c (Ferase_buffer): Initialise last_window_start as Qnil in both these functions, meaning use the beginning of the visible region within redisplay. * buffer.h (struct buffer): Remove last_window_start from the non-Lisp_Object part of the buffer slots. * bufslots.h: Save it here instead, mark it as a Lisp_Object as appropriate. * window.c (unshow_buffer): Save the last point for this window and the window start as zero-length extents rather than markers. * window.c (Fset_window_buffer): Use the zero-length extents rather than markers or ints. * window.c (delete_saved_window_start): New. Delete a saved window start zero-length extent, unless it is the last_window_start for the associated buffer. * window.c (Fdelete_window): Use this when cleaning up the saved_last_window_start_cache. * winslots.h: Update the documentation for saved_point_cache, saved_last_window_start_cache. diff -r cc27eb6f808b -r 93f9adafefb3 src/ChangeLog --- a/src/ChangeLog Mon Feb 18 14:53:34 2019 +0000 +++ b/src/ChangeLog Sun Mar 10 13:09:43 2019 +0000 ＠＠ -1,3 +1,36 ＠＠ +2019-03-10 Aidan Kehoe <kehoea(a)parhasard.net&gt; + + Move the buffer last_window_start to being a zero-length extent, + not an int. + + Move the window saved point to being a zero-length extent also, to + give better behaviour with editing and deleting as we saw + previously with + http://mid.xemacs.org/539b.674c.c8912.0ca7＠parhasard.net . + + * buffer.c (finish_init_buffer): + * buffer.c (Ferase_buffer): + Initialise last_window_start as Qnil in both these functions, + meaning use the beginning of the visible region within redisplay. + + * buffer.h (struct buffer): Remove last_window_start from the + non-Lisp_Object part of the buffer slots. + * bufslots.h: + Save it here instead, mark it as a Lisp_Object as appropriate. + * window.c (unshow_buffer): + Save the last point for this window and the window start as + zero-length extents rather than markers. + * window.c (Fset_window_buffer): + Use the zero-length extents rather than markers or ints. + * window.c (delete_saved_window_start): New. + Delete a saved window start zero-length extent, unless it is the + last_window_start for the associated buffer. + * window.c (Fdelete_window): + Use this when cleaning up the saved_last_window_start_cache. + * winslots.h: + Update the documentation for saved_point_cache, + saved_last_window_start_cache. + 2019-02-18 Aidan Kehoe <kehoea(a)parhasard.net&gt; Prefer byte_beginning_of_line_p() throughout the C code. diff -r cc27eb6f808b -r 93f9adafefb3 src/buffer.c --- a/src/buffer.c Mon Feb 18 14:53:34 2019 +0000 +++ b/src/buffer.c Sun Mar 10 13:09:43 2019 +0000 ＠＠ -621,7 +621,7 ＠＠ reset_buffer_local_variables (b, 1); b->directory = current_buffer ? current_buffer->directory : Qnil; - b->last_window_start = 1; + b->last_window_start = Qnil; b->name = name; if (string_byte (name, 0) != ' ') ＠＠ -1762,7 +1762,7 ＠＠ widen_buffer (b, no_clip); buffer_delete_range (b, BUF_BEG (b), BUF_Z (b), 0); - b->last_window_start = 1; + b->last_window_start = Qnil; /* Prevent warnings, or suspension of auto saving, that would happen if future size is less than past size. Use of erase-buffer diff -r cc27eb6f808b -r 93f9adafefb3 src/buffer.h --- a/src/buffer.h Mon Feb 18 14:53:34 2019 +0000 +++ b/src/buffer.h Sun Mar 10 13:09:43 2019 +0000 ＠＠ -254,10 +254,6 ＠＠ Or -1 if we didn't have a failure. */ int auto_save_failure_time; - /* Position in buffer at which display started - the last time this buffer was displayed. */ - int last_window_start; - /* Everything from here down must be a Lisp_Object */ #define MARKED_SLOT(x) Lisp_Object x; diff -r cc27eb6f808b -r 93f9adafefb3 src/bufslots.h --- a/src/bufslots.h Mon Feb 18 14:53:34 2019 +0000 +++ b/src/bufslots.h Sun Mar 10 13:09:43 2019 +0000 ＠＠ -233,6 +233,12 ＠＠ /* Last time this buffer was displayed using set-window-buffer. */ MARKED_SLOT (display_time) + /* Position in buffer at which display started the last time this buffer + was displayed, as a zero-length extent or Qnil, if this buffer was + never displayed. It is rare that this is not also reachable through + some window saved_last_window_start_cache hash table. */ + MARKED_SLOT (last_window_start) + /* A hash table that maps from a "generic extent" (an extent in `modeline-format') into a buffer-specific extent. */ MARKED_SLOT (modeline_extent_table) diff -r cc27eb6f808b -r 93f9adafefb3 src/window.c --- a/src/window.c Mon Feb 18 14:53:34 2019 +0000 +++ b/src/window.c Sun Mar 10 13:09:43 2019 +0000 ＠＠ -2047,72 +2047,81 ＠＠ unshow_buffer (struct window *w) { Lisp_Object buf = w->buffer; + Lisp_Object saved_point = Fgethash (buf, w->saved_point_cache, Qnil); + Lisp_Object saved_window_start + = Fgethash (buf, w->saved_last_window_start_cache, Qnil); + Boolint selected = EQ (wrap_window (w), Fselected_window (Qnil)); struct buffer *b = XBUFFER (buf); assert (b == XMARKER (w->pointm[CURRENT_DISP])->buffer); - /* FSF disables this check, so I'll do it too. I hope it won't - break things. --ben */ -#if 0 - if (w == XWINDOW (Fselected_window (Qnil)) - || ! EQ (buf, XWINDOW (Fselected_window (Qnil))->buffer)) - /* Do this except when the selected window's buffer - is being removed from some other window. */ -#endif - /* last_window_start records the start position that this buffer - had in the last window to be disconnected from it. - Now that this statement is unconditional, - it is possible for the buffer to be displayed in the - selected window, while last_window_start reflects another - window which was recently showing the same buffer. - Some people might say that might be a good thing. Let's see. */ - XBUFFER (buf)->last_window_start = - marker_position (w->start[CURRENT_DISP]); - /* Point in the selected window's buffer is actually stored in that buffer, and the window's pointm isn't used. So don't clobber point in that buffer. */ - if (! EQ (buf, XWINDOW (Fselected_window (Qnil))->buffer)) - BUF_SET_PT (b, - charbpos_clip_to_bounds - (BUF_BEGV (b), - marker_position (w->pointm[CURRENT_DISP]), - BUF_ZV (b))); - - { - Lisp_Object marker; - Lisp_Object saved_point = Fgethash (buf, w->saved_point_cache, Qnil); - int selected = EQ (wrap_window (w), Fselected_window (Qnil)); - - if (NILP (saved_point)) - { - saved_point = Fmake_extent (Qnil, Qnil, buf); - Fset_extent_property (saved_point, Qstart_open, Qt); - Fputhash (buf, saved_point, w->saved_point_cache); - } - - if (selected) - { - set_extent_endpoints (XEXTENT (saved_point), - BYTE_BUF_PT (b), BYTE_BUF_PT (b), buf); - } - else - { - set_extent_endpoints (XEXTENT (saved_point), - byte_marker_position (w->pointm[CURRENT_DISP]), - byte_marker_position (w->pointm[CURRENT_DISP]), - buf); - } - - marker = Fgethash (buf, w->saved_last_window_start_cache, Qnil); - - if (NILP (marker)) - { - marker = Fmake_marker (); - Fputhash (buf, marker, w->saved_last_window_start_cache); - } - Fset_marker (marker, w->start[CURRENT_DISP], buf); - } + if (!EQ (buf, XWINDOW (Fselected_window (Qnil))->buffer)) + { + set_marker_restricted (b->point_marker, w->pointm[CURRENT_DISP], buf); + } + + if (NILP (saved_point)) + { + /* This is a start-open extent rather than a marker because of the + behaviour of markers on deletion of text. See + http://mid.xemacs.org/539b.674c.c8912.0ca7＠parhasard.net and the + associated thread. */ + saved_point = Fmake_extent (Qnil, Qnil, buf); + Fset_extent_property (saved_point, Qstart_open, Qt); + Fputhash (buf, saved_point, w->saved_point_cache); + } + + if (selected) + { + set_extent_endpoints (XEXTENT (saved_point), + BYTE_BUF_PT (b), BYTE_BUF_PT (b), buf); + } + else + { + set_extent_endpoints (XEXTENT (saved_point), + byte_marker_position (w->pointm[CURRENT_DISP]), + byte_marker_position (w->pointm[CURRENT_DISP]), + buf); + } + + if (NILP (saved_window_start)) + { + /* This is a start-open extent rather than a marker because of the + behaviour of markers on deletion of text. See + http://mid.xemacs.org/539b.674c.c8912.0ca7＠parhasard.net and the + associated thread. */ + saved_window_start = Fmake_extent (Qnil, Qnil, buf); + Fset_extent_property (saved_window_start, Qstart_open, Qt); + Fputhash (buf, saved_window_start, w->saved_last_window_start_cache); + } + + set_extent_endpoints (XEXTENT (saved_window_start), + byte_marker_position (w->start[CURRENT_DISP]), + byte_marker_position (w->start[CURRENT_DISP]), + buf); + + /* last_window_start records the start position that this buffer + had in the last window to be disconnected from it. + Now that this statement is unconditional, + it is possible for the buffer to be displayed in the + selected window, while last_window_start reflects another + window which was recently showing the same buffer. + Some people might say that might be a good thing. Let's see. + + The above is from Stallman in 1994, and it seems to work well. A more + recent XEmacs change is that last_window_start is now either a + zero-length extent, or Qnil, something which may or may not work + well. (It previously was just an int (not even a Charbpos) reflecting + the marker position of w->start[CURRENT_DISP] when this function was + called.) This costs approximately nothing in terms of memory and + buffer-editing speed, since we re-use the extent that's already in + the window cache. + + Aidan Kehoe, So 10 Mär 2019 12:32:29 GMT */ + XBUFFER (buf)->last_window_start = saved_window_start; } /* Put REPLACEMENT into the window structure in place of OLD. */ ＠＠ -2246,6 +2255,26 ＠＠ return 0; } +static int +delete_saved_window_start (Lisp_Object UNUSED (buffer), + Lisp_Object saved_window_start, + void *UNUSED (closure)) +{ + Lisp_Object obj = Fextent_object (saved_window_start); + + if (BUFFERP (obj) + && EQ (saved_window_start, XBUFFER (obj)->last_window_start)) + { + /* Keep this extent around, it may be helpful the next time the buffer + is shown. */; + } + else + { + Fdelete_extent (saved_window_start); + } + return 0; +} + DEFUN ("delete-window", Fdelete_window, 0, 2, "", /* Remove WINDOW from the display. Default is selected window. If window is the only one on its frame, the frame is deleted as well. ＠＠ -2364,6 +2393,10 ＠＠ is. */ elisp_maphash_unsafe (delete_saved_point, w->saved_point_cache, NULL); + /* Ditto the saved window starts. */ + elisp_maphash_unsafe (delete_saved_window_start, + w->saved_last_window_start_cache, NULL); + /* close up the hole in the sibling list */ if (!NILP (w->next)) XWINDOW (w->next)->prev = w->prev; ＠＠ -3731,9 +3764,10 ＠＠ */ (window, buffer, norecord)) { - Lisp_Object tem; struct window *w = decode_window (window); + Lisp_Object tem, saved_point, saved_window_start; int old_buffer_local_face_property = 0; + Bytebpos bpoint, bstart; buffer = Fget_buffer (buffer); CHECK_BUFFER (buffer); ＠＠ -3761,45 +3795,61 ＠＠ w->window_end_pos[CURRENT_DISP] = 0; w->hscroll = 0; w->modeline_hscroll = 0; -#if 0 /* pre point caches */ - Fset_marker (w->pointm[CURRENT_DISP], - make_fixnum (BUF_PT (XBUFFER (buffer))), - buffer); - set_marker_restricted (w->start[CURRENT_DISP], - make_fixnum (XBUFFER (buffer)->last_window_start), - buffer); -#else - { - Lisp_Object saved_point = Fgethash (buffer, w->saved_point_cache, Qnil); - Lisp_Object newpoint = - (EXTENTP (saved_point) && NILP (Fextent_detached_p (saved_point))) - ? Fextent_start_position (saved_point) - : make_fixnum (BUF_PT (XBUFFER (buffer))); - Lisp_Object marker; - /* Previously, we had in here set-window-point, which did one of the - following two, but not both. However, that could result in pointm - being in a different buffer from the window's buffer! Probably - not a travesty since it always occurred when the window was - selected, meaning its value of point was ignored in favor of the - buffer's; but it tripped an assert() in unshow_buffer(). */ - set_marker_restricted (w->pointm[CURRENT_DISP], newpoint, buffer); - if (EQ (wrap_window (w), Fselected_window (Qnil))) - Fgoto_char (newpoint, buffer); /* this will automatically clip to - accessible */ - marker = Fgethash (buffer, w->saved_last_window_start_cache, Qnil); - set_marker_restricted (w->start[CURRENT_DISP], - !NILP (marker) ? - make_fixnum (marker_position (marker)) : - make_fixnum (XBUFFER (buffer)->last_window_start), - buffer); - } -#endif - - Fset_marker (w->sb_point, w->start[CURRENT_DISP], buffer); + + saved_point = Fgethash (buffer, w->saved_point_cache, Qnil); + bpoint = (EXTENTP (saved_point) && NILP (Fextent_detached_p (saved_point))) + ? extent_endpoint_byte (XEXTENT (saved_point), 0) + : BYTE_BUF_PT (XBUFFER (buffer)); + + /* Adjust this so we don't have to call set_marker_restricted(). */ + bpoint = bytebpos_clip_to_bounds (BYTE_BUF_BEGV (XBUFFER (buffer)), + bpoint, + BYTE_BUF_ZV (XBUFFER (buffer))); + + structure_checking_assert (!EXTENTP (saved_point) + || EQ (Fextent_object (saved_point), buffer)); + + /* Previously, we had in here set-window-point, which did one of the + following two, but not both. However, that could result in pointm being + in a different buffer from the window's buffer! Probably not a travesty + since it always occurred when the window was selected, meaning its value + of point was ignored in favor of the buffer's; but it tripped an assert() + in unshow_buffer(). */ + set_byte_marker_position (w->pointm[CURRENT_DISP], bpoint, buffer); + if (EQ (wrap_window (w), Fselected_window (Qnil))) + { + BYTE_BUF_SET_PT (XBUFFER (buffer), bpoint); + } + + saved_window_start + = Fgethash (buffer, w->saved_last_window_start_cache, Qnil); + if (!EXTENTP (saved_window_start)) + { + saved_window_start = XBUFFER (buffer)->last_window_start; + } + + structure_checking_assert (!EXTENTP (saved_window_start) + || EQ (Fextent_object (saved_window_start), + buffer)); + + if (EXTENTP (saved_window_start) + && NILP (Fextent_detached_p (saved_window_start))) + { + bstart = bytebpos_clip_to_bounds (BYTE_BUF_BEGV (XBUFFER (buffer)), + extent_endpoint_byte + (XEXTENT (saved_window_start), 0), + BYTE_BUF_ZV (XBUFFER (buffer))); + } + else + { + bstart = BYTE_BUF_BEGV (XBUFFER (buffer)); + } + + set_byte_marker_position (w->start[CURRENT_DISP], bstart, buffer); + set_byte_marker_position (w->sb_point, bstart, buffer); + /* set start_at_line_beg correctly. GE */ - w->start_at_line_beg = - byte_beginning_of_line_p (XBUFFER (buffer), - byte_marker_position (w->start[CURRENT_DISP])); + w->start_at_line_beg = byte_beginning_of_line_p (XBUFFER (buffer), bstart); w->force_start = 0; /* XEmacs fix */ SET_LAST_MODIFIED (w, 1); SET_LAST_FACECHANGE (w); diff -r cc27eb6f808b -r 93f9adafefb3 src/winslots.h --- a/src/winslots.h Mon Feb 18 14:53:34 2019 +0000 +++ b/src/winslots.h Sun Mar 10 13:09:43 2019 +0000 ＠＠ -125,18 +125,18 ＠＠ /* A marker pointing to where in the text the scrollbar is pointing; #### moved to scrollbar.c? */ WINDOW_SLOT (sb_point) - /* A table that remembers (in marker form) the value of point in buffers - previously displayed in this window. Switching back to those buffers - causes the remembered point value to become current, rather than the - buffer's point. This is so that you get sensible behavior if you have - a buffer displayed in multiple windows and temporarily switch away and - then back in one window. We don't save or restore this table in a - window configuration, since that would be counterproductive -- we - always want to remember the most recent value of point in buffers we - switched away from. */ + /* A table that remembers (in zero-length-extent form) the value of point in + buffers previously displayed in this window. Switching back to those + buffers causes the remembered point value to become current, rather than + the buffer's point. This is so that you get sensible behavior if you + have a buffer displayed in multiple windows and temporarily switch away + and then back in one window. We don't save or restore this table in a + window configuration, since that would be counterproductive -- we always + want to remember the most recent value of point in buffers we switched + away from. */ WINDOW_SLOT (saved_point_cache) - /* A table that remembers (in marker form) the value of window start in - buffers previously displayed in this window. Save reason as for + /* A table that remembers (in zero-length extent form) the value of window + start in buffers previously displayed in this window. Same reason as for the previous table. */ WINDOW_SLOT (saved_last_window_start_cache) -- ‘As I sat looking up at the Guinness ad, I could never figure out / How your man stayed up on the surfboard after forty pints of stout’ (C. Moore)