From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Matt Armstrong Newsgroups: gmane.emacs.bugs Subject: bug#58928: 29.0.50; overlays in org-mode are disrupted after call `org-capture` Date: Thu, 03 Nov 2022 21:33:02 -0700 Message-ID: <87o7tn49vl.fsf@rfc20.org> References: <87eduohyds.fsf@gmail.com> <834jvjdwwl.fsf@gnu.org> <87sfj3gmsx.fsf@gmail.com> <87bkprm4a9.fsf@localhost> <87sfiz4pov.fsf@rfc20.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="13970"; mail-complaints-to="usenet@ciao.gmane.io" Cc: Ihor Radchenko , Eli Qian , Eli Zaretskii , 58928@debbugs.gnu.org, Stefan Monnier To: Eason Huang , Gerd =?UTF-8?Q?M=C3=B6llmann?= Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Fri Nov 04 05:34:25 2022 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 1oqoPQ-0003Q0-A2 for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 04 Nov 2022 05:34:24 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oqoP7-0001n9-0e; Fri, 04 Nov 2022 00:34:05 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oqoP5-0001mt-Oz for bug-gnu-emacs@gnu.org; Fri, 04 Nov 2022 00:34:03 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1oqoP5-0005fV-4n for bug-gnu-emacs@gnu.org; Fri, 04 Nov 2022 00:34:03 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1oqoP4-0004TA-I7 for bug-gnu-emacs@gnu.org; Fri, 04 Nov 2022 00:34:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Matt Armstrong Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 04 Nov 2022 04:34:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 58928 X-GNU-PR-Package: emacs Original-Received: via spool by 58928-submit@debbugs.gnu.org id=B58928.166753639717129 (code B ref 58928); Fri, 04 Nov 2022 04:34:02 +0000 Original-Received: (at 58928) by debbugs.gnu.org; 4 Nov 2022 04:33:17 +0000 Original-Received: from localhost ([127.0.0.1]:51390 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1oqoOK-0004SC-L2 for submit@debbugs.gnu.org; Fri, 04 Nov 2022 00:33:17 -0400 Original-Received: from relay7-d.mail.gandi.net ([217.70.183.200]:52063) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1oqoOH-0004Rw-8j for 58928@debbugs.gnu.org; Fri, 04 Nov 2022 00:33:15 -0400 Original-Received: (Authenticated sender: matt@rfc20.org) by mail.gandi.net (Postfix) with ESMTPSA id 5D85820002; Fri, 4 Nov 2022 04:33:04 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rfc20.org; s=gm1; t=1667536387; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=TvVFTKXO973jQWi6NbjyB68IekaspklUBy5mHDNRGE8=; b=hi7pUOjaipWblEsS4XOLCjVB+iSpPnCWVs8arkjTRbqiscYLESh5taCnnqOKxxVvExWpfI 3ewMH1BBEw6DHBJo8rpPKRDqrZwYjaIGyJRqsvYzC5J90ZoZgFsfc0jtzyRgzPrAo6zuUz xfLkwoD/ttpapp2S7KLRhbJB6KUGU3z+ZDFq+iVkRbo/ekkhZyVbhXAL2QbtMhtfIuJumr uN2W/PjEooOFkWcjX1dwmrOSo4o5BKVgtPL+Un3AT9TDXS7bSd0LalAp2boKpCrXXx0/8E 4aI2ee7DlI0FzomSyJFwVVNfDtrDu1hQjnZab3rC0vaJoA4W8eebvim6AbSTPg== Original-Received: from matt by naz with local (Exim 4.96) (envelope-from ) id 1oqoO6-0019x1-1F; Thu, 03 Nov 2022 21:33:02 -0700 In-Reply-To: <87sfiz4pov.fsf@rfc20.org> 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: , Original-Sender: "bug-gnu-emacs" Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:247040 Archived-At: --=-=-= Content-Type: text/plain Matt Armstrong writes: > Eason Huang writes: > >> I can reproduce the following steps, may be you can have try: >> >> 1. emacs -Q , launch Emacs >> 2. M-x org-capture, and then type `t` (task) >> Now the buffer CAPTURE-.notes will open with contents as bellow: >> >> ** TODO (now the cursor is here, you can type some characters, such as "test1", and then type C-C C-c ) >> [2022-11-03 Thu] >> >> 3. C-x b, switch-to-buffer .notes >> The contents is as below: >> * Tasks >> ** TODO test1(move cusror here and type TAB) >> [2022-11-03 Thu] >> >> 4. Now the .notes buffer is like this: >> * Tasks >> ** TODO test1... >> 5. Try step 2 again, now you will see the issue. Now the .notes buffer looks like this: >> * Tasks >> ** TODO test1...TODO >> [2022-11-03 Thu] >> test1 > > I can reproduce this too. Note that this is a new set of repro steps > that are easier to do after "emacs -Q". Thanks Eason! See attached fix to fix the bug found by Eason's steps quoted above. The bug was that we ignored the "before markers" part of `insert-before-markers'. Stefan, this and a couple other minor patches are in https://git.sr.ht/~matta/emacs/log/scratch/matta/for_stefan Now, with respect to Eason's repro above, Emacs is "bug/behavior compatible" with itself before the noverlay merge to master. There may still be an org-mode bug here. In step 3 above org puts an invisible overlay with "ellipsis" over the "TODO test1" heading and content. When it later appends the new heading via org-capture it does so with `insert-before-markers' which causes the end of the "ellipsis" overlay to cover the new entry too, which later confuses other org functions until the heading is expanded and the overlay is deleted. This seems wrong and doesn't happen in Emacs 27. But, it does happen as of commit 69121c33e4a11805bf6438131c8aec72411a0e5d (the predecessor to Stefan's noverlay->master merge commit). --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=0003-Fix-insert-before-markers-for-the-new-overlays-imple.patch >From 23715529c7b87dfbed7810f674a161ddb4fbd909 Mon Sep 17 00:00:00 2001 From: Matt Armstrong Date: Thu, 3 Nov 2022 20:57:29 -0700 Subject: [PATCH 3/3] Fix insert-before-markers for the new overlays implementation. * src/itree.h: Add a before_markers arg to itree_insert_gap. * src/itree.c (itree_insert_gap): Use it, fixing bug#58928. * src/lisp.h: Add a before_markers arg to adjust_overlays_for_insert. * src/buffer.c (adjust_overlays_for_insert): Pass it through to itree_insert_gap. * src/insdel.c (insert_1_both): Pass before_markers through to adjust_overlays_for_insert. (insert_from_string_1): Ditto. (insert_from_gap): Pass before_markers=false to adjust_overlays_for_insert. (insert_from_buffer_1): ditto. (adjust_after_replace): ditto. (replace_range): ditto. (replace_range_2): ditto. * test/src/buffer-tests.el (buffer-tests-overlay-string): New helper. (test-overlay-insert-before-markers-at-start): New test. (test-overlay-insert-before-markers-at-end): New test. --- src/buffer.c | 11 ++++--- src/insdel.c | 14 ++++----- src/itree.c | 28 +++++++++++------- src/itree.h | 3 +- src/lisp.h | 2 +- test/src/buffer-tests.el | 62 ++++++++++++++++++++++++++++++++++++++++ 6 files changed, 97 insertions(+), 23 deletions(-) diff --git a/src/buffer.c b/src/buffer.c index 3129aa2890e..5e15e9e1ec3 100644 --- a/src/buffer.c +++ b/src/buffer.c @@ -3454,20 +3454,23 @@ overlay_strings (ptrdiff_t pos, struct window *w, unsigned char **pstr) void -adjust_overlays_for_insert (ptrdiff_t pos, ptrdiff_t length) +adjust_overlays_for_insert (ptrdiff_t pos, ptrdiff_t length, + bool before_markers) { if (!current_buffer->indirections) - itree_insert_gap (current_buffer->overlays, pos, length); + itree_insert_gap (current_buffer->overlays, pos, length, + before_markers); else { struct buffer *base = current_buffer->base_buffer ? current_buffer->base_buffer : current_buffer; Lisp_Object tail, other; - itree_insert_gap (base->overlays, pos, length); + itree_insert_gap (base->overlays, pos, length, before_markers); FOR_EACH_LIVE_BUFFER (tail, other) if (XBUFFER (other)->base_buffer == base) - itree_insert_gap (XBUFFER (other)->overlays, pos, length); + itree_insert_gap (XBUFFER (other)->overlays, pos, length, + before_markers); } } diff --git a/src/insdel.c b/src/insdel.c index 6d56a76c77a..33958f53e9c 100644 --- a/src/insdel.c +++ b/src/insdel.c @@ -917,7 +917,7 @@ insert_1_both (const char *string, if (Z - GPT < END_UNCHANGED) END_UNCHANGED = Z - GPT; - adjust_overlays_for_insert (PT, nchars); + adjust_overlays_for_insert (PT, nchars, before_markers); adjust_markers_for_insert (PT, PT_BYTE, PT + nchars, PT_BYTE + nbytes, before_markers); @@ -1043,7 +1043,7 @@ insert_from_string_1 (Lisp_Object string, ptrdiff_t pos, ptrdiff_t pos_byte, if (Z - GPT < END_UNCHANGED) END_UNCHANGED = Z - GPT; - adjust_overlays_for_insert (PT, nchars); + adjust_overlays_for_insert (PT, nchars, before_markers); adjust_markers_for_insert (PT, PT_BYTE, PT + nchars, PT_BYTE + outgoing_nbytes, before_markers); @@ -1115,7 +1115,7 @@ insert_from_gap (ptrdiff_t nchars, ptrdiff_t nbytes, bool text_at_gap_tail) insert_from_gap_1 (nchars, nbytes, text_at_gap_tail); - adjust_overlays_for_insert (ins_charpos, nchars); + adjust_overlays_for_insert (ins_charpos, nchars, false); adjust_markers_for_insert (ins_charpos, ins_bytepos, ins_charpos + nchars, ins_bytepos + nbytes, 0); @@ -1257,7 +1257,7 @@ insert_from_buffer_1 (struct buffer *buf, if (Z - GPT < END_UNCHANGED) END_UNCHANGED = Z - GPT; - adjust_overlays_for_insert (PT, nchars); + adjust_overlays_for_insert (PT, nchars, false); adjust_markers_for_insert (PT, PT_BYTE, PT + nchars, PT_BYTE + outgoing_nbytes, 0); @@ -1323,7 +1323,7 @@ adjust_after_replace (ptrdiff_t from, ptrdiff_t from_byte, record_insert (from, len); if (len > nchars_del) - adjust_overlays_for_insert (from, len - nchars_del); + adjust_overlays_for_insert (from, len - nchars_del, false); else if (len < nchars_del) adjust_overlays_for_delete (from, nchars_del - len); @@ -1513,7 +1513,7 @@ replace_range (ptrdiff_t from, ptrdiff_t to, Lisp_Object new, /* Adjust the overlay center as needed. This must be done after adjusting the markers that bound the overlays. */ adjust_overlays_for_delete (from, nchars_del); - adjust_overlays_for_insert (from, inschars); + adjust_overlays_for_insert (from, inschars, false); offset_intervals (current_buffer, from, inschars - nchars_del); @@ -1648,7 +1648,7 @@ replace_range_2 (ptrdiff_t from, ptrdiff_t from_byte, adjusting the markers that bound the overlays. */ if (nchars_del != inschars) { - adjust_overlays_for_insert (from, inschars); + adjust_overlays_for_insert (from, inschars, false); adjust_overlays_for_delete (from + inschars, nchars_del); } diff --git a/src/itree.c b/src/itree.c index bd4e8cc5740..0b56feed43f 100644 --- a/src/itree.c +++ b/src/itree.c @@ -1190,11 +1190,14 @@ itree_iterator_finish (struct itree_iterator *iter) /* Insert a gap at POS of length LENGTH expanding all intervals intersecting it, while respecting their rear_advance and - front_advance setting. */ + front_advance setting. + + When BEFORE_MARKERS, all overlays beginning/ending at POS are + treated as if their front_advance/rear_advance was true. */ void -itree_insert_gap (struct itree_tree *tree, - ptrdiff_t pos, ptrdiff_t length) +itree_insert_gap (struct itree_tree *tree, ptrdiff_t pos, + ptrdiff_t length, bool before_markers) { if (!tree || length <= 0 || tree->root == NULL) return; @@ -1202,14 +1205,16 @@ itree_insert_gap (struct itree_tree *tree, /* FIXME: Don't allocate iterator/stack anew every time. */ - /* Nodes with front_advance starting at pos may mess up the tree - order, so we need to remove them first. */ + /* Nodes starting at pos with front_advance (or before_markers) may + mess up the tree order, so we need to remove them first. */ struct interval_stack *saved = interval_stack_create (0); struct itree_node *node = NULL; ITREE_FOREACH (node, tree, pos, pos + 1, PRE_ORDER) { - if (node->begin == pos && node->front_advance - && (node->begin != node->end || node->rear_advance)) + if (node->begin == pos + && (node->front_advance || before_markers) + && (node->begin != node->end || node->rear_advance + || before_markers)) interval_stack_push (saved, node); } for (int i = 0; i < saved->length; ++i) @@ -1246,7 +1251,9 @@ itree_insert_gap (struct itree_tree *tree, /* node->begin == pos implies no front-advance. */ if (node->begin > pos) node->begin += length; - if (node->end > pos || (node->end == pos && node->rear_advance)) + if (node->end > pos + || (node->end == pos + && (node->rear_advance || before_markers))) { node->end += length; eassert (node != NULL); @@ -1256,7 +1263,8 @@ itree_insert_gap (struct itree_tree *tree, interval_stack_destroy (stack); } - /* Reinsert nodes starting at POS having front-advance. */ + /* Reinsert nodes starting at POS with front-advance (or + before_markers). */ uintmax_t notick = tree->otick; nodeptr_and_flag nav; while ((nav = interval_stack_pop (saved), @@ -1264,7 +1272,7 @@ itree_insert_gap (struct itree_tree *tree, { eassert (node->otick == ootick); node->begin += length; - if (node->end != pos || node->rear_advance) + if (node->end != pos || (node->rear_advance || before_markers)) node->end += length; node->otick = notick; interval_tree_insert (tree, node); diff --git a/src/itree.h b/src/itree.h index c6b68d36672..3ef9e76812a 100644 --- a/src/itree.h +++ b/src/itree.h @@ -119,7 +119,8 @@ #define ITREE_H ptrdiff_t, ptrdiff_t); extern struct itree_node *itree_remove (struct itree_tree *, struct itree_node *); -extern void itree_insert_gap (struct itree_tree *, ptrdiff_t, ptrdiff_t); +extern void itree_insert_gap (struct itree_tree *, ptrdiff_t, + ptrdiff_t, bool); extern void itree_delete_gap (struct itree_tree *, ptrdiff_t, ptrdiff_t); /* Iteration functions. Almost all code should use ITREE_FOREACH diff --git a/src/lisp.h b/src/lisp.h index d87f9549382..472472d87f8 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -4690,7 +4690,7 @@ XMODULE_FUNCTION (Lisp_Object o) extern bool mouse_face_overlay_overlaps (Lisp_Object); extern Lisp_Object disable_line_numbers_overlay_at_eob (void); extern AVOID nsberror (Lisp_Object); -extern void adjust_overlays_for_insert (ptrdiff_t, ptrdiff_t); +extern void adjust_overlays_for_insert (ptrdiff_t, ptrdiff_t, bool); extern void adjust_overlays_for_delete (ptrdiff_t, ptrdiff_t); extern void fix_start_end_in_overlays (ptrdiff_t, ptrdiff_t); extern void report_overlay_modification (Lisp_Object, Lisp_Object, bool, diff --git a/test/src/buffer-tests.el b/test/src/buffer-tests.el index a4a1f609fd2..c8bd86ca9f1 100644 --- a/test/src/buffer-tests.el +++ b/test/src/buffer-tests.el @@ -1165,6 +1165,68 @@ test-delete-all-overlay-1 (should-not (delete-all-overlays)))) +;; +==========================================================================+ +;; | insert-before-markers and overlays +;; +==========================================================================+ + +(defun buffer-tests-overlay-string (overlay) + (buffer-substring-no-properties + (overlay-start overlay) + (overlay-end overlay))) + +(ert-deftest test-overlay-insert-before-markers-at-start () + "`insert-before-markers' always advances an overlay's start. +Test both front-advance and non-front-advance overlays." + (with-temp-buffer + (insert "1234") + (let ((front-advance (make-overlay 2 3 nil t)) + (default-advance (make-overlay 2 3 nil nil))) + (goto-char 2) + (insert-before-markers "x") + (should (equal "2" (buffer-tests-overlay-string front-advance))) + (should (equal "2" (buffer-tests-overlay-string default-advance)))))) + +(ert-deftest test-overlay-insert-before-markers-at-end () + "`insert-before-markers' always advances an overlay's start. +Test both front-advance and non-front-advance overlays." + (with-temp-buffer + (insert "1234") + (let ((rear-advance (make-overlay 2 3 nil nil t)) + (default-advance (make-overlay 2 3 nil nil nil))) + (goto-char 3) + (insert-before-markers "x") + (should (equal "2x" (buffer-tests-overlay-string rear-advance))) + (should (equal "2x" (buffer-tests-overlay-string default-advance)))))) + +;; (ert-deftest test-overlay-insert-before-markers-at-start () +;; (with-temp-buffer +;; (insert "1234") +;; (let ((front-advance (make-overlay 2 3 nil 'front-advance)) +;; (default-advance (make-overlay 2 3 nil nil))) +;; (goto-char 2) +;; (insert-before-markers "x") +;; (should (equal -1 (overlay-start front-advance))) +;; (should (equal -1 (overlay-start default-advance))) +;; (should (equal -1 (overlay-end overlay))) +;; (should (equal -1 (overlay-end overlay))) +;; ))) + +;; (ert-deftest test-overlay-insert-before-markers-empty () +;; (with-temp-buffer +;; (insert "1234") +;; (let ((overlay (make-overlay 2 2))) +;; (goto-char 2) +;; (insert-before-markers "x") +;; (should (equal 3 (overlay-end overlay)))))) + +;; (ert-deftest test-overlay-insert-before-markers-non-empty () +;; (with-temp-buffer +;; (insert "1234") +;; (let ((overlay (make-overlay 2 3))) +;; (goto-char 3) +;; (insert-before-markers "x") +;; (should (equal 4 (overlay-end overlay)))))) + ;; +==========================================================================+ ;; | get-pos-property ;; +==========================================================================+ -- 2.35.1 --=-=-=--