GNU bug report logs - #56135
29.0.50; python.el - forward-sexp regression over triple-quoted strings

Previous Next

To reply to this bug, email your comments to 56135 AT debbugs.gnu.org.

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

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

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 29.0.50; python.el - forward-sexp regression over triple-quoted
 strings
Date: Wed, 22 Jun 2022 08:50:44 +0200

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

Hi!

I believe this commit:

2021-09-21 "Make syntax-ppss more accurate for Python triple quotes
(bug#49518)" (0646c68171)

… introduced a regression for users who set python-forward-sexp-function
to nil (or users who set forward-sexp-function buffer-locally, as was
suggested by python.el's commentary before Emacs 28.1).

To reproduce:

$ echo '"""Quotes for days"""' > repro.py
$ emacs -Q -eval '(setq python-forward-sexp-function nil)' repro.py
; C-M-f

- Point should move to position 22,
- As of the above commit, it lies at position 3.

(Note: if one visits the file before setting the user option, the
setting will not be taken into account; to reproduce, one should then
either set forward-sexp-function directly, or revert the buffer)

I've attached an ert test for this recipe; it passes when run against
0646c68171^ (i.e. the parent commit) and fails as of 0646c68171.

I haven't started to dig into what the change meant to fix, so I cannot
say whether TRT would be to keep the change and improve on it or revert
the change and fix the original problem some other way.

Let me know if I can assist further; thanks for your time.

[ert-python-forward-sexp.patch (text/x-patch, attachment)]

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

In GNU Emacs 29.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.33, cairo version 1.16.0)
 of 2022-05-08 built on amdahl30
Repository revision: e8ed4317e879683872d956cc919d0957fff18531
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101003
System Description: openSUSE Tumbleweed

Configured using:
 'configure --with-cairo --with-gconf --with-sqlite3 --with-xinput2'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY
INOTIFY PDUMPER PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF
TOOLKIT_SCROLL_BARS WEBP X11 XDBE XIM XINPUT2 XPM GTK3 ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=local
  locale-coding-system: utf-8-unix

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

From: João Távora <joaotavora <at> gmail.com>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>, 
 56135 <at> debbugs.gnu.org
Cc: Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#56135: 29.0.50; python.el - forward-sexp regression over
 triple-quoted strings
Date: Wed, 22 Jun 2022 23:19:40 +0100

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

On Wed, Jun 22, 2022 at 10:48 PM Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
wrote:

> Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:
>
> > João Távora <joaotavora <at> gmail.com> writes:
> >
> >> On a very superficial analysis, I would say you're _not_ experiencing a
> >> bug, because setting the f-sexp-f to nil buffer-locally would mean
> >> you don't want any python-specific behaviour for triple quotes.
> >>
> >> It might be a regression for a previous state of the working tree, but
> then
> >> again that state seems to have been buggy itself, undoubtedly, at least
> as
> >> described by myself in 0646c6817139.
> >>
> >> Why are you setting python-forward-sexp-function to nil?
> >
> > [wall of text follows]
>
> (
>     Hadn't realized we had taken this off-list.  FWIW if you found all
>     my verbiage informative, don't hesitate to (tell me to) forward it
>     back on the bug tracker.
> )
>

Ups.  That was my mistake.  Wondering what to do now.  Should we re-send to
the
tracker.  Or let future generations wonder about the missing great wall of
text.

Anyway, I did find your wall of text informative, particularly this point.

> Wondering what the best way forward would be.  Maybe a third suggested
> value for python-forward-sexp-function, which would handle triple-quotes
> but not add those "implicit paired delimiters"?

Yes, I think this makes sense.  Let's see what other think, but a patch
that extracts only that bit from the current functionality and puts it in
a third value would be a good idea, I think.

João

[Message part 2 (text/html, inline)]

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

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: João Távora <joaotavora <at> gmail.com>
Cc: 56135 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#56135: 29.0.50; python.el - forward-sexp regression over
 triple-quoted strings
Date: Thu, 23 Jun 2022 08:49:31 +0200

João Távora <joaotavora <at> gmail.com> writes:

> On Wed, Jun 22, 2022 at 10:48 PM Kévin Le Gouguec <kevin.legouguec <at> gmail.com> wrote:
>
>  Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:
>
>  > João Távora <joaotavora <at> gmail.com> writes:
>  >
>  >> Why are you setting python-forward-sexp-function to nil?
>  >
>  > [wall of text follows]
>
>  (
>      Hadn't realized we had taken this off-list.  FWIW if you found all
>      my verbiage informative, don't hesitate to (tell me to) forward it
>      back on the bug tracker.
>  )
>
> Ups.  That was my mistake.  Wondering what to do now.  Should we re-send to the 
> tracker.  Or let future generations wonder about the missing great wall of text.

😁 I'll forward it in a separate message, in case someone finds other
points to bounce off of.

> Anyway, I did find your wall of text informative, particularly this point.
>
>> Wondering what the best way forward would be.  Maybe a third suggested
>> value for python-forward-sexp-function, which would handle triple-quotes
>> but not add those "implicit paired delimiters"?
>
> Yes, I think this makes sense.  Let's see what other think, but a patch
> that extracts only that bit from the current functionality and puts it in 
> a third value would be a good idea, I think.

Mm.  I've stepped through python-nav--forward-sexp with Edebug, and
there wasn't any obvious "triple-quote handler" to extract.  IIUC

* being on either end of a triple-quote block satisfies
  python-info-{beginning,end}-of-statement-p, so 'context' becomes
  'statement-{start,end},

* (then we dance a little gigue to compute next-sexp-{context,pos},
  which we do not end up using in this scenario,)

* eventually we call python-nav-{end,beginning}-of-statement.

So the "magic" happens in those functions IIUC.  E.g. in
python-nav-end-of-statement we either 

* skip the whole cond form because the initial (goto-char
  (line-end-position)) took us at EOL after a single-line string, or…

* go down the (setq string-start (python-syntax-context 'string)) path
  for multiline strings, or…

* … oh!  Mm, well, here's a bug maybe?  Consider:

"""string 1"""

def foo():
    """string 2

    (multiline).
    """
    print("""string 3""")
          ^

So *with python-forward-sexp-function set to its default value*
(python-nav-forward-sexp), with point on the ^ mark (at the beginning of
"""string 3"""),

* before commit 0646c68, C-M-f moved to the end of """string 3""",
* now it just moves over one pair of quotes.

(Feel like I should note that single-line triple-quoted strings have
their uses FWIW, since they let one write single and double quotes into
the string without the need to backslash-escape them)

I see that the new commentary in python-syntax-stringify addresses this,
I guess?

>           ;; FIXME: This makes sexp-movement a bit suboptimal since """a"""
>           ;; is now treated as 3 strings.

So AFAICT while the change meant to improve the situation with
electric-pair-mode (which I'm grateful for, since I'm a happy user of
that minor mode), it did make sexp-movement a bit more "janky" (meaning
"hard to predict and rely on", no offense meant), *regardless* of
whether users (un)set (python-)forward-sexp-function 😕

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

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: 56135 <at> debbugs.gnu.org
Subject: [Kévin Le Gouguec] Re: bug#56135: 29.0.50;
 python.el - forward-sexp regression over triple-quoted strings
Date: Thu, 23 Jun 2022 08:53:41 +0200

(Off-list message that João already replied to in
<CALDnm53bj4rWzkP50L7TdwF3CRpy4rST3oy23bujWgC9YCJnAA <at> mail.gmail.com>;
re-sent for archival)

-------------------- Start of forwarded message --------------------
From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: João Távora <joaotavora <at> gmail.com>
Cc: Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#56135: 29.0.50; python.el - forward-sexp regression over
 triple-quoted strings
Date: Wed, 22 Jun 2022 22:39:42 +0200

João Távora <joaotavora <at> gmail.com> writes:

> On a very superficial analysis, I would say you're _not_ experiencing a 
> bug, because setting the f-sexp-f to nil buffer-locally would mean
> you don't want any python-specific behaviour for triple quotes.  
>
> It might be a regression for a previous state of the working tree, but then
> again that state seems to have been buggy itself, undoubtedly, at least as 
> described by myself in 0646c6817139.  
>
> Why are you setting python-forward-sexp-function to nil?

The motivation behind that user option (or the commentary suggesting to
set forward-sexp-function to nil before the user option existed) is to
change how python-mode's *ward-sexp commands behaves wrt blocks.
Consider this function:

    def foo():
        return bar_baz(quux, corge)

By default, when point is…

* at the start of "def": C-M-f moves to the end of the function (at the
  end of "bar_baz(quux, corge)"),

* at the end of "bar_baz(quux, corge)": C-M-b moves to the beginning of
  the function (at the start of "def").

IOW by default python-mode uses sexp commands to move across whole
blocks, even when point is on something that might be considered a sexp
in other major modes (identifiers, or paired delimiters); I imagine that
since Python blocks are based on whitespace rather than explicit
delimiters, python-mode tries to be helpful by adding "implicit paired
delimiters":

    (a)[b]def foo():(b)
           (c)return bar_baz(quux, corge)[c](a)

(Read "(x) jumps to the corresponding (x)"; using [y] with square
brackets to designate "destination points" for C-M-f/C-M-b that cannot
be used as "source" points for C-M-b/C-M-f, since they are shadowed by
(x) "source" points in the same spot)

Setting (python-)forward-sexp-function to nil was the recommended way to
take those implicit paired delimiters out of the equation[1].


So that was the situation before this commit, AFAICT: triple-quote
docstrings did not enter the picture and even with forward-sexp-function
set to nil, C-M-[bf] did TRT (skipping over them).

I can readily believe that things working the way I liked was a happy
accident and, on balance, your change fixed more than it broke.

Wondering what the best way forward would be.  Maybe a third suggested
value for python-forward-sexp-function, which would handle triple-quotes
but not add those "implicit paired delimiters"?

FWIW my impression (from some light Googling) is that setting this
option to nil (or the forward-sexp-function variable buffer-locally) is
something that a fair number of python-mode users have been doing for
years, without worrying about how that would impact docstring
navigation.


[1] Irrelevant motivation tucked under a footnote:

    Getting rid of the "implicit paired delimiters" suits me well, since
    it's easier for me to reason about:

    * I frequently overshoot my movement and need to hit e.g. a couple
      of back- commands after a string of forward- commands;
      python-mode's those invisible pair delimiters make
      {forward,backward}-sexp no longer cancel each other, which is
      disorienting;

    * if point is on a word or a paired delimiter, I expect *ward-sexp
      to move over that, instead of leaping over a whole block.

    E.g. when point is on the closing parenthesis of "bar_baz(quux,
    corge"): if I want to move to the start of "bar_baz",

    * with "traditional" sexps, it's just two C-M-b away,

    * with python-mode's default sexps, it's… I dunno; M-b C-M-u C-M-b?
      M-m M-f C-f?  It's frustrating because in any other major mode, I
      don't need to _stop and think_ about how to solve that puzzle;
      those invisible pairs force me to hold my fingers, squint and
      guess their position, then plan my route.

    I'm sure python-mode's default block movement is useful to some
    folks; IME I do fine with defun-movement (C-M-[ae]) or python-mode's
    other block commands (M-[ae]).  I get more value out of C-M-[bf] by
    keeping them limited to identifiers and pairs.
-------------------- End of forwarded message --------------------

Previous Next

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