GNU bug report logs - #68445
[PATCH] Problem with python--treesit-syntax-propertize

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 68445 in the body.
You can then email your comments to 68445 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: kobarity <kobarity <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Problem with python--treesit-syntax-propertize
Date: Sun, 14 Jan 2024 18:15:07 +0900

[Message part 1 (text/plain, inline)]

Hi,

I found a problem with python--treesit-syntax-propertize recently
introduced by the Bug#67977 patch.

1. emacs -Q
2. Open a file in python-ts-mode with the following contents:

#+begin_src python
"""Docstring.

test.
"""
S = """string."""
#+end_src

3. Locate the point on the third line.
4. M-q
5. An empty line will be inserted.
6. M-q
7. The string literal on the last line will be split as follows:

S = ""

"string."""

This problem does not occur in python-mode.

The direct cause of this problem is that the string-delimiter property
set in the docstring is removed.  python--treesit-syntax-propertize is
called to set the property, but it fails to set it properly.  Here is
the trace of python--treesit-syntax-propertize from step 4 above.

======================================================================
1 -> (python--treesit-syntax-propertize 1 45)
1 <- python--treesit-syntax-propertize: nil
======================================================================
1 -> (python--treesit-syntax-propertize 16 45)
1 <- python--treesit-syntax-propertize: nil

python--treesit-syntax-propertize is called with argument START 16.
This is the position inside the docstring.

It seems to me that python--treesit-syntax-propertize assumes that the
START argument is outside the triple-quoted string.  So one solution
might be to change START to the start of the string if it is within a
string, as in the attached patch.  However, I'm not sure this is the
right approach.  Should we use
syntax-propertize-extend-region-functions?

--
In GNU Emacs 30.0.50 (build 5, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.16.0, Xaw scroll bars) of 2024-01-13 built on ubuntu
Repository revision: 106cd9aafe8248ef91d7e89161adc5f912ea54eb
Repository branch: master
System Description: Ubuntu 22.04.3 LTS

[0001-Fix-python-treesit-syntax-propertize.patch (text/plain, attachment)]

Message #8 received at 68445 <at> debbugs.gnu.org (full text, mbox):

From: kobarity <kobarity <at> gmail.com>
To: Yuan Fu <casouri <at> gmail.com>, Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 68445 <at> debbugs.gnu.org
Subject: Re: [PATCH] Problem with python--treesit-syntax-propertize
Date: Sun, 21 Jan 2024 23:47:39 +0900

I am resending my mail, as I made a mistake in X-Debbugs-CC.

I wrote:
> Hi,
> 
> I found a problem with python--treesit-syntax-propertize recently
> introduced by the Bug#67977 patch.
> 
> 1. emacs -Q
> 2. Open a file in python-ts-mode with the following contents:
> 
> #+begin_src python
> """Docstring.
> 
> test.
> """
> S = """string."""
> #+end_src
> 
> 3. Locate the point on the third line.
> 4. M-q
> 5. An empty line will be inserted.
> 6. M-q
> 7. The string literal on the last line will be split as follows:
> 
> S = ""
> 
> "string."""
> 
> This problem does not occur in python-mode.
> 
> The direct cause of this problem is that the string-delimiter property
> set in the docstring is removed.  python--treesit-syntax-propertize is
> called to set the property, but it fails to set it properly.  Here is
> the trace of python--treesit-syntax-propertize from step 4 above.
> 
> ======================================================================
> 1 -> (python--treesit-syntax-propertize 1 45)
> 1 <- python--treesit-syntax-propertize: nil
> ======================================================================
> 1 -> (python--treesit-syntax-propertize 16 45)
> 1 <- python--treesit-syntax-propertize: nil
> 
> python--treesit-syntax-propertize is called with argument START 16.
> This is the position inside the docstring.
> 
> It seems to me that python--treesit-syntax-propertize assumes that the
> START argument is outside the triple-quoted string.  So one solution
> might be to change START to the start of the string if it is within a
> string, as in the attached patch.  However, I'm not sure this is the
> right approach.  Should we use
> syntax-propertize-extend-region-functions?
> 
> --
> In GNU Emacs 30.0.50 (build 5, x86_64-pc-linux-gnu, X toolkit, cairo
>  version 1.16.0, Xaw scroll bars) of 2024-01-13 built on ubuntu
> Repository revision: 106cd9aafe8248ef91d7e89161adc5f912ea54eb
> Repository branch: master
> System Description: Ubuntu 22.04.3 LTS

I appreciate your comments.

