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

Multiline docstrings indented inconsistently on console #183

Closed
pekkaklarck opened this Issue Oct 1, 2014 · 7 comments

Comments

Projects
None yet
2 participants
@pekkaklarck

pekkaklarck commented Oct 1, 2014

If I have standard multiline docstring like

@task
def example():
    """Short doc.

    Some more text here.
    In two lines.
    """
    pass

and run invoke example, I get this output with inconsistent documentation indentation:

Usage: inv[oke] [--core-opts] example [other tasks here ...]

Docstring:
  Short doc.

      Some more text here.
      In two lines.

Options:
  none
@pekkaklarck

This comment has been minimized.

pekkaklarck commented Oct 1, 2014

If this is considered a bug, I can take a look at providing a pull request to fix it.

@bitprophet

This comment has been minimized.

Member

bitprophet commented Oct 1, 2014

I think this is because the expectation is that you'd have symmetric docstrings of the form:

@task
def example():
    """
    Docstring really starts here, not on the previous line.

    And so on and so forth. Now every line has the same left-margin,
    which is what the indent/dedent code expects.
    """
    pass

I checked and while I personally find the above style far more readable and aesthetic than the """it starts here style, apparently both are compatible with PEP-8/PEP-257:

The summary line may be on the same line as the opening quotes or on the next line.

It's probably not a lot of extra code to handle the other case and I'd accept a PR for it as long as it still also handles the form I showed above :) thanks!

@pekkaklarck

This comment has been minimized.

pekkaklarck commented Oct 2, 2014

I actually thought having the summary line on the same as the opening quotes was the recommended approach but apparently both styles are fine. I wonder is the code presented here available as a utility somewhere or should it just be copied:
http://legacy.python.org/dev/peps/pep-0257/#handling-docstring-indentation

@bitprophet

This comment has been minimized.

Member

bitprophet commented Oct 6, 2014

Yea, no idea on that last question. If there's a third party lib out there that handles it I'd be +1 to vendoring it, but it's not like that snippet you linked is very long either so we could simply drop it into a util function and tweak as (if) necessary.

@pekkaklarck

This comment has been minimized.

pekkaklarck commented Oct 7, 2014

Sorry, haven't had time to look at this due to being busy with getting Robot Framework 2.8.6 release ready. One of the tasks has been writing new packaging infrastructure that includes this:
https://github.com/robotframework/robotframework/blob/master/tasks.py


It seems that the standard inspect.cleandoc function does exactly what we need. It is used by inspect.getdoc so using that instead of accessing __doc__ directly ought to be enough. One gotcha with getdoc is that it returns None, not an empty string, if an object has no docstring. This is how I remember using it earlier:

doc = inspect.getdoc(obj) or ''

Once we got the RF 2.8.6 release out (hopefully today), I can take a look how to use getdoc in Invoke code base. If the fix is trivial and you know the place where it belongs, I obviously don't have anything against you just doing it yourself.

bitprophet added a commit that referenced this issue Oct 8, 2014

@bitprophet bitprophet closed this in 0fd505b Oct 8, 2014

@bitprophet

This comment has been minimized.

Member

bitprophet commented Oct 8, 2014

Using getdoc seems to work fine and makes the code a bit cleaner too. Thanks!

bitprophet added a commit that referenced this issue Oct 8, 2014

@pekkaklarck

This comment has been minimized.

pekkaklarck commented Oct 9, 2014

Thanks for fixing this! Wouldn't have taken me much longer to figure out the exact change location and to create a test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment