From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: dick Newsgroups: gmane.emacs.bugs Subject: bug#52302: 28.0.50; [PATCH] Overlay strings should not increment vpos Date: Sun, 05 Dec 2021 13:52:35 -0500 Message-ID: <87r1aqj34c.fsf@dick> References: <87v903hs0w.fsf@dick> <83lf0y3np1.fsf@gnu.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="1803"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.14 (Gnus v5.14) Commercial/28.0.50 (gnu/linux) Cc: 52302@debbugs.gnu.org To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sun Dec 05 19:53:08 2021 Return-path: Envelope-to: geb-bug-gnu-emacs@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 1mtwdI-0000Hy-OS for geb-bug-gnu-emacs@m.gmane-mx.org; Sun, 05 Dec 2021 19:53:08 +0100 Original-Received: from localhost ([::1]:59010 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mtwdH-0001hY-Ld for geb-bug-gnu-emacs@m.gmane-mx.org; Sun, 05 Dec 2021 13:53:07 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:47038) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mtwdB-0001hQ-Pp for bug-gnu-emacs@gnu.org; Sun, 05 Dec 2021 13:53:01 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]:47912) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mtwdB-0003By-IM for bug-gnu-emacs@gnu.org; Sun, 05 Dec 2021 13:53:01 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1mtwdB-0001lH-IL for bug-gnu-emacs@gnu.org; Sun, 05 Dec 2021 13:53:01 -0500 X-Loop: help-debbugs@gnu.org Resent-From: dick Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 05 Dec 2021 18:53:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 52302 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 52302-submit@debbugs.gnu.org id=B52302.16387303646747 (code B ref 52302); Sun, 05 Dec 2021 18:53:01 +0000 Original-Received: (at 52302) by debbugs.gnu.org; 5 Dec 2021 18:52:44 +0000 Original-Received: from localhost ([127.0.0.1]:59458 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mtwct-0001kl-S5 for submit@debbugs.gnu.org; Sun, 05 Dec 2021 13:52:44 -0500 Original-Received: from mail-qk1-f175.google.com ([209.85.222.175]:33449) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mtwcs-0001kU-8L for 52302@debbugs.gnu.org; Sun, 05 Dec 2021 13:52:42 -0500 Original-Received: by mail-qk1-f175.google.com with SMTP id de30so9223271qkb.0 for <52302@debbugs.gnu.org>; Sun, 05 Dec 2021 10:52:42 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version; bh=sS96QjHd/J1k4Ea/pAEm4CGF7sKNFAUCifjxo7Mkt/4=; b=qRuFC9Eo0LoeqztC0bHRKL7wSPTJlxKy6rSznoZ1W6+3RIHVqPtFF3GjvKarsf3KY7 gDfSqHNxNUxbX8figJJsUXivG36pk5Hsn19UtGimhD/vtGztrYN/8rh0zyEVlegLF1eV n5EoP1m5Yfpp4HMvsj2gbnkVMDKApmA6FKewulX18opiuwJd8R7wTmf1j3nRVytufIBZ rcDakShlkl0dsqROn+SIggd+xWPTwyYVpiSBd0ho8IjGGxuzAV5q/H+su2gmkkR5E2os y1eXQL84A6oIhv7HNlERsk/0s1k1VgNNNhOwhKECH4SrMqEuaNrI31GxwMuPjlbGzHfL WQgQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version; bh=sS96QjHd/J1k4Ea/pAEm4CGF7sKNFAUCifjxo7Mkt/4=; b=n8MepjrNYn4ORBwzZyk0MZmqhPGmRCQBik/ZtHo2Km2DPEx4+rTn4arQaQgXDV7lVn jbiDMUNjjm7q87qwKeQiBezrocfAHcms6z6wli3IZC8t1gSTijFNtwQegu80yBXPCL4X HGgsLDwIiVzymYPQ7wZd0hR0hoQJSzzQu0V3pdl0UczLLP5idrbtKNr2UCND06ba6gvL 1ZGtWvb8pLysFqEyvG7+oJT3FxcXLV4RpAKpMPTSjWyAgZrxrmxil/qN5sYyEh/8WWX4 kKJwDHLGhz2bHQL6kXH85LaN5jB5G6eyPovkRCyAeS6QA3dNSPNrC69+dGkVmqTPM7DS RFDw== X-Gm-Message-State: AOAM533A3l1kVeC/z6QezOr3VUKfGtnhwNZhpoOYUBOgGJX3UoDT5Fqa 8CX6mFBYy4IqGWrL3TKak+Q= X-Google-Smtp-Source: ABdhPJxNo2Opx7bFGbKyp6oy96MfW98oJblsng/91hLBmWIjJghrMPUGs75yyH78inuTYiZCTifxrg== X-Received: by 2002:a05:620a:706:: with SMTP id 6mr28714942qkc.374.1638730356662; Sun, 05 Dec 2021 10:52:36 -0800 (PST) Original-Received: from localhost ([68.237.93.126]) by smtp.gmail.com with ESMTPSA id j124sm5404586qkd.98.2021.12.05.10.52.36 (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Sun, 05 Dec 2021 10:52:36 -0800 (PST) In-Reply-To: <83lf0y3np1.fsf@gnu.org> (Eli Zaretskii's message of "Sun, 05 Dec 2021 20:34:50 +0200") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:221634 Archived-At: --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=0001-Overlay-strings-at-to_charpos-should-not-increment-v.patch >From 390e5866888a740bca2daaa9b528d1905df3ffd7 Mon Sep 17 00:00:00 2001 From: dickmao Date: Sun, 5 Dec 2021 13:46:59 -0500 Subject: [PATCH] Overlay strings at `to_charpos` should not increment vpos * src/xdisp.c (move_it_to): Recognize the invisible overlay string should not increment vpos. (move_it_vertically_backward): Rectify assertions. (resize_mini_window): Should not need to call move_it_vertically_backward (it, 0) twice. * test/src/xdisp-tests.el (xdisp-tests--minibuffer-resizing): Test. --- src/xdisp.c | 41 ++++++++++++++++++++--------------------- test/src/xdisp-tests.el | 30 +++++++++++++----------------- 2 files changed, 33 insertions(+), 38 deletions(-) diff --git a/src/xdisp.c b/src/xdisp.c index 0ff6286af74..c264d7a3be1 100644 --- a/src/xdisp.c +++ b/src/xdisp.c @@ -10081,6 +10081,7 @@ move_it_to (struct it *it, ptrdiff_t to_charpos, int to_x, int to_y, int to_vpos for (;;) { + bool reached_continued = false; orig_charpos = IT_CHARPOS (*it); orig_method = it->method; if (op & MOVE_TO_VPOS) @@ -10125,6 +10126,12 @@ move_it_to (struct it *it, ptrdiff_t to_charpos, int to_x, int to_y, int to_vpos break; } } + else if (skip == MOVE_LINE_CONTINUED + && op & MOVE_TO_POS + && it->method == GET_FROM_STRING + && IT_CHARPOS (*it) == to_charpos) + /* TO_CHARPOS reached, now consuming overlay string. */ + reached_continued = true; } } else if (op & MOVE_TO_Y) @@ -10377,13 +10384,16 @@ move_it_to (struct it *it, ptrdiff_t to_charpos, int to_x, int to_y, int to_vpos /* Reset/increment for the next run. */ recenter_overlay_lists (current_buffer, IT_CHARPOS (*it)); it->current_x = line_start_x; - line_start_x = 0; - it->hpos = 0; - it->line_number_produced_p = false; - it->current_y += it->max_ascent + it->max_descent; - ++it->vpos; last_height = it->max_ascent + it->max_descent; - it->max_ascent = it->max_descent = 0; + if (! reached_continued) + { + line_start_x = 0; + it->hpos = 0; + it->line_number_produced_p = false; + it->current_y += it->max_ascent + it->max_descent; + ++it->vpos; + it->max_ascent = it->max_descent = 0; + } } out: @@ -10490,11 +10500,11 @@ move_it_vertically_backward (struct it *it, int dy) || (it2.method == GET_FROM_STRING && IT_CHARPOS (it2) == start_pos && SREF (it2.string, IT_STRING_BYTEPOS (it2) - 1) == '\n'))); - eassert (IT_CHARPOS (*it) >= BEGV); + eassert (IT_CHARPOS (it2) >= BEGV); SAVE_IT (it3, it2, it3data); move_it_to (&it2, start_pos, -1, -1, -1, MOVE_TO_POS); - eassert (IT_CHARPOS (*it) >= BEGV); + eassert (IT_CHARPOS (it2) >= BEGV); /* H is the actual vertical distance from the position in *IT and the starting position. */ h = it2.current_y - it->current_y; @@ -12218,7 +12228,6 @@ resize_mini_window (struct window *w, bool exact_p) struct it it; int unit = FRAME_LINE_HEIGHT (f); int height, max_height; - struct text_pos start; struct buffer *old_current_buffer = NULL; int windows_height = FRAME_INNER_HEIGHT (f); @@ -12272,25 +12281,15 @@ resize_mini_window (struct window *w, bool exact_p) { init_iterator (&it, w, ZV, ZV_BYTE, NULL, DEFAULT_FACE_ID); move_it_vertically_backward (&it, height - unit); - /* The following move is usually a no-op when the stuff - displayed in the mini-window comes entirely from buffer - text, but it is needed when some of it comes from overlay - strings, especially when there's an after-string at ZV. - This happens with some completion packages, like - icomplete, ido-vertical, etc. With those packages, if we - don't force w->start to be at the beginning of a screen - line, important parts of the stuff in the mini-window, - such as user prompt, will be hidden from view. */ - move_it_by_lines (&it, 0); - start = it.current.pos; /* Prevent redisplay_window from recentering, and thus from overriding the window-start point we computed here. */ w->start_at_line_beg = false; - SET_MARKER_FROM_TEXT_POS (w->start, start); + SET_MARKER_FROM_TEXT_POS (w->start, it.current.pos); } } else { + struct text_pos start; SET_TEXT_POS (start, BEGV, BEGV_BYTE); SET_MARKER_FROM_TEXT_POS (w->start, start); } diff --git a/test/src/xdisp-tests.el b/test/src/xdisp-tests.el index ae4aacd9c7c..d1d7262665a 100644 --- a/test/src/xdisp-tests.el +++ b/test/src/xdisp-tests.el @@ -34,23 +34,19 @@ xdisp-tests--in-minibuffer (ert-deftest xdisp-tests--minibuffer-resizing () ;; bug#43519 (should - (equal - t - (xdisp-tests--in-minibuffer - (insert "hello") - (let ((ol (make-overlay (point) (point))) - (max-mini-window-height 1) - (text "askdjfhaklsjdfhlkasjdfhklasdhflkasdhflkajsdhflkashdfkljahsdlfkjahsdlfkjhasldkfhalskdjfhalskdfhlaksdhfklasdhflkasdhflkasdhflkajsdhklajsdgh")) - ;; (save-excursion (insert text)) - ;; (sit-for 2) - ;; (delete-region (point) (point-max)) - (put-text-property 0 1 'cursor t text) - (overlay-put ol 'after-string text) - (redisplay 'force) - ;; Make sure we do the see "hello" text. - (prog1 (equal (window-start) (point-min)) - ;; (list (window-start) (window-end) (window-width)) - (delete-overlay ol))))))) + (xdisp-tests--in-minibuffer + (insert "hello") + (let ((ol (make-overlay (point) (point))) + (max-mini-window-height 1) + (text (let ((s "")) + (dotimes (i 137) + (setq s (concat s (char-to-string (+ (% i 26) ?a))))) + s))) + (put-text-property 0 1 'cursor t text) + (overlay-put ol 'after-string text) + (redisplay) + (prog1 (equal (window-start) (point-min)) + (delete-overlay ol)))))) (ert-deftest xdisp-tests--minibuffer-scroll () ;; bug#44070 (let ((posns -- 2.26.2 --=-=-= Content-Type: text/plain >>>>> "EZ" == Eli Zaretskii writes: >> From: dick.r.chiang@gmail.com >> Date: Sun, 05 Dec 2021 12:37:35 -0500 >> >> >From 010b26de2993754db6bb42243b5c6c89fc5e8a50 Mon Sep 17 00:00:00 2001 >> From: dickmao >> Date: Sun, 5 Dec 2021 12:25:32 -0500 >> Subject: [PATCH] Overlay strings at `to_charpos` should not increment vpos >> >> Previously, two calls to `move_it_vertically_backward (it, 0)` were >> required to get IT back to line start. It should only ever >> take one call. EZ> Please tell more about the motivation. In which use cases this EZ> change behaves better, and why? This is a delicate code, used in EZ> many places, so we need a very good understanding of what gets EZ> fixed. >> + else if (skip == MOVE_LINE_CONTINUED >> + && it->method == GET_FROM_STRING >> + && IT_CHARPOS (*it) == to_charpos) >> + /* TO_CHARPOS reached, now consuming overlay string. */ it-> method == GET_FROM_STRING doesn't necessarily mean we are it-> consuming an overlay string. It could be a string from display it-> property, for example. >> - ++it->vpos; >> + if (! reached_continued) >> + ++it->vpos; EZ> I don't think I see the connection between the above condition and EZ> the need to increment (or not increment) VPOS. Can you elaborate on EZ> that? >> @@ -10490,11 +10497,11 @@ move_it_vertically_backward (struct it *it, >> int dy) || (it2.method == GET_FROM_STRING && IT_CHARPOS (it2) == >> start_pos && SREF (it2.string, IT_STRING_BYTEPOS (it2) - 1) == >> '\n'))); - eassert (IT_CHARPOS (*it) >= BEGV); + eassert (IT_CHARPOS >> (it2) >= BEGV); SAVE_IT (it3, it2, it3data); move_it_to (&it2, >> start_pos, -1, -1, -1, MOVE_TO_POS); - eassert (IT_CHARPOS (*it) >= >> BEGV); + eassert (IT_CHARPOS (it2) >= BEGV); EZ> Why are you replacing the assertions here? >> --- a/test/src/xdisp-tests.el >> +++ b/test/src/xdisp-tests.el EZ> What exactly is changed in this test? It looks like purely EZ> stylistic changes to me (which for some reason also lots the EZ> comments). Did I miss something? --=-=-=--