Message #11 received at 68445 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: kobarity <kobarity <at> gmail.com>, Yuan Fu <casouri <at> gmail.com>
Cc: 68445 <at> debbugs.gnu.org
Subject: Re: [PATCH] Problem with python--treesit-syntax-propertize
Date: Sun, 21 Jan 2024 20:09:03 +0200

Hi!

On 21/01/2024 16:47, kobarity wrote:
> 
> I am resending my mail, as I made a mistake in X-Debbugs-CC.

Was it supposed to appear in the bug's thread? I don't see it anywhere.

> I wrote:
>> Hi,
>>
>> I found a problem with python--treesit-syntax-propertize recently
>> introduced by the Bug#67977 patch.
>>
>> 1. emacs -Q
>> 2. Open a file in python-ts-mode with the following contents:
>>
>> #+begin_src python
>> """Docstring.
>>
>> test.
>> """
>> S = """string."""
>> #+end_src
>>
>> 3. Locate the point on the third line.
>> 4. M-q
>> 5. An empty line will be inserted.
>> 6. M-q
>> 7. The string literal on the last line will be split as follows:
>>
>> S = ""
>>
>> "string."""
>>
>> This problem does not occur in python-mode.
>>
>> The direct cause of this problem is that the string-delimiter property
>> set in the docstring is removed.  python--treesit-syntax-propertize is
>> called to set the property, but it fails to set it properly.  Here is
>> the trace of python--treesit-syntax-propertize from step 4 above.
>>
>> ======================================================================
>> 1 -> (python--treesit-syntax-propertize 1 45)
>> 1 <- python--treesit-syntax-propertize: nil
>> ======================================================================
>> 1 -> (python--treesit-syntax-propertize 16 45)
>> 1 <- python--treesit-syntax-propertize: nil
>>
>> python--treesit-syntax-propertize is called with argument START 16.
>> This is the position inside the docstring.
>>
>> It seems to me that python--treesit-syntax-propertize assumes that the
>> START argument is outside the triple-quoted string.  So one solution
>> might be to change START to the start of the string if it is within a
>> string, as in the attached patch.  However, I'm not sure this is the
>> right approach.

Sounds good to me. I don't see the patch, though, or where to read it.

>> Should we use
>> syntax-propertize-extend-region-functions?

That's another option, but it shouldn't be necessary. After all, the 
absence of a notification from the parser (which would extend the range) 
should mean that the node before position 16 is untouched, so there's no 
real need to clear the properties there.

I think there is also another approach--handle two different types of 
nodes separately, instead of just string_content, so we don't have to 
start from the beginning of the literal. Like this:

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index e2f614f52c2..4f8b0cb9473 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -1361,13 +1361,15 @@ python--treesit-syntax-propertize
     (while (re-search-forward (rx (or "\"\"\"" "'''")) end t)
       (let ((node (treesit-node-at (point))))
         ;; The triple quotes surround a non-empty string.
-        (when (equal (treesit-node-type node) "string_content")
-          (let ((start (treesit-node-start node))
-                (end (treesit-node-end node)))
-            (put-text-property (1- start) start
-                               'syntax-table (string-to-syntax "|"))
-            (put-text-property end (min (1+ end) (point-max))
-                               'syntax-table (string-to-syntax "|"))))))))
+        (cond
+         ((equal (treesit-node-type node) "string_content")
+          (put-text-property (1- (treesit-node-start node))
+                             (treesit-node-start node)
+                             'syntax-table (string-to-syntax "|")))
+         ((and (equal (treesit-node-type node) "string_end")
+               (= (treesit-node-start node) (- (point) 3)))
+          (put-text-property (- (point) 3) (- (point) 2)
+                             'syntax-table (string-to-syntax "|"))))))))

 
 ;;; Indentation

Message #14 received at 68445 <at> debbugs.gnu.org (full text, mbox):

From: kobarity <kobarity <at> gmail.com>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: Yuan Fu <casouri <at> gmail.com>, 68445 <at> debbugs.gnu.org
Subject: Re: [PATCH] Problem with python--treesit-syntax-propertize
Date: Tue, 23 Jan 2024 00:44:16 +0900

Hi,

Dmitry Gutov wrote:
> On 21/01/2024 16:47, kobarity wrote:
> > I am resending my mail, as I made a mistake in X-Debbugs-CC.
> Was it supposed to appear in the bug's thread? I don't see it anywhere.

My first mail was registered as Bug#68445, and my patch is there.

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=68445

It says:

Report forwarded to casouri <at> gmail.com, dmitry@.gutov.dev, bug-gnu-emacs <at> gnu.org:

The extra period is my mistake and it may have caused the problem.
I'm sorry for the confusion.

> I think there is also another approach--handle two different types of
> nodes separately, instead of just string_content, so we don't have to
> start from the beginning of the literal. Like this:
> 
> diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
> index e2f614f52c2..4f8b0cb9473 100644
> --- a/lisp/progmodes/python.el
> +++ b/lisp/progmodes/python.el
> @@ -1361,13 +1361,15 @@ python--treesit-syntax-propertize
>      (while (re-search-forward (rx (or "\"\"\"" "'''")) end t)
>        (let ((node (treesit-node-at (point))))
>          ;; The triple quotes surround a non-empty string.
> -        (when (equal (treesit-node-type node) "string_content")
> -          (let ((start (treesit-node-start node))
> -                (end (treesit-node-end node)))
> -            (put-text-property (1- start) start
> -                               'syntax-table (string-to-syntax "|"))
> -            (put-text-property end (min (1+ end) (point-max))
> -                               'syntax-table (string-to-syntax "|"))))))))
> +        (cond
> +         ((equal (treesit-node-type node) "string_content")
> +          (put-text-property (1- (treesit-node-start node))
> +                             (treesit-node-start node)
> +                             'syntax-table (string-to-syntax "|")))
> +         ((and (equal (treesit-node-type node) "string_end")
> +               (= (treesit-node-start node) (- (point) 3)))
> +          (put-text-property (- (point) 3) (- (point) 2)
> +                             'syntax-table (string-to-syntax "|"))))))))
> 
>  
>  ;;; Indentation
> 

This approach seems better than my patch, but it does not seem to
address the following special case.

#+begin_src python
"""a""""""b"""
#+end_src

Message #17 received at 68445 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: kobarity <kobarity <at> gmail.com>
Cc: Yuan Fu <casouri <at> gmail.com>, 68445 <at> debbugs.gnu.org
Subject: Re: [PATCH] Problem with python--treesit-syntax-propertize
Date: Mon, 22 Jan 2024 20:52:58 +0200

On 22/01/2024 17:44, kobarity wrote:
> Hi,
> 
> Dmitry Gutov wrote:
>> On 21/01/2024 16:47, kobarity wrote:
>>> I am resending my mail, as I made a mistake in X-Debbugs-CC.
>> Was it supposed to appear in the bug's thread? I don't see it anywhere.
> 
> My first mail was registered as Bug#68445, and my patch is there.
> 
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=68445
> 
> It says:
> 
> Report forwarded to casouri <at> gmail.com, dmitry@.gutov.dev, bug-gnu-emacs <at> gnu.org:
> 
> The extra period is my mistake and it may have caused the problem.
> I'm sorry for the confusion.

Yeah, but even so that's odd: I'm subscribed to the bug tracker, so the 
email should have at least arrived in my inbox, but it did not.

>> I think there is also another approach--handle two different types of
>> nodes separately, instead of just string_content, so we don't have to
>> start from the beginning of the literal. Like this:
>>
>> diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
>> index e2f614f52c2..4f8b0cb9473 100644
>> --- a/lisp/progmodes/python.el
>> +++ b/lisp/progmodes/python.el
>> @@ -1361,13 +1361,15 @@ python--treesit-syntax-propertize
>>       (while (re-search-forward (rx (or "\"\"\"" "'''")) end t)
>>         (let ((node (treesit-node-at (point))))
>>           ;; The triple quotes surround a non-empty string.
>> -        (when (equal (treesit-node-type node) "string_content")
>> -          (let ((start (treesit-node-start node))
>> -                (end (treesit-node-end node)))
>> -            (put-text-property (1- start) start
>> -                               'syntax-table (string-to-syntax "|"))
>> -            (put-text-property end (min (1+ end) (point-max))
>> -                               'syntax-table (string-to-syntax "|"))))))))
>> +        (cond
>> +         ((equal (treesit-node-type node) "string_content")
>> +          (put-text-property (1- (treesit-node-start node))
>> +                             (treesit-node-start node)
>> +                             'syntax-table (string-to-syntax "|")))
>> +         ((and (equal (treesit-node-type node) "string_end")
>> +               (= (treesit-node-start node) (- (point) 3)))
>> +          (put-text-property (- (point) 3) (- (point) 2)
>> +                             'syntax-table (string-to-syntax "|"))))))))
>>
>>   
>>   ;;; Indentation
>>
> 
> This approach seems better than my patch, but it does not seem to
> address the following special case.
> 
> #+begin_src python
> """a""""""b"""
> #+end_src

All right, try the patch below, please. It also covers the case of the 
empty literal.

I've tried to find a case where it would behave poorly (e.g. by 
misdetecting three quotes from a combination of some other string 
literals), but couldn't. E.g.,

  s = '''asdasd'

is not a concatenation. It's always an error, at least according to the 
TS grammar.

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index e2f614f52c2..41f612c8b1c 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -1359,15 +1359,15 @@ python--treesit-syntax-propertize
   (save-excursion
     (goto-char start)
     (while (re-search-forward (rx (or "\"\"\"" "'''")) end t)
-      (let ((node (treesit-node-at (point))))
-        ;; The triple quotes surround a non-empty string.
-        (when (equal (treesit-node-type node) "string_content")
-          (let ((start (treesit-node-start node))
-                (end (treesit-node-end node)))
-            (put-text-property (1- start) start
-                               'syntax-table (string-to-syntax "|"))
-            (put-text-property end (min (1+ end) (point-max))
-                               'syntax-table (string-to-syntax "|"))))))))
+      (let ((node (treesit-node-at (- (point) 3))))
+        ;; Handle triple-quoted strings.
+        (pcase (treesit-node-type node)
+          ("string_start"
+           (put-text-property (1- (point)) (point)
+                              'syntax-table (string-to-syntax "|")))
+          ("string_end"
+           (put-text-property (- (point) 3) (- (point) 2)
+                              'syntax-table (string-to-syntax "|"))))))))

 
 ;;; Indentation

Message #20 received at 68445 <at> debbugs.gnu.org (full text, mbox):

From: kobarity <kobarity <at> gmail.com>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: Yuan Fu <casouri <at> gmail.com>, 68445 <at> debbugs.gnu.org
Subject: Re: [PATCH] Problem with python--treesit-syntax-propertize
Date: Tue, 23 Jan 2024 23:14:42 +0900

Dmitry Gutov wrote:
> 
> On 22/01/2024 17:44, kobarity wrote:
> > Hi,
> > 
> > Dmitry Gutov wrote:
> >> On 21/01/2024 16:47, kobarity wrote:
> >>> I am resending my mail, as I made a mistake in X-Debbugs-CC.
> >> Was it supposed to appear in the bug's thread? I don't see it anywhere.
> > 
> > My first mail was registered as Bug#68445, and my patch is there.
> > 
> > https://debbugs.gnu.org/cgi/bugreport.cgi?bug=68445
> > 
> > It says:
> > 
> > Report forwarded to casouri <at> gmail.com, dmitry@.gutov.dev, bug-gnu-emacs <at> gnu.org:
> > 
> > The extra period is my mistake and it may have caused the problem.
> > I'm sorry for the confusion.
> 
> Yeah, but even so that's odd: I'm subscribed to the bug tracker, so
> the email should have at least arrived in my inbox, but it did not.

I agree.  I can't find my first mail in the bug-gnu-emacs archive.

> >> I think there is also another approach--handle two different types of
> >> nodes separately, instead of just string_content, so we don't have to
> >> start from the beginning of the literal. Like this:
> >> 
> >> diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
> >> index e2f614f52c2..4f8b0cb9473 100644
> >> --- a/lisp/progmodes/python.el
> >> +++ b/lisp/progmodes/python.el
> >> @@ -1361,13 +1361,15 @@ python--treesit-syntax-propertize
> >>       (while (re-search-forward (rx (or "\"\"\"" "'''")) end t)
> >>         (let ((node (treesit-node-at (point))))
> >>           ;; The triple quotes surround a non-empty string.
> >> -        (when (equal (treesit-node-type node) "string_content")
> >> -          (let ((start (treesit-node-start node))
> >> -                (end (treesit-node-end node)))
> >> -            (put-text-property (1- start) start
> >> -                               'syntax-table (string-to-syntax "|"))
> >> -            (put-text-property end (min (1+ end) (point-max))
> >> -                               'syntax-table (string-to-syntax "|"))))))))
> >> +        (cond
> >> +         ((equal (treesit-node-type node) "string_content")
> >> +          (put-text-property (1- (treesit-node-start node))
> >> +                             (treesit-node-start node)
> >> +                             'syntax-table (string-to-syntax "|")))
> >> +         ((and (equal (treesit-node-type node) "string_end")
> >> +               (= (treesit-node-start node) (- (point) 3)))
> >> +          (put-text-property (- (point) 3) (- (point) 2)
> >> +                             'syntax-table (string-to-syntax "|"))))))))
> >> 
> >>   
> >>   ;;; Indentation
> >> 
> > 
> > This approach seems better than my patch, but it does not seem to
> > address the following special case.
> > 
> > #+begin_src python
> > """a""""""b"""
> > #+end_src
> 
> All right, try the patch below, please. It also covers the case of the
> empty literal.

Thanks, it looks good to me.

> I've tried to find a case where it would behave poorly (e.g. by
> misdetecting three quotes from a combination of some other string
> literals), but couldn't. E.g.,
> 
>   s = '''asdasd'
> 
> is not a concatenation. It's always an error, at least according to
> the TS grammar.

I think the TS grammar is correct, because this example is also an
error according to the Python interpreter.

Message #25 received at 68445-done <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: kobarity <kobarity <at> gmail.com>
Cc: Yuan Fu <casouri <at> gmail.com>, 68445-done <at> debbugs.gnu.org
Subject: Re: [PATCH] Problem with python--treesit-syntax-propertize
Date: Fri, 26 Jan 2024 03:15:24 +0200

On 23/01/2024 16:14, kobarity wrote:
> 
> Dmitry Gutov wrote:
>>
>> On 22/01/2024 17:44, kobarity wrote:
>>> Hi,
>>>
>>> Dmitry Gutov wrote:
>>>> On 21/01/2024 16:47, kobarity wrote:
>>>>> I am resending my mail, as I made a mistake in X-Debbugs-CC.
>>>> Was it supposed to appear in the bug's thread? I don't see it anywhere.
>>>
>>> My first mail was registered as Bug#68445, and my patch is there.
>>>
>>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=68445
>>>
>>> It says:
>>>
>>> Report forwarded to casouri <at> gmail.com, dmitry@.gutov.dev, bug-gnu-emacs <at> gnu.org:
>>>
>>> The extra period is my mistake and it may have caused the problem.
>>> I'm sorry for the confusion.
>>
>> Yeah, but even so that's odd: I'm subscribed to the bug tracker, so
>> the email should have at least arrived in my inbox, but it did not.
> 
> I agree.  I can't find my first mail in the bug-gnu-emacs archive.
> 
>>>> I think there is also another approach--handle two different types of
>>>> nodes separately, instead of just string_content, so we don't have to
>>>> start from the beginning of the literal. Like this:
>>>>
>>>> diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
>>>> index e2f614f52c2..4f8b0cb9473 100644
>>>> --- a/lisp/progmodes/python.el
>>>> +++ b/lisp/progmodes/python.el
>>>> @@ -1361,13 +1361,15 @@ python--treesit-syntax-propertize
>>>>        (while (re-search-forward (rx (or "\"\"\"" "'''")) end t)
>>>>          (let ((node (treesit-node-at (point))))
>>>>            ;; The triple quotes surround a non-empty string.
>>>> -        (when (equal (treesit-node-type node) "string_content")
>>>> -          (let ((start (treesit-node-start node))
>>>> -                (end (treesit-node-end node)))
>>>> -            (put-text-property (1- start) start
>>>> -                               'syntax-table (string-to-syntax "|"))
>>>> -            (put-text-property end (min (1+ end) (point-max))
>>>> -                               'syntax-table (string-to-syntax "|"))))))))
>>>> +        (cond
>>>> +         ((equal (treesit-node-type node) "string_content")
>>>> +          (put-text-property (1- (treesit-node-start node))
>>>> +                             (treesit-node-start node)
>>>> +                             'syntax-table (string-to-syntax "|")))
>>>> +         ((and (equal (treesit-node-type node) "string_end")
>>>> +               (= (treesit-node-start node) (- (point) 3)))
>>>> +          (put-text-property (- (point) 3) (- (point) 2)
>>>> +                             'syntax-table (string-to-syntax "|"))))))))
>>>>
>>>>    
>>>>    ;;; Indentation
>>>>
>>>
>>> This approach seems better than my patch, but it does not seem to
>>> address the following special case.
>>>
>>> #+begin_src python
>>> """a""""""b"""
>>> #+end_src
>>
>> All right, try the patch below, please. It also covers the case of the
>> empty literal.
> 
> Thanks, it looks good to me.
> 
>> I've tried to find a case where it would behave poorly (e.g. by
>> misdetecting three quotes from a combination of some other string
>> literals), but couldn't. E.g.,
>>
>>    s = '''asdasd'
>>
>> is not a concatenation. It's always an error, at least according to
>> the TS grammar.
> 
> I think the TS grammar is correct, because this example is also an
> error according to the Python interpreter.

Thanks for testing! Installed, and closing.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.