py-beginning-of-def-or-class looks inside strings

[Dhagglund]

[I mistakenly submitted this to the python project first as bug 830374.
Sorry...]

Using python-mode.el 4.6 and gnu emacs 21.1.

To demonstrate this problem:

1. Open the attached file with emacs.

2. Position point at the "#" on the last line.

3. Type `M-C-a', py-beginning-of-def-or-class.

Point will now be inside the docstring for class x.

py-b-o-d-o-c uses regexps to look backwards for def or class lines. When it
finds a candidate line, should it maybe use partial-parse-sexp to decide if
it's in a string or not?

[http://sourceforge.net/tracker/index.php?func=detail&aid=834290&group_id=86916&atid=581349]

[Dhagglund (dhagglund)]

Attach a patch fixing the problem.
The patch also fixes an incorrect call to py-beginning-o-d-o-c in
py-end-o-d-o-c.

[Andreas Roehler (a-roehler)]

Hi,

starting a bug-report for `py-beginning-of-def-or-class' yours showed up.
Think you are right, your solution is valid.

Ignoring yours, already wrote a different one - beside bugfixing with need of simplifying in mind.
After seeing yours, got some optimisation still --step-forms are yours-- thanks BTW.

Send it herewith for discussion:

as here already stated, we can't use

(re-search-... [...] count)

Result is valid only if not within a string,
`re-search-...' can't provide this

Therefor:

while count
when (re-search-... [...] single-step)
setq count...

Weeding out useless stuff:

For `py-beginning-of-def-or-class' we just need four things

- `count' together with `backward' or `forward'
- regexp as implemented:
  (start-re (cond ((eq class 'either) "^[ \t]*\\(class\\|def\\)\\>")
                        (class "^[ \t]*class\\>")
                        (t "^[ \t]*def\\>")))

- when backward, if inside or looking at regexp when start
- make sure, regexp found is not inside a string

No counting of columns needed at all, no
(at-or-before-p (<= (current-column) (current-indentation)))
etc. IMHO

Andreas

(defun py-beginning-of-def-or-class (&optional class count)
  "Move point to start of `def' or `class'.
...]'."
  (interactive "P") ; raw prefix arg
  (lexical-let* ((count (or count 1))
                 (step (if (< 0 count) -1 1))
                 (start-re (cond ((eq class 'either) "^[ \t]*\\(class\\|def\\)\\>")
                                 (class "^[ \t]*class\\>")
                                 (t "^[ \t]*def\\>"))))
    (while (/= 0 count)
      (if (< 0 count)
          (unless (looking-at start-re) (end-of-line))
        (end-of-line))
      (if
          (re-search-backward start-re nil 'move (- step))
          (unless
              ;; if inside a string
              (nth 3 (parse-partial-sexp (point-min) (point)))
            (goto-char (match-beginning 0))
            (setq count (+ count step)))
        (setq count 0)))))

[Andreas Roehler (a-roehler)]

Hi Barry,

attached a test-file, which does some move-checks on branch

https://code.launchpad.net/~a-roehler/python-mode/fixes-328781-bod-lands-in-string/+merge/18030

Also it checks last merge introducing hide-show support.

As tests went well, set branch for merge.

BTW wrote tests with perspective of further extensions,
every ...-test-forms addressing a patch:

(defun ar-py-mode-tests-intern ...
  (ar-py-mode-mv-test-forms)
  (ar-py-mode-hs-test-forms)
  )

Comments welcome.

So far

Andreas

--
https://code.launchpad.net/s-x-emacs-werkstatt/