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

Allow docstrings to happen anywhere in the code #18

Merged
merged 1 commit into from
Mar 9, 2014

Conversation

YorikSar
Copy link
Contributor

@YorikSar YorikSar commented Mar 9, 2014

In Python docstring can happen just in the middle of the code, e.g. documentation for class or instance variables. So let's fold them.
This also removes assumption that any string ending with '):' is an end for def: or class: (it can by if: or anything else).

In Python docstring can happen just in the middle of the code, e.g.
documentation for class or instance variables. So let's fold them.
This also removes assumption that any string ending with '):' is an end
for def: or class: (it can by if: or anything else).
@tmhedberg
Copy link
Owner

The removal of the ): assumption that you mention reintroduces the bug that was fixed in #17. I can't apply this due to that regression.

Additionally, the use of docstrings on instance variables is, in my admittedly subjective experience, very rare in Python. PEP 257 states that, while they are allowed, they do not get any of the special functionality associated with class and method docstrings (e.g. runtime accessibility via the __doc__) property. So I'm not sure what advantage a docstring has, if any, over an ordinary comment in this case.

In general, I am not particularly interested in supporting all of the nuances of technically-legal Python syntax; I only care that the common usage works correctly with this plugin. I would rather not introduce support for rarely-used syntax if that comes at the cost of heightened complexity.

If you can accomplish this in a different way that does not cause regressions or significantly increase complexity, I'll consider merging it.

@YorikSar
Copy link
Contributor Author

YorikSar commented Mar 9, 2014

It didn't reintroduce that bug. Strings that end with '):' are strings that don't end with ''.

In fact I wanted to do this change not to allow rare docstrings to fold but to get rid of that '):' because it seemed... non-deterministic?

tmhedberg added a commit that referenced this pull request Mar 9, 2014
Allow docstrings to happen anywhere in the code
@tmhedberg tmhedberg merged commit d5d1f25 into tmhedberg:master Mar 9, 2014
@tmhedberg
Copy link
Owner

You're right. I both misread your patch and wrote a flawed test case to verify it. Sorry about that!

In that case, I think this looks good. Thanks!

@YorikSar YorikSar deleted the docstring_everywhere branch March 9, 2014 20:54
@YorikSar
Copy link
Contributor Author

YorikSar commented Mar 9, 2014

What do you think about adding automated testing here? I'd be happy to play with vim-vspec and see how it goes :)

@tmhedberg
Copy link
Owner

Testing would be a good thing to have, for sure. If you want to try that out, that would be great.

tmhedberg added a commit that referenced this pull request Mar 14, 2014
It has been pointed out that this reintroduces the bug that was fixed in

This reverts commit d5d1f25, reversing
changes made to 922b387.
@tmhedberg
Copy link
Owner

@quasarj pointed out that this change reintroduced the bug from #14. I've reverted the change for now. If you can fix the bug, I'll reapply your patch.

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

Successfully merging this pull request may close these issues.

2 participants