Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support trailing commas? #20

Open
meliache opened this issue Feb 18, 2018 · 2 comments
Open

Support trailing commas? #20

meliache opened this issue Feb 18, 2018 · 2 comments

Comments

@meliache
Copy link

In some codebases I have seen that trailing commas are used in function definitions, especially when functions take a lot of keyword parameters and people frequently change or add them. Trailing commas then help with version control and guard against forgotten commas. You might argue whether this is good stile, but the following is valid python:

def foo(arg1, arg2, arg3,):
    return bar

sphinx-doc then fails, because sphinx-doc-fun-args creates an empy string "" as the last arg. I fixed that in place by just removing empty strings from the argument list with remove "". But maybe there is a better place to implement a fix. I didn't go really went into understanding your code, so I refrained from a PR.

(defun sphinx-doc-fun-args (argstrs)
  "Extract list of arg objects from string ARGSTRS.
ARGSTRS is the string representing function definition in Python.
Note that the arguments self, *args and **kwargs are ignored."
  (when (not (string= argstrs ""))
    (mapcar #'sphinx-doc-str->arg
            (-filter
             (lambda (str)
               (and (not (string= (substring str 0 1) "*"))
                    (not (string= str "self"))))
             (remove ""
              (mapcar #'s-trim
                      (split-string argstrs ",")))))))
@naiquevin
Copy link
Owner

It's a small change so I don't mind merging it. But I think only the last empty arg should be stripped and not all empty args. eg.

   def foo(a, b, , c): 
       pass

Is invalid python and sphinx-doc should not handle this case.

@kbeismann
Copy link

The fix in #22 covers both cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants