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

Allowing passing of ranges directly to formatting tools #188

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

bzvl
Copy link

@bzvl bzvl commented Sep 4, 2018

Neoformat supports formatting only part of a file, but it just passes
that chunk to formatting tools. A lot of tools need the context of the
surrounding lines, but support passing the requested range as an
argument.

This commit adds support for ranges by allowing certain variables to be
substituted in the arguments. In that case the 'range' option should be
set to '1' to indicate that the tool itself will handle the range.

@bzvl
Copy link
Author

bzvl commented Feb 1, 2019

Is there any interest in this feature? I've been using it locally and it's been really helpful for partially formatted files.

let stdin = getbufline(bufnr('%'), 1, '$')
else
let lines_after = getbufline(bufnr('%'), a:end_line + 1, '$')
let lines_before = getbufline(bufnr('%'), 1, a:start_line - 1)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that lines_after and lines_before

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like your comment was cutoff, what about them?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops. Yeah I meant to say that it seems that lines_after and lines_before aren't used

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, will fix.

@sbdchd
Copy link
Owner

sbdchd commented Feb 1, 2019

Yeah I think it looks pretty nice.
I had a minor comment.
Could you also update the README.md with the config info from the help file neoformat.txt?

@bzvl
Copy link
Author

bzvl commented Feb 2, 2019

Sure. Also, since using this, I've realized that just support_range isn't enough, because various tools work different ways. Some of them take in a range and output formatted code for just the lines that match the range. Other ones take in a range and output the entire file, with only that range formatted.

So I had some local changes to handle that. I'll rebase to the latest master and push a version with these changes. And yeah, I'll update the README.md, didn't notice that, thanks.

Neoformat supports formatting only part of a file, but it just passes
that chunk to formatting tools. A lot of tools need the context of the
surrounding lines, but support passing the requested range as an
argument.

This commit adds support for ranges by allowing certain variables to be
substituted in the arguments. In that case the 'range_mode' option should be
to indicate that the tool itself will handle the range.

Most tools use range_mode 1, which means that the tool takes the
entire buffer and a range as input, and outputs the entire buffer with
only that range formatted.

Some tools use range_mode 2, which means that the tool takes same input
as range_mode 1, but outputs only the range that was formatted.
@bzvl
Copy link
Author

bzvl commented Feb 2, 2019

A few changes here:

  1. Renamed from range to range_mode
  2. Added range_mode 2, which outputs only the range that was passed in. The only tool I know so far that uses this mode is hackfmt (which neoformat doesn't support, but I'll add that later)
  3. Reworked the docs a bit, and added them to README.md
  4. Actually committed the unit test. Whoops.

A handful of supported formatters support ranges. But it'll take a bit to add them and test them, so I'll do that separately. Also, I'm not sure I want to install EVERY formatter to test them :)

@bzvl
Copy link
Author

bzvl commented Feb 2, 2019

It looks like visual_selection_before.txt is failing, because that test uses yapf and prettier, the two tools I changed to use range mode. But that test verifies that you can select different file types within a file when you use ranges, which isn't true when the entire file is passed to the formatters.

I guess I'll try to use two other languages. But I'll worry about that tomorrow.

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