|
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).
Thanks, Fabián
[Prev in Thread] | Current Thread | [Next in Thread] |