From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Po Lu Newsgroups: gmane.emacs.devel Subject: Re: Allowing point to be outside the window? Date: Sun, 06 Feb 2022 19:46:15 +0800 Message-ID: <87leyonrp4.fsf@yahoo.com> References: <87ilwd7zaq.fsf.ref@yahoo.com> <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> <83h79cz0sm.fsf@gnu.org> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="22281"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.60 (gnu/linux) Cc: emacs-devel@gnu.org To: Eli Zaretskii Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Sun Feb 06 12:54:43 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 1nGg7v-0005e8-0U for ged-emacs-devel@m.gmane-mx.org; Sun, 06 Feb 2022 12:54:43 +0100 Original-Received: from localhost ([::1]:42876 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1nGg7t-0004b2-HW for ged-emacs-devel@m.gmane-mx.org; Sun, 06 Feb 2022 06:54:41 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:34274) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nGg08-0000Gm-FQ for emacs-devel@gnu.org; Sun, 06 Feb 2022 06:46:43 -0500 Original-Received: from sonic303-21.consmr.mail.ne1.yahoo.com ([66.163.188.147]:37091) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1nGg05-0004b7-Db for emacs-devel@gnu.org; Sun, 06 Feb 2022 06:46:39 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yahoo.com; s=s2048; t=1644147986; bh=LHarH2HutIO3w51b0mSYKAtzlWbfR0uGdAUnFJrjYIU=; h=From:To:Cc:Subject:References:Date:In-Reply-To:From:Subject:Reply-To; b=J6D29YmHmKiSZ4DoaLm9YBfkAQZyYHRUj+4NLZ4FTjAyRgeAmKURguMA3Pi6j1vv7s4dRVzBbAjTfSO26TDQzqueKil0IX+XmsFvoImd0sVIhZS0UQLWfjhc5WdnGPTU/ee5avYn5R0sJpNCQkXGs29HWLtDth6/W0Y29F0VBchKt5BXh6EDpIQtmjM9+1N4d8Mhf4CAjJnA4UtuZj3KaETtkdqiTPQqQD39H/uZVX/azmFkiCMZtMXCJ0C7Mo1ZVCXONsMlL6QoD1jYvWJT6dfM4IxPgqNCv+fAroSGUkRzlBZdyG9dD8V2GM1dRqoRgDqQDFwP7y0yHPjN1buTzw== X-SONIC-DKIM-SIGN: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yahoo.com; s=s2048; t=1644147986; bh=ftz5ytBRqfSo7VFLNWIDnsogZvuh3HkVUE3YM4J8nNw=; h=X-Sonic-MF:From:To:Subject:Date:From:Subject; b=FCeX6g/p3q6VBAmIcL3dMEGA/FopLdsRQZ2Y3gXu609GAY6kr7ztNkeRbgJWwreLITW0ip8ZphaZ5rcNhK1pZnKIHczhJzutWgcj9k2GnUuuUyTQg7XBX82KsmBeHU0/X00J9Ek7ub9pbKp0FWf6OabG94+KMTBfSVNA60/FM/Otfd3OGMfV7P9iXV2GRoRpZwChDjTYCszer/6KMMPBCnOH3GPnDf+SoutJAibKTb4umBNrDhbtumK2lzODO9Cs38PYvWwmRoK5E182V1xQGFUWcQthRs/UGqkGo2BIqYXbmAuKFv/qluigWErJ9lSyCJVImBw/Cm7ySHfsJbqUMw== X-YMail-OSG: moeB3e4VM1kF.CCB87JT9PxdyTksDBBf.sgEDJa7e6lBpLkHer.CxADtBE_.MQZ gEu5rDJzeGKmd2BVf6sqj1fOgbUZDExIMN1PCLaEo3r_AEyy8nuXk_2si.i0IS_MUisimdYwctFG QbvMthGCWqA4nRfeVx9XKcwj2GHDO.bkERkHXKANPr.crK7WJmWb0t2qwUtkUvcwRE8iV9lK52Kg eQiHSwJvJ4TtFAl9j1XlONIG4Dk4bHDGXzMTR2HrK5TcOwmRCTwkrzTylTEwkE32zWxar80P2KhO aesAitWTLL4svHr.sKclGat0WxAT4FeTueQkrRM3T9kWoCUld.EQ2TVlg4K7grghEqB1N5SDhE73 Qb3YXKC867ZLChxUKodo7rnvlnIj5bRL4zwZm_ug87iy4qoII14rmm6ZAU.pAFhXezmCEysq9L2z hSd7_tuRQ5ZZlYNye_OYSiJvDyVzAZqF2HpnvWBFcoDL61ZA1Rp1_hGQiaAcMArI_RP6AUHoatJ7 4iJH5LDYpjLFMlMdG3CriDBr6s1r_iIl0IiUqe.GS9gUppSIqUn4WY5wP9VOiHCpdvwBzbKT7Vlf 8L.rVZI_cOqGyI4_8gAwBZx9l9iAp82gZmg1Mx0hpX9WWUnOKQK1QtcMZtbunPEaEa6wUcTrQMtl QxfW5PzCEFWj10ZCJj1ckdz9U_792OaiEBhfsIryYTGEOP8hDAQq8xNMGvwm54ZbCBtU73ZzzZQ. HPrzrdX_FP4_hmzOyD_2ZIjUfeCXQEMjr29fSPSf4pM_wDdhqsp0DCH_27cIfqJuhHYI6HbCtvqX aTbKfZIF5kEQRXTxSRQzuEpDzne6FPpJLmRRFQYQ8c X-Sonic-MF: Original-Received: from sonic.gate.mail.ne1.yahoo.com by sonic303.consmr.mail.ne1.yahoo.com with HTTP; Sun, 6 Feb 2022 11:46:26 +0000 Original-Received: by kubenode512.mail-prod1.omega.sg3.yahoo.com (VZM Hermes SMTP Server) with ESMTPA ID 20f02b7708bd138778403256085fae07; Sun, 06 Feb 2022 11:46:20 +0000 (UTC) In-Reply-To: <83h79cz0sm.fsf@gnu.org> (Eli Zaretskii's message of "Sun, 06 Feb 2022 13:34:17 +0200") X-Mailer: WebService/1.1.19711 mail.backend.jedi.jws.acl:role.jedi.acl.token.atz.jws.hermes.yahoo Received-SPF: pass client-ip=66.163.188.147; envelope-from=luangruo@yahoo.com; helo=sonic303-21.consmr.mail.ne1.yahoo.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=unavailable autolearn_force=no X-Spam_action: no action 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:285960 Archived-At: Eli Zaretskii writes: > 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. Sorry for that, and I don't quite remember the problems either. IIRC, the one problem that remained unsolved was to find a better solution for the case where some text changed behind window start than to recenter the window around point, which I think I've found. >> +@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)? The first flag controls the behaviour of redisplay, while the second controls that of the scrolling commands, so I think they are orthogonal. > This is waaaay too terse for such a significant change... Agreed, though I can't think of anything better. > 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. It allows pixel-scroll to take advantage of the new feature. You can just ignore it for now, I simply forgot to remove it. >> - 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? Thanks for catching that. >> @@ -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; > What does it mean when you set the w->start point, but do NOT set the > w->force_start flag? w->force_start will result in point being constrained inside window start by the code under force_start: in redisplay_window. >> @@ -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?? ??? That seems to be a problem which came up after I rebased the changes onto master from some version of master around late December, not before. Thanks for catching it. >> + 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. I saw the code do that in try_window_id, so even though I didn't really understand it I thought it would be safer to do that here as well. >> - /* -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. Thanks. >> + /* 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? Yes, IMO. If the change is actually at point, then the window will be recentered around point by the `PT != w->last_point' conditional. Otherwise, such surprising changes to point would be unwanted. > Why do we need this maybe_try_window stuff? It seems to repeat > existing code, doesn't it? It does. There was a reason I put it there last December, but I can't remember it right now. Thanks.