glyph / python-docstring-mode

Emacs minor-mode for editing Python docstrings.
MIT License
70 stars 22 forks source link

Elpy breaks python-docstring-mode #33

Closed tels7ar closed 1 year ago

tels7ar commented 2 years ago

I've observed that python-docstring-mode is no longer working after upgrading to emacs 28.1. Pressing in a docstring does not indent the line. Also, python-docstring-fill produced incorrect results. Consider the following:

def myfunc():
    """
    this is a docstring

    :param someparam: foo
    :returns: blah
    """

Running python-docstring-fill on this produces:

def myfunc():
   8

    this is a docstring

    :param someparam: foo
    :returns: blah
    """

I am unclear what the number represents, it seems perhaps to be related to the number of lines in the docstring.

This is with the latest version of python-docstring-mode as of May 2022.

jamadden commented 2 years ago

I've been having this issue too. I finally decided to look into it a little bit. caution: I am not an elisp or emacs expert.

From debugging through the function, it looks like the problem is in how string-end is calculated.

                       (string-end
                        (- (condition-case ()        ; for unbalanced quotes
                               (progn (forward-sexp)
                                      (point))
                             (error (point-max)))
                           3))

In one example of a triple quoted docstring, I get a string-start of 12866 and a string-end of 12862, that is, the end is before the start. When the preceding call to goto-char put the point on the first " of the opening triple quotes, the forward-sexp here just moved the point to the last of the opening three quotes.

Looking at the emacs change notes, I found a mention of the new user customization variable python-forward-sexp-function. That gets copied into the buffer-local forward-sexp-function when python-mode initializes. For some reason, the value I had for forward-sexp-function was nil, meaning to use the "CC-mode like" sexp navigation. Changing that with (setq-local forward-sexp-function #'python-nav-forward-sexp) fixed the problem for me.

jamadden commented 2 years ago

That "some reason" making forward-sexp-function nil seems to be part of elpy-mode:

(defun elpy-module-sane-defaults (command &rest _args)
  "Module for sane Emacs default for python."
  (pcase command
    (`buffer-init
     ;; Set `forward-sexp-function' to nil in python-mode. See
     ;; http://debbugs.gnu.org/db/13/13642.html
     (set (make-local-variable 'forward-sexp-function) nil)
     ;; PEP8 recommends two spaces in front of inline comments.
     (set (make-local-variable 'comment-inline-offset) 2))
    (`buffer-stop
     (kill-local-variable 'forward-sexp-function)
     (kill-local-variable 'comment-inline-offset))))

Because this is invoked through elpy's module mechanism, just putting the correct value back in my mode hook wasn't enough. The easiest thing to do was to take that function out of the default module list, though I suspect you could also put "after" advice on it.

;; this breaks navigation and python-docstring-mode in emacs 28
;; https://github.com/glyph/python-docstring-mode/issues/33
(setq elpy-modules
      (remove 'elpy-module-sane-defaults elpy-modules))
tels7ar commented 2 years ago

@jamadden your solution worked for me. Thank you!

blarghmatey commented 2 years ago

That worked for me as well, thank you for doing that digging!

glyph commented 1 year ago

Does anyone want to write a PR to address this either here or in elpy-mode so we can close this issue? :)

glyph commented 1 year ago

I've significantly refactored the code, and added tree-sitter support which removes the dependency on sexp parsing and motions, so that might also work as a workaround for anyone on emacs29.

glyph commented 1 year ago

OK hopefully eliminating all reliance on forward-sexp will address any incompatibility.