unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Revert recent buggy font-locking changes in python.el
@ 2021-03-03 21:28 Doug Davis
  2021-03-04 11:31 ` Lars Ingebrigtsen
  2021-03-04 23:56 ` Daniel Martín
  0 siblings, 2 replies; 6+ messages in thread
From: Doug Davis @ 2021-03-03 21:28 UTC (permalink / raw)
  To: emacs-devel

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

(Bug#45679) and (Bug#46233) report buggy font-locking in python.el. I
was able to trace it back to b2ce94fa5e - but I haven't been able to
figure out _what_ the bug is (it's easy to trigger the issue, i.e.
trigger the wonky font-locking), but not obvious how it's happening).

Would it be reasonable to revert those changes (the attached patch does
it)? I would also be happy to help solve the underlying issue (by
testing new patches or being pointed to any tips for debugging font
locking issues).


[-- Attachment #2: 0001-Revert-python-font-locking-logic-that-introduced-bug.patch --]
[-- Type: text/x-patch, Size: 6618 bytes --]

From 3915c45944daf63fed5275afd517ecdf186789bb Mon Sep 17 00:00:00 2001
From: Doug Davis <ddavis@ddavis.io>
Date: Wed, 3 Mar 2021 11:16:16 -0500
Subject: [PATCH] Revert python font-locking logic that introduced bugs.

(Bug#45679) and (Bug#46233) are both caused by the code this commit
reverts. Buggy code added in b2ce94fa5eecee0afd0e6237956cfb2b02b8bb0b

* lisp/progmodes/python.el (python-rx): remove the additional
assignment targets.
(python-font-lock-assignment-matcher): remove new function that is
only used in...
(python-font-lock-keyword-maximum-decoration): remove new assignment
font-locking logic that leads to the bugs quoted above. Bring back
logic from before the commit quoted above.
---
 lisp/progmodes/python.el | 97 +++++++++++-----------------------------
 1 file changed, 27 insertions(+), 70 deletions(-)

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index afb96974b1..c3d77f15a8 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -4,7 +4,7 @@
 
 ;; Author: Fabián E. Gallina <fgallina@gnu.org>
 ;; URL: https://github.com/fgallina/python.el
-;; Version: 0.27.1
+;; Version: 0.27.2
 ;; Package-Requires: ((emacs "24.2") (cl-lib "1.0"))
 ;; Maintainer: emacs-devel@gnu.org
 ;; Created: Jul 2010
@@ -394,12 +394,6 @@ python-rx
                                     (any ?' ?\") "__main__" (any ?' ?\")
                                     (* space) ?:))
             (symbol-name       (seq (any letter ?_) (* (any word ?_))))
-            (assignment-target (seq (? ?*)
-                                    (* symbol-name ?.) symbol-name
-                                    (? ?\[ (+ (not ?\])) ?\])))
-            (grouped-assignment-target (seq (? ?*)
-                                            (* symbol-name ?.) (group symbol-name)
-                                            (? ?\[ (+ (not ?\])) ?\])))
             (open-paren        (or "{" "[" "("))
             (close-paren       (or "}" "]" ")"))
             (simple-operator   (any ?+ ?- ?/ ?& ?^ ?~ ?| ?* ?< ?> ?= ?%))
@@ -611,18 +605,6 @@ python-font-lock-keywords-level-2
 `python-font-lock-keywords-level-1', as well as keywords and
 builtins.")
 
-(defun python-font-lock-assignment-matcher (regexp)
-  "Font lock matcher for assignments based on REGEXP.
-Return nil if REGEXP matched within a `paren' context (to avoid,
-e.g., default values for arguments or passing arguments by name
-being treated as assignments) or is followed by an '=' sign (to
-avoid '==' being treated as an assignment."
-  (lambda (limit)
-    (let ((res (re-search-forward regexp limit t)))
-      (unless (or (python-syntax-context 'paren)
-                  (equal (char-after (point)) ?=))
-        res))))
-
 (defvar python-font-lock-keywords-maximum-decoration
   `((python--font-lock-f-strings)
     ,@python-font-lock-keywords-level-2
@@ -670,57 +652,32 @@ python-font-lock-keywords-maximum-decoration
            )
           symbol-end)
      . font-lock-type-face)
-    ;; multiple assignment
-    ;; (note that type hints are not allowed for multiple assignments)
-    ;;   a, b, c = 1, 2, 3
-    ;;   a, *b, c = 1, 2, 3, 4, 5
-    ;;   [a, b] = (1, 2)
-    ;;   (l[1], l[2]) = (10, 11)
-    ;;   (a, b, c, *d) = *x, y = 5, 6, 7, 8, 9
-    ;;   (a,) = 'foo'
-    ;;   (*a,) = ['foo', 'bar', 'baz']
-    ;;   d.x, d.y[0], *d.z = 'a', 'b', 'c', 'd', 'e'
-    ;; and variants thereof
-    ;; the cases
-    ;;   (a) = 5
-    ;;   [a] = 5
-    ;;   [*a] = 5, 6
-    ;; are handled separately below
-    (,(python-font-lock-assignment-matcher
-        (python-rx (? (or "[" "(") (* space))
-                   grouped-assignment-target (* space) ?, (* space)
-                   (* assignment-target (* space) ?, (* space))
-                   (? assignment-target (* space))
-                   (? ?, (* space))
-                   (? (or ")" "]") (* space))
-                   (group assignment-operator)))
-     (1 font-lock-variable-name-face)
-     (,(python-rx grouped-assignment-target)
-      (progn
-        (goto-char (match-end 1))       ; go back after the first symbol
-        (match-beginning 2))            ; limit the search until the assignment
-      nil
-      (1 font-lock-variable-name-face)))
-    ;; single assignment with type hints, e.g.
-    ;;   a: int = 5
-    ;;   b: Tuple[Optional[int], Union[Sequence[str], str]] = (None, 'foo')
-    ;;   c: Collection = {1, 2, 3}
-    ;;   d: Mapping[int, str] = {1: 'bar', 2: 'baz'}
-    (,(python-font-lock-assignment-matcher
-        (python-rx grouped-assignment-target (* space)
-                   (? ?: (* space) (+ not-simple-operator) (* space))
-                   assignment-operator))
-     (1 font-lock-variable-name-face))
-    ;; special cases
-    ;;   (a) = 5
-    ;;   [a] = 5
-    ;;   [*a] = 5, 6
-    (,(python-font-lock-assignment-matcher
-       (python-rx (or "[" "(") (* space)
-                  grouped-assignment-target (* space)
-                  (or ")" "]") (* space)
-                  assignment-operator))
-     (1 font-lock-variable-name-face)))
+    ;; assignments
+    ;; support for a = b = c = 5
+    (,(lambda (limit)
+        (let ((re (python-rx (group (+ (any word ?. ?_)))
+                             (? ?\[ (+ (not (any  ?\]))) ?\]) (* space)
+                             ;; A type, like " : int ".
+                             (? ?: (* space) (+ (any word ?. ?_)) (* space))
+                             assignment-operator))
+              (res nil))
+          (while (and (setq res (re-search-forward re limit t))
+                      (or (python-syntax-context 'paren)
+                          (equal (char-after (point)) ?=))))
+          res))
+     (1 font-lock-variable-name-face nil nil))
+    ;; support for a, b, c = (1, 2, 3)
+    (,(lambda (limit)
+        (let ((re (python-rx (group (+ (any word ?. ?_))) (* space)
+                             (* ?, (* space) (+ (any word ?. ?_)) (* space))
+                             ?, (* space) (+ (any word ?. ?_)) (* space)
+                             assignment-operator))
+              (res nil))
+          (while (and (setq res (re-search-forward re limit t))
+                      (goto-char (match-end 1))
+                      (python-syntax-context 'paren)))
+          res))
+     (1 font-lock-variable-name-face nil nil)))
   "Font lock keywords to use in python-mode for maximum decoration.
 
 This decoration level includes everything in
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: Revert recent buggy font-locking changes in python.el
  2021-03-03 21:28 Revert recent buggy font-locking changes in python.el Doug Davis
@ 2021-03-04 11:31 ` Lars Ingebrigtsen
  2021-03-04 14:12   ` Eli Zaretskii
  2021-03-04 15:26   ` Doug Davis
  2021-03-04 23:56 ` Daniel Martín
  1 sibling, 2 replies; 6+ messages in thread
From: Lars Ingebrigtsen @ 2021-03-04 11:31 UTC (permalink / raw)
  To: Doug Davis; +Cc: emacs-devel

Doug Davis <ddavis@ddavis.io> writes:

> (Bug#45679) and (Bug#46233) report buggy font-locking in python.el. I
> was able to trace it back to b2ce94fa5e - but I haven't been able to
> figure out _what_ the bug is (it's easy to trigger the issue, i.e.
> trigger the wonky font-locking), but not obvious how it's happening).
>
> Would it be reasonable to revert those changes (the attached patch does
> it)? I would also be happy to help solve the underlying issue (by
> testing new patches or being pointed to any tips for debugging font
> locking issues).

The patch seemed to fix the issue in bug#45341, so it'd be better to try
to determine what it doesn't get right instead of reverting outright.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Revert recent buggy font-locking changes in python.el
  2021-03-04 11:31 ` Lars Ingebrigtsen
@ 2021-03-04 14:12   ` Eli Zaretskii
  2021-03-04 15:26   ` Doug Davis
  1 sibling, 0 replies; 6+ messages in thread
From: Eli Zaretskii @ 2021-03-04 14:12 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: ddavis, emacs-devel

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Thu, 04 Mar 2021 12:31:54 +0100
> Cc: emacs-devel@gnu.org
> 
> Doug Davis <ddavis@ddavis.io> writes:
> 
> > (Bug#45679) and (Bug#46233) report buggy font-locking in python.el. I
> > was able to trace it back to b2ce94fa5e - but I haven't been able to
> > figure out _what_ the bug is (it's easy to trigger the issue, i.e.
> > trigger the wonky font-locking), but not obvious how it's happening).
> >
> > Would it be reasonable to revert those changes (the attached patch does
> > it)? I would also be happy to help solve the underlying issue (by
> > testing new patches or being pointed to any tips for debugging font
> > locking issues).
> 
> The patch seemed to fix the issue in bug#45341, so it'd be better to try
> to determine what it doesn't get right instead of reverting outright.

I agree.



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Revert recent buggy font-locking changes in python.el
  2021-03-04 11:31 ` Lars Ingebrigtsen
  2021-03-04 14:12   ` Eli Zaretskii
@ 2021-03-04 15:26   ` Doug Davis
  1 sibling, 0 replies; 6+ messages in thread
From: Doug Davis @ 2021-03-04 15:26 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: emacs-devel

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Doug Davis <ddavis@ddavis.io> writes:
>
>> (Bug#45679) and (Bug#46233) report buggy font-locking in python.el. I
>> was able to trace it back to b2ce94fa5e - but I haven't been able to
>> figure out _what_ the bug is (it's easy to trigger the issue, i.e.
>> trigger the wonky font-locking), but not obvious how it's happening).
>>
>> Would it be reasonable to revert those changes (the attached patch does
>> it)? I would also be happy to help solve the underlying issue (by
>> testing new patches or being pointed to any tips for debugging font
>> locking issues).
>
> The patch seemed to fix the issue in bug#45341, so it'd be better to try
> to determine what it doesn't get right instead of reverting outright.

Thanks, understood- I'll keep poking at it (and repeat that I'm happy to
volunteer to test any patches if anyone else tries to work on it and
believes they have a fix).



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Revert recent buggy font-locking changes in python.el
  2021-03-03 21:28 Revert recent buggy font-locking changes in python.el Doug Davis
  2021-03-04 11:31 ` Lars Ingebrigtsen
@ 2021-03-04 23:56 ` Daniel Martín
  2021-03-05  1:22   ` Doug Davis
  1 sibling, 1 reply; 6+ messages in thread
From: Daniel Martín @ 2021-03-04 23:56 UTC (permalink / raw)
  To: Doug Davis; +Cc: emacs-devel

Doug Davis <ddavis@ddavis.io> writes:

> Would it be reasonable to revert those changes (the attached patch does
> it)? I would also be happy to help solve the underlying issue (by
> testing new patches or being pointed to any tips for debugging font
> locking issues).

There's a potentially useful Emacs package to debug font lock issues
more easily: https://github.com/Lindydancer/font-lock-studio

Maybe it helps shed some light on these particular issues with
python.el.



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Revert recent buggy font-locking changes in python.el
  2021-03-04 23:56 ` Daniel Martín
@ 2021-03-05  1:22   ` Doug Davis
  0 siblings, 0 replies; 6+ messages in thread
From: Doug Davis @ 2021-03-05  1:22 UTC (permalink / raw)
  To: Daniel Martín; +Cc: emacs-devel

Daniel Martín <mardani29@yahoo.es> writes:

> Doug Davis <ddavis@ddavis.io> writes:
>
>> Would it be reasonable to revert those changes (the attached patch does
>> it)? I would also be happy to help solve the underlying issue (by
>> testing new patches or being pointed to any tips for debugging font
>> locking issues).
>
> There's a potentially useful Emacs package to debug font lock issues
> more easily: https://github.com/Lindydancer/font-lock-studio
>
> Maybe it helps shed some light on these particular issues with
> python.el.

Thanks! I'll take a look.



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-03-05  1:22 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-03 21:28 Revert recent buggy font-locking changes in python.el Doug Davis
2021-03-04 11:31 ` Lars Ingebrigtsen
2021-03-04 14:12   ` Eli Zaretskii
2021-03-04 15:26   ` Doug Davis
2021-03-04 23:56 ` Daniel Martín
2021-03-05  1:22   ` Doug Davis

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).