From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel Subject: Re: Allowing point to be outside the window? Date: Sun, 06 Feb 2022 13:34:17 +0200 Message-ID: <83h79cz0sm.fsf@gnu.org> References: <87ilwd7zaq.fsf.ref@yahoo.com> <87ilwb68ck.fsf@yahoo.com> <83zgpnunfo.fsf@gnu.org> <87fsrf3xmd.fsf@yahoo.com> <83y257ulfp.fsf@gnu.org> <8735ne4e0e.fsf@yahoo.com> <87czmcvcs1.fsf@yahoo.com> <83sfv85y36.fsf@gnu.org> <87v904tsvv.fsf@yahoo.com> <83h7bo5m1x.fsf@gnu.org> <87ilw3ubfp.fsf@yahoo.com> <83h7bn4e55.fsf@gnu.org> <877dcipjmk.fsf@yahoo.com> <83mtld254e.fsf@gnu.org> <87lf0xjgxu.fsf@yahoo.com> <83ilw0zg38.fsf@gnu.org> <87mtlbgajq.fsf@yahoo.com> <83czm7vx0s.fsf@gnu.org> <87mtlad3sv.fsf@yahoo.com> <83mtlaurxj.fsf@gnu.org> <87fsqh9o7s.fsf@yahoo.com> <878ruoqx0u.fsf@yahoo.com> Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="12494"; mail-complaints-to="usenet@ciao.gmane.io" Cc: emacs-devel@gnu.org To: Po Lu Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Sun Feb 06 12:35:17 2022 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1nGfp6-00032r-6u for ged-emacs-devel@m.gmane-mx.org; Sun, 06 Feb 2022 12:35:16 +0100 Original-Received: from localhost ([::1]:35008 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1nGfp4-0006hS-IB for ged-emacs-devel@m.gmane-mx.org; Sun, 06 Feb 2022 06:35:14 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:32782) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nGfoQ-0005wa-GJ for emacs-devel@gnu.org; Sun, 06 Feb 2022 06:34:34 -0500 Original-Received: from [2001:470:142:3::e] (port=55528 helo=fencepost.gnu.org) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nGfoQ-0002oK-6B; Sun, 06 Feb 2022 06:34:34 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=DH1h8TPZC3uCmQQht8Ufb9B6+FcaCNhAhu8DMkVEElA=; b=iZAn3UCQTD3U fO+5UhvA0k3U2l4kFoXzqST3cUUx6HmwLwqD5zVJfzbSVWU7SZohJj926TJKMlDtI03LbO1PgqWAP yR/0ExtTGPGSP4fJChhZ2jObMTC1urPjgano4B4C90baIQcscQpyqfnMMATsDVFThPxJukDpzCAsQ BFixvJHte8KP/QAOXQNbts1u5xLvHJAHMZBmtAgiJG1bJ2108crLNOHjpWA4n5QgEOZbGIedwm1pF pgblJ7Sx67XPpu5JIp1oDUMf/Q/tBUgvZmeCOzks4oMRpJP5IB8DdweRtD+E4O/f/XQ25CyzXlFu4 sg2QNZSMJV+ySJ6ywUgiyA==; Original-Received: from [87.69.77.57] (port=3467 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nGfoP-0006mI-Hk; Sun, 06 Feb 2022 06:34:33 -0500 In-Reply-To: <878ruoqx0u.fsf@yahoo.com> (message from Po Lu on Sun, 06 Feb 2022 15:22:57 +0800) X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.io gmane.emacs.devel:285956 Archived-At: > From: Po Lu > Cc: emacs-devel@gnu.org > Date: Sun, 06 Feb 2022 15:22:57 +0800 > > Po Lu writes: > > > To be completely clear, the "recenter around point" fallback you allude > > to is the code under the `recenter' label in `redisplay_window', > > correct? > > > > Please forgive me if this has been answered before, but I haven't been > > working on this in a while, and my memory is already getting rusty. > > > > Thanks. > > How about this: we recenter around the position of the first modified > character? Point isn't moved at all during that process. It's very hard to keep a discussion with such long pauses. I don't even remember what we were discussing the last time and what issues remained unresolved. > +@vindex keep-point-visible > + If @code{keep-point-visible} is nil, redisplay will not move recenter > +the display when the window start is changed. > + > +@vindex scroll-move-point > + If @code{scroll-move-point} is nil, scrolling commands will not move > +point to keep it inside the visible part of the window. Why do we need 2 flags? Are they indeed orthogonal, or can we have a single variable (perhaps with more than 2 states)? > +** New variable 'keep-point-visible'. > +This variable controls if redisplay will try to keep point visible > +inside the window. > + > ++++ > +** New variable 'scroll-move-point'. > +This variable controls if scrolling moves point to stay inside the > +window. This is waaaay too terse for such a significant change... > --- a/lisp/pixel-scroll.el > +++ b/lisp/pixel-scroll.el Why do we need to have changes in pixel-scroll.el be part of this changeset? It makes the changes harder to review, and is not really related to the changes in the display code. > --- a/src/window.c > +++ b/src/window.c > @@ -5578,7 +5578,8 @@ window_scroll_pixel_based (Lisp_Object window, int n, bool whole, bool noerror) > something like (scroll-down 1) with PT in the line before > the partially visible one would recenter. */ > > - if (!pos_visible_p (w, PT, &x, &y, &rtop, &rbot, &rowh, &vpos)) > + if (!pos_visible_p (w, PT, &x, &y, &rtop, &rbot, &rowh, &vpos) > + && scroll_move_point) Don't you need to test keep_point_visible as well here? If not, why not? > @@ -5659,8 +5660,9 @@ window_scroll_pixel_based (Lisp_Object window, int n, bool whole, bool noerror) > w->start_at_line_beg = true; > wset_update_mode_line (w); > /* Set force_start so that redisplay_window will run the > - window-scroll-functions. */ > - w->force_start = true; > + window-scroll-functions, unless scroll_move_point is false, > + in which case forcing the start will cause recentering. */ > + w->force_start = scroll_move_point; Should the logic of whether and how to obey the force_start flag be confined to the display code, instead of having part of it here? What does it mean when you set the w->start point, but do NOT set the w->force_start flag? > @@ -5844,8 +5846,9 @@ window_scroll_pixel_based (Lisp_Object window, int n, bool whole, bool noerror) > w->start_at_line_beg = (pos == BEGV || FETCH_BYTE (bytepos - 1) == '\n'); > wset_update_mode_line (w); > /* Set force_start so that redisplay_window will run the > - window-scroll-functions. */ > - w->force_start = true; > + window-scroll-functions, unless scroll_move_point is false, > + in which case forcing the start will cause recentering. */ > + w->force_start = scroll_move_point; Same here. > @@ -5857,7 +5860,7 @@ window_scroll_pixel_based (Lisp_Object window, int n, bool whole, bool noerror) > even if there is a header line. */ > this_scroll_margin = window_scroll_margin (w, MARGIN_IN_PIXELS); > > - if (n > 0) > + if (scroll_move_point) This and the rest of changes in window_scroll_pixel_based are impossible to review: you replaced the "n > 0" condition with a different one, and now the rest of the diffs are completely unreadable. I also don't understand how come the exact same code which was previously run only for n > 0 is now run for any value of 'n', and _by_default_ (since scroll_move_point is non-zero by default)? How can that be TRT?? > + if (!keep_point_visible && window_outdated (w)) > + { > + /* If some text changed between window start, then recenter the > + display around the first character that changed, to avoid > + confusing the user by not updating the display to reflect the > + changes. */ > + ptrdiff_t last_changed_charpos, first_changed_charpos; > + > + /* Make sure beg_unchanged and end_unchanged are up to date. Do it > + only if buffer has really changed. The reason is that the gap is > + initially at Z for freshly visited files. The code below would > + set end_unchanged to 0 in that case. */ > + if (GPT - BEG < BEG_UNCHANGED) > + BEG_UNCHANGED = GPT - BEG; > + if (Z - GPT < END_UNCHANGED) > + END_UNCHANGED = Z - GPT; I'm not sure I understand this part. Why do you need to change the values of BEG_UNCHANGED and END_UNCHANGED -- those are supposed to be changed only by code that modifies the buffer text. > - /* -1 means we need to scroll. > - 0 means we need new matrices, but fonts_changed > - is set in that case, so we will detect it below. */ > - goto try_to_scroll; > + { > + /* -1 means we need to scroll. > + 0 means we need new matrices, but fonts_changed > + is set in that case, so we will detect it below. */ > + goto try_to_scroll; > + } These braces are redundant. > + /* Determine the window start relative to where we want to recenter > + to. */ > + > + if (need_recenter_even_if_point_can_be_invisible) > + init_iterator (&it, w, that_recentering_position, > + that_recentering_byte, NULL, DEFAULT_FACE_ID); > + else > + init_iterator (&it, w, PT, PT_BYTE, NULL, DEFAULT_FACE_ID); > it.current_y = it.last_visible_y; > + You want to display window around the change, but not bring point there? Is that a good idea? > + maybe_try_window: Why do we need this maybe_try_window stuff? It seems to repeat existing code, doesn't it? Thanks.