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

Range of lines for fixers #850

Open
IngoMeyer441 opened this issue Aug 14, 2017 · 13 comments
Open

Range of lines for fixers #850

IngoMeyer441 opened this issue Aug 14, 2017 · 13 comments

Comments

@IngoMeyer441
Copy link
Contributor

IngoMeyer441 commented Aug 14, 2017

Hey, thanks for this great linter plugin. I switched from syntastic recently and it's amazing how fast and responsive real time linting works. :)

In this commit I have added clang-tidy as a possible fixer for C and C++ code. However, often I only want to fix one specific line in a huge file (for example a missing semicolon or a missing override). Would it be possible to modify ALEFix to accept a specific line or a range of lines? To stay compatible with existing code fixers could get an optional third parameter with a range of lines they can operate on.

@w0rp
Copy link
Member

w0rp commented Aug 14, 2017

I wonder if this use case is worth supporting. Not all tools support fixing ranges of lines. You often have to look at an entire file to know if you can fix something. A program for fixing errors should only really make changes where there are problems. Otherwise, why change anything?

@bb010g
Copy link
Contributor

bb010g commented Aug 22, 2017

google/google-java-format supports a similar argument.

@wolph
Copy link

wolph commented Oct 4, 2017

While I agree that it won't work in all cases, I can think of many where it would provide clear benefit to simply refactor a single line/method/class.

The most important argument being... I don't always want to refactor an entire file which causes a huge diff (and acompanying merge conflicts).

@w0rp
Copy link
Member

w0rp commented Oct 5, 2017

I recommend writing smaller files.

@wolph
Copy link

wolph commented Oct 5, 2017

I generally don't need the fixers for code I wrote ;) And I can't control what my customers have running in all cases

@sherwinyu
Copy link

sherwinyu commented Dec 13, 2017

I agree that this would be a useful feature. I was excited about using :ALEFix and a python fixer (autopep8 / yapf) on my codebase at work, but ended up disabling the fix-on-write behavior because it introduce a lot of unrelated style fixes, since it's changing the entire file, not just the lines I touched. Frequently I want to limit it to just the changes I've made.

Supporting this would be incredible! (And thanks for the already incredible plugin)

My current workaround is something like this: :'<,'>!autopep8 - to format the selection

@w0rp
Copy link
Member

w0rp commented Dec 13, 2017

Someone needs to figure out how to fit this into the existing API, including how fixer functions should be defined. Considerations need to be made when combining tools which do support fixing ranges with those that do not.

@wolph
Copy link

wolph commented Dec 13, 2017

Agreed, especially with languages such as python which are largely relying on whitespace this can be dangerous

@lithammer
Copy link
Contributor

Some tools like clang-format and yapf (Python) takes a "range" argument, but actually require the whole file for context.

My current workaround is something like this: :'<,'>!autopep8 - to format the selection

I'm curious, is this even working as you expect given what I wrote in my first paragraph? As far as I know you need to pass the --line-range/--range to autopep8:

  --line-range line line, --range line line
                        only fix errors found within this inclusive range of
                        line numbers (e.g. 1 99); line numbers are indexed at
                        1

@wolph
Copy link

wolph commented Dec 13, 2017

That works, assuming you select an entire function/class or other top-level code. Which already helps a lot imho :)

It beats having to format everything yourself so I use similar tricks

@w0rp
Copy link
Member

w0rp commented Dec 17, 2017

I've thought about how to support this a bit, and here is my plan.

  1. Add commands for fixing ranges of lines which work with selection modes.
  2. When a range command is used, send [buffer_number, [lnum, col], [end_lnum, end_col]] to the functions as the first fixer function argument instead of just buffer_number.
  3. Expect functions to return a Dictionary containing the key fix_range set to 1 for fixing a range. For applying a list immediately, look for a new new_lines key with a List of lines. Ignore all other output.
  4. Consider catching E745 only when fixing ranges of lines, so problems with handling the List for other functions can be ignored.

With the above it would be possible to fix ranges of lines for the functions that support it, and ignore the output of others. You should expect weird results when mixing fixers which do and do not support fixing ranges.

@leikoilja
Copy link

@w0rp, is there any update for us? It would be so so cool to be able to send a range of lines for a python linter.
Thanks

@plasmon360
Copy link

I agree that this would be a useful feature. I was excited about using :ALEFix and a python fixer (autopep8 / yapf) on my codebase at work, but ended up disabling the fix-on-write behavior because it introduce a lot of unrelated style fixes, since it's changing the entire file, not just the lines I touched. Frequently I want to limit it to just the changes I've made.

Supporting this would be incredible! (And thanks for the already incredible plugin)

My current workaround is something like this: :'<,'>!autopep8 - to format the selection

Thanks. I am using :'<,'>!black-macchiato . black-macchiato works on top of black

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

No branches or pull requests

8 participants