spadarian / docblock-python

Atom plugin to insert documentation blocks for python functions
GNU General Public License v2.0
22 stars 9 forks source link

Generating docblocks from multiple cursors causes a stack overflow #14

Closed SKalt closed 6 years ago

SKalt commented 6 years ago

With a document

def foo(x): ...

def bar(y): ...

placing two cursors in both function definitions and calling generate_docblock causes a stack overflow within get_header's recursion. Generating docstrings for multiple things at once isn't what this package is designed for, but it would be nice to have.

spadarian commented 6 years ago

Hi @SKalt Thanks for all your suggestions and sorry for the delay. I was overseas with not much time to look into all this.

Let's see if I understand. If you have two functions, with two cursors (marked as |):

def f|oo(x):
    pass

def b|ar(y):
    pass 

and you run docblock-python you get a stack overflow.

I tried to replicate that but, in my case, it only generated the docblock for the latest cursor. I just finished the support for multiple cursors, but I would like to test if that solves this issue.

SKalt commented 6 years ago

Hey, no worries, this is open source! Unfortunately, upgrading 0.7.1 -> to 0.7.2 still raises

RangeError: Maximum call stack size exceeded
    at Object.get_header (/home/steven/.atom/packages/docblock-python/lib/docblock-python.js:23:18)

on the valid python 3 code I gave. However, I was unable to reproduce the error on the example you gave. Looks like it's the one-liner function definitions or the ellipsis that's breaking the recursion.

spadarian commented 6 years ago

Out of curiosity... What would your code do?

I think the problem is that the code looks for a : at the end of the line to get the header of the function.

SKalt commented 6 years ago

Pretty much what yours does, passes. The end-of-line thing will be a problem for more useful one-liners like def foo(): return 0. You might also encounter actually used one-liners and ellipsis is .pyi stub files.

On Sun, Jul 29, 2018, 11:10 AM José Padarian notifications@github.com wrote:

Out of curiosity... What would your code do?

I think the problem is that the code looks for a : at the end of the line to get the header of the function.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/spadarian/docblock-python/issues/14#issuecomment-408684185, or mute the thread https://github.com/notifications/unsubscribe-auth/AJ9G5UBy9C-DiTMKUEtVfzWKOEs2mHq2ks5uLdBPgaJpZM4VBcVi .

spadarian commented 6 years ago

And how would you document: def foo(x): ... or def bar(): return 0? I guess a single line description is ok.

I guess the main idea is that get_header doesn't fail.

SKalt commented 6 years ago

Ha, I guess no docstring is the correct behavior. Infinite recursion is not. Thanks for closing this!

On Sun, Jul 29, 2018, 8:03 PM José Padarian notifications@github.com wrote:

Closed #14 https://github.com/spadarian/docblock-python/issues/14 via 1fdd700 https://github.com/spadarian/docblock-python/commit/1fdd700390459ba3e75f4b656f9a556d2c525a0e .

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/spadarian/docblock-python/issues/14#event-1759086339, or mute the thread https://github.com/notifications/unsubscribe-auth/AJ9G5XkuV24TOUn7zV7P1zCQCxZfh-Uwks5uLk1qgaJpZM4VBcVi .