search

PyCQA / redbaron

Bottom-up approach to refactoring in python
http://redbaron.pycqa.org/
694 stars 74 forks source link

Issue #117: Fix bug with finding comment node #118

Closed gtors closed 7 years ago

gtors commented 8 years ago

Added find_iter method, which is became base for find and find_all methods.

b5y commented 8 years ago

You should start from baron. Take a glance for #95 issue.

gtors commented 8 years ago

Before my changes:

r = RedBaron("""
class X:
    # test
    def __init__(self): 
        pass
""")
r.find('comment') #  None <---------- Wrong! Expected CommentNode here

After my changes:

r = RedBaron("""
class X:
    # test
    def __init__(self): 
        pass
""")
r.find('comment') # '#test' <---------- OK! As expected, CommentNode is found.
b5y commented 8 years ago

You should add test cases. Without them it is difficult to say something about code. I advise you to check another nodes with find and find_all methods.

gtors commented 8 years ago

But I've added a test cases: https://github.com/PyCQA/redbaron/pull/118/files#diff-38f83afa1ef19109e4de1a26496bc4c9L922

b5y commented 8 years ago

@gtors Okay. In first commit I did not see it.

bootandy commented 7 years ago

Is this getting merged in ? I just encounted a similar comment parsing bug. @b5y

gtors commented 7 years ago

But can you link to the exact line(s) in the commit that fixes it? My eye can't find

AFAICR, find and find_all have different behaviors: L671 L766 The fix was in eliminating these differences.

Also, could you add two lines to the CHANGELOG explaining the new find_iter function and the fix as well as adding the find_iter function in the documentation?

Ok, but as soon as I have some free time.

ibizaman commented 7 years ago

The fix was in eliminating these differences.

Nice catch!

Ok, but as soon as I have some free time.

@gtors no worries, I'll add the changes myself

Psycojoker commented 7 years ago

Fix released https://pypi.python.org/pypi/redbaron/0.6.3