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

Possibly bug in calculating diffs #396

Open
attila0x2A opened this issue May 4, 2016 · 9 comments
Open

Possibly bug in calculating diffs #396

attila0x2A opened this issue May 4, 2016 · 9 comments

Comments

@attila0x2A
Copy link
Contributor

When running coala with following .coafile:

[python]
bears = PEP8Bear
files = py.py

the py.py content:

def helloWorld():




    return(print("hello World"))

 #s

helloWorld()

coala proposes two diffs instead of one, which doesn't feel right. Here are the diff. First:

|   1|   1| def helloWorld():
|   2|   2| 
|    |   3|+    return(print("hello World"))
|   3|   4| 
|   4|   5| 
|   5|   6| 

Second:

|   1|   1| def helloWorld():
|   2|   2| 
|   3|   3| 
|   4|    |-
|   5|    |-
|   6|    |-    return(print("hello World"))
|   7|    |-
|   8|    |- #s
|    |   4|+ # s
|   9|   5| 
|  10|   6| helloWorld()

Somehow I think this should be only one diff.
The problem may be with PEP8Bear, so that's why I'm opening it here.

@gitmate-bot
Copy link
Collaborator

Thanks for reporting this issue!

Your aid is required, fellow coalaian. Help us triage and solving this issue!

CC @sils1297, @AbdealiJK

@Makman2
Copy link
Member

Makman2 commented May 4, 2016

We use Diff.split_diff() to part up a single big diff. Indeed your example demonstrates that sometimes this is weak^^
Maybe it's possible to group issues that belong together inside a diff with PEP8?

@attila0x2A
Copy link
Contributor Author

It may be a problem not only with PEP8. By the way, @Makman2 can you reproduce this. There is still a chance that I have outdated coala version or something.

@sils
Copy link
Member

sils commented May 4, 2016

No this is definitely the case, we sadly can't do more than either split the diff or not split it at all. Well,we could allow say 2ish lines unchanged for the hunks to remain in the same diff?

This definitely applies to other bears too

@sils
Copy link
Member

sils commented May 15, 2016

I have introduced a distance parameter in coala that allows seperating diffs only if they're further away than n lines, we should make use of this in various bears.

@sils sils self-assigned this May 15, 2016
@sils sils removed this from the 0.3 milestone Sep 4, 2016
@sils sils removed their assignment Sep 4, 2016
@jack17529
Copy link

Can I assign this to me as I have not done anything in coala-bear

@Makman2
Copy link
Member

Makman2 commented Mar 23, 2017

Cause of coala/coala#2898.

@Makman2
Copy link
Member

Makman2 commented Mar 23, 2017

@jack17529 feel free to break this issue up into several smaller issues for each bear you find you can improve 👍

@Naveenaidu
Copy link
Member

@Makman2 Can i please be assigned to this issue.

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

No branches or pull requests

6 participants