[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Emacs-diffs] /srv/bzr/emacs/emacs-24 r111281: * progmodes/python.el

From: Fabián Ezequiel Gallina
Subject: Re: [Emacs-diffs] /srv/bzr/emacs/emacs-24 r111281: * progmodes/python.el (python-info-current-defun): Fix failed
Date: Wed, 20 Feb 2013 17:54:59 -0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130109 Thunderbird/17.0.2

On 02/20/2013 11:01 AM, Stefan Monnier wrote:
FWIW this very small change corrected the python-info-current-defun behavior
and was able to pass the test suite I've been working on.
I don't doubt it works, I'm just pointing out that it would be better to
place the save-match-data elsewhere.  The general rule is: put the
save-match-data between the "match" and the corresponding "get
match-data" rather than around some code that might modify the match-data.
Thanks for the clarification, I pushed a change for this in revno 111284.
So far my local test/automated/python-tests.el is covering
indentation, movement and python-info-* functions completely.  Would it
be OK to introduce these tests into the emacs-24 branch?
Yes, it's OK to add files to emacs-24/test/...
Pushed in revno 111283.
In this regard, I have a change in the works that will cause python.el to
stop messing with match-data so much (use of looking-at-p instead of
looking-at and such), and document those functions that are intended to.
If everyone agrees I could get this to the emacs-24 branch.
I generally recommend not to go down that route.  Instead, just make sure
all the match-{beginning/end/string} calls come right after the
corresponding search, and if not, place a save-match-data around the
intermediate code.

That's good, I will not get picky about match-data modification. In any case these replacements of looking-at and string-match are part of a set of good and fairly cheap refactors (partially based on https://github.com/fgallina/python.el/pull/121).


reply via email to

[Prev in Thread] Current Thread [Next in Thread]