[-- Attachment #1: Type: text/plain, Size: 1178 bytes --] 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. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: ert-python-forward-sexp.patch --] [-- Type: text/x-patch, Size: 630 bytes --] diff --git a/test/lisp/progmodes/python-tests.el b/test/lisp/progmodes/python-tests.el index e17bc0df92..7243eb0e59 100644 --- a/test/lisp/progmodes/python-tests.el +++ b/test/lisp/progmodes/python-tests.el @@ -2763,6 +2763,13 @@ python-nav-forward-sexp-safe-1 (python-nav-forward-sexp-safe 1) (should (looking-at "$")))) +(ert-deftest python-nav-forward-sexp-nil () + (let ((python-forward-sexp-function nil)) + (python-tests-with-temp-buffer + "'''A docstring.'''" + (forward-sexp nil t) + (should (= (point) 19))))) + (ert-deftest python-nav-up-list-1 () (python-tests-with-temp-buffer " [-- Attachment #3: Type: text/plain, Size: 828 bytes --] 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
[-- Attachment #1: Type: text/plain, Size: 1595 bytes --] On Wed, Jun 22, 2022 at 10:48 PM Kévin Le Gouguec <kevin.legouguec@gmail.com> wrote: > Kévin Le Gouguec <kevin.legouguec@gmail.com> writes: > > > João Távora <joaotavora@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 [-- Attachment #2: Type: text/html, Size: 2378 bytes --]
João Távora <joaotavora@gmail.com> writes: > On Wed, Jun 22, 2022 at 10:48 PM Kévin Le Gouguec <kevin.legouguec@gmail.com> wrote: > > Kévin Le Gouguec <kevin.legouguec@gmail.com> writes: > > > João Távora <joaotavora@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 😕
(Off-list message that João already replied to in
<CALDnm53bj4rWzkP50L7TdwF3CRpy4rST3oy23bujWgC9YCJnAA@mail.gmail.com>;
re-sent for archival)
-------------------- Start of forwarded message --------------------
From: Kévin Le Gouguec <kevin.legouguec@gmail.com>
To: João Távora <joaotavora@gmail.com>
Cc: Stefan Monnier <monnier@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@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 --------------------