bug#59693: 29.0.50; treesitter in base buffer doesn't respond to modifications in indirect buffer correctly

[Yuan Fu <casouri@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 901 bytes --]



> On Dec 6, 2022, at 4:17 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> From: Yuan Fu <casouri@gmail.com>
>> Date: Mon, 5 Dec 2022 18:15:07 -0800
>> Cc: Stefan Monnier <monnier@iro.umontreal.ca>,
>> 59693@debbugs.gnu.org,
>> miha@kamnitnik.top
>> 
>> 1. Only allow base buffer to have parsers, no change is needed for insdel.c, treesit_record_change can find the base buffer and update its parsers. We can ask indirect buffers to use their base buffer’s parser. Unless the base buffer is narrowed, I think it will work fine. 
> 
> I think this is fine, but we need to document it.
> 
>> I remember that there were a discussion along the lines of user-narrow vs low-level narrow, what was the outcome of that discussion?
> 
> Nothing in particular, and I don't think it's relevant.  If some mode needs
> to widen, it can.
> 
> Thanks.

Here is a patch that does #1.

Yuan


[-- Attachment #2: indirect.patch --]
[-- Type: application/octet-stream, Size: 8169 bytes --]

From 92f3fe1fe6cb021b24247b518fbe852592c0852c Mon Sep 17 00:00:00 2001
From: Yuan Fu <casouri@gmail.com>
Date: Wed, 7 Dec 2022 14:50:16 -0800
Subject: [PATCH] Make indirect buffers use tree-sitter parsers in their base
 buffer

* src/treesit.c (treesit_record_change): Always use the base buffer.
(Ftreesit_parser_create): Always use the base buffer.  Also change the
for loop into FOR_EACH_TAIL (stylistic change).
(Ftreesit_parser_list): Always use the base buffer.

* doc/lispref/parsing.texi (Using Parser): Update manual.
* test/src/treesit-tests.el (treesit-indirect-buffer): New test.
---
 doc/lispref/parsing.texi  | 12 ++++++++-
 src/treesit.c             | 56 +++++++++++++++++++++++++++------------
 test/src/treesit-tests.el | 34 ++++++++++++++++++++++++
 3 files changed, 84 insertions(+), 18 deletions(-)

diff --git a/doc/lispref/parsing.texi b/doc/lispref/parsing.texi
index 3223875320a..98582227fd7 100644
--- a/doc/lispref/parsing.texi
+++ b/doc/lispref/parsing.texi
@@ -409,6 +409,14 @@ Using Parser
 By default, this function reuses a parser if one already exists for
 @var{language} in @var{buffer}, but if @var{no-reuse} is
 non-@code{nil}, this function always creates a new parser.
+
+If @var{buffer} (or the current buffer) is an indirect buffer, its
+base buffer is used instead.  That is, indirect buffers uses their
+base buffer's parsers.  If the base buffer is narrowed, an indirect
+buffer might not be able to retrieve information of the portion of the
+buffer text that are invisible in the base buffer.  Lisp programs
+should widen as necessary should they want to use a parser in an
+indirect buffer.
 @end defun
 
 Given a parser, we can query information about it.
@@ -447,7 +455,9 @@ Using Parser
 @defun treesit-parser-list &optional buffer
 This function returns the parser list of @var{buffer}.  If
 @var{buffer} is @code{nil} or omitted, it defaults to the current
-buffer.
+buffer.  If @var{buffer} (or the current buffer) is an indirect
+buffer, its base buffer is used instead.  That is, indirect buffers
+uses their base buffer's parsers.
 @end defun
 
 @defun treesit-parser-delete parser
diff --git a/src/treesit.c b/src/treesit.c
index 8b485ca4ece..74370cd772d 100644
--- a/src/treesit.c
+++ b/src/treesit.c
@@ -384,7 +384,10 @@ #define ts_tree_root_node fn_ts_tree_root_node
    mysteriously drops.  3) what if a user uses so many stuff that the
    default cache size (20) is not enough and we end up thrashing?
    These are all imaginary scenarios but they are not impossible
-   :-) */
+   :-)
+
+   Parsers in indirect buffers: We make indirect buffers to share the
+   parser of its base buffer.  See bug#59693 for reasoning.  */
 
 \f
 /*** Initialization */
@@ -697,9 +700,10 @@ treesit_tree_edit_1 (TSTree *tree, ptrdiff_t start_byte,
 treesit_record_change (ptrdiff_t start_byte, ptrdiff_t old_end_byte,
 		       ptrdiff_t new_end_byte)
 {
-  Lisp_Object parser_list;
-
-  parser_list = BVAR (current_buffer, ts_parser_list);
+  struct buffer *base_buffer = current_buffer;
+  if (current_buffer->base_buffer)
+    base_buffer = current_buffer->base_buffer;
+  Lisp_Object parser_list = BVAR (base_buffer, ts_parser_list);
 
   FOR_EACH_TAIL_SAFE (parser_list)
     {
@@ -1252,12 +1256,19 @@ DEFUN ("treesit-parser-create",
        1, 3, 0,
        doc: /* Create and return a parser in BUFFER for LANGUAGE.
 
-The parser is automatically added to BUFFER's parser list, as
-returned by `treesit-parser-list'.
-LANGUAGE is a language symbol.  If BUFFER is nil or omitted, it
-defaults to the current buffer.  If BUFFER already has a parser for
-LANGUAGE, return that parser, but if NO-REUSE is non-nil, always
-create a new parser.  */)
+The parser is automatically added to BUFFER's parser list, as returned
+by `treesit-parser-list'.  LANGUAGE is a language symbol.  If BUFFER
+is nil or omitted, it defaults to the current buffer.  If BUFFER
+already has a parser for LANGUAGE, return that parser, but if NO-REUSE
+is non-nil, always create a new parser.
+
+If BUFFER (or the current buffer) is an indirect buffer, its base
+buffer is used instead.  That is, indirect buffers uses their base
+buffer's parsers.  If the base buffer is narrowed, an indirect buffer
+might not be able to retrieve information of the portion of the buffer
+text that are invisible in the base buffer.  Lisp programs should
+widen as necessary should they want to use a parser in an indirect
+buffer.  */)
   (Lisp_Object language, Lisp_Object buffer, Lisp_Object no_reuse)
 {
   treesit_initialize ();
@@ -1271,16 +1282,21 @@ DEFUN ("treesit-parser-create",
       CHECK_BUFFER (buffer);
       buf = XBUFFER (buffer);
     }
+  if (buf->base_buffer)
+    buf = buf->base_buffer;
+
   treesit_check_buffer_size (buf);
 
   /* See if we can reuse a parser.  */
-  for (Lisp_Object tail = BVAR (buf, ts_parser_list);
-       NILP (no_reuse) && !NILP (tail);
-       tail = XCDR (tail))
+  if (NILP (no_reuse))
     {
-      struct Lisp_TS_Parser *parser = XTS_PARSER (XCAR (tail));
-      if (EQ (parser->language_symbol, language))
-	return XCAR (tail);
+      Lisp_Object tail = BVAR (buf, ts_parser_list);
+      FOR_EACH_TAIL (tail)
+      {
+	struct Lisp_TS_Parser *parser = XTS_PARSER (XCAR (tail));
+	if (EQ (parser->language_symbol, language))
+	  return XCAR (tail);
+      }
     }
 
   /* Load language.  */
@@ -1329,7 +1345,10 @@ DEFUN ("treesit-parser-list",
        Ftreesit_parser_list, Streesit_parser_list,
        0, 1, 0,
        doc: /* Return BUFFER's parser list.
-BUFFER defaults to the current buffer.  */)
+
+BUFFER defaults to the current buffer.  If BUFFER (or the current
+buffer) is an indirect buffer, its base buffer is used instead.  That
+is, indirect buffers uses their base buffer's parsers.  */)
   (Lisp_Object buffer)
 {
   struct buffer *buf;
@@ -1340,6 +1359,9 @@ DEFUN ("treesit-parser-list",
       CHECK_BUFFER (buffer);
       buf = XBUFFER (buffer);
     }
+  if (buf->base_buffer)
+    buf = buf->base_buffer;
+
   /* Return a fresh list so messing with that list doesn't affect our
      internal data.  */
   Lisp_Object return_list = Qnil;
diff --git a/test/src/treesit-tests.el b/test/src/treesit-tests.el
index aba12759c34..1cc2217bd3b 100644
--- a/test/src/treesit-tests.el
+++ b/test/src/treesit-tests.el
@@ -161,6 +161,40 @@ treesit-node-api
       (should (treesit-node-eq root-node root-node))
       (should (not (treesit-node-eq root-node doc-node))))))
 
+(ert-deftest treesit-indirect-buffer ()
+  "Tests for indirect buffers."
+  (skip-unless (treesit-language-available-p 'json))
+  (let ((base (get-buffer-create "*treesit test*"))
+        parser indirect)
+    (unwind-protect
+        (progn
+          (with-current-buffer base
+            (setq indirect (clone-indirect-buffer "*treesit test 1*" nil)))
+          (with-current-buffer indirect
+            (setq parser (treesit-parser-create 'json)))
+          ;; 1. Parser created in the indirect buffer should be
+          ;; actually be created in the base buffer.
+          (with-current-buffer base
+            (should (equal (list parser)
+                           (treesit-parser-list)))
+            (insert "[1,2,3]"))
+          ;; Change in the base buffer should be reflected in the
+          ;; indirect buffer.
+          (with-current-buffer indirect
+            (should (eq (treesit-node-end
+                         (treesit-buffer-root-node))
+                        8))
+            (erase-buffer))
+          ;; Change in the indirect buffer should be reflected in the
+          ;; base buffer.
+          (with-current-buffer base
+            (should (eq (treesit-node-end
+                         (treesit-buffer-root-node))
+                        1))
+            (erase-buffer)))
+      (kill-buffer base)
+      (kill-buffer indirect))))
+
 (ert-deftest treesit-query-api ()
   "Tests for query API."
   (skip-unless (treesit-language-available-p 'json))
-- 
2.33.1


[-- Attachment #3: Type: text/plain, Size: 2 bytes --]