stevearc / conform.nvim

Lightweight yet powerful formatter plugin for Neovim
MIT License
2.99k stars 154 forks source link

feature request: different range formatting strategy #256

Open ftelnov opened 9 months ago

ftelnov commented 9 months ago

Did you check existing requests?

Describe the feature

Provide different range formatting strategy(or mode): it would firstly extract the selected lines, then pass them as-is to the formatter, and put them back.

Provide background

I am Rust developer, and rustfmt doesn't allow range formatting(well, it does allow, but not through the stdin range selection), but your range formatting works pretty good with it, brilliant work! However, rustfmt is not ideal itself - it can't format code inside macroses(well, for various reasons). So it won't ever format code inside macros itself. But if I manually supply him lines inside macro, it does job very well.

So my proposed new workflow is as follows: 1) Get line selection; 1) Paste lines as is to the formatter; 1) Get lines back into their place.

What is the significance of this feature?

nice to have

Additional details

I would be grateful as well if you simply provide custom code snippet/function, if it is already possible.

stevearc commented 9 months ago

For your use case, you may already be able to use the injected formatter for this. You'll need to set up a treesitter injection for the macro regions (this may be built-in, I'm not sure) and then configure it so that the injected formatter uses rustfmt for whatever language is injected there (probably also rust).

ftelnov commented 9 months ago

@stevearc It seems that it is not really possible ATM, because of condition here: https://github.com/stevearc/conform.nvim/blob/ad2b5ecd907128ed1b66b1cf4bed57902ef836ee/lua/conform/formatters/injected.lua#L159

If injected formatter encounters AST children with the exact language, it simply skips it(and, well, it's kinda logical). And it prevents my desired usage. Any idea how can I tweak it?

ftelnov commented 9 months ago

Even if I remove the line I stated above, it is still problematic, because Rust's injections are bad. Anyway, would you be interested in this proposed feature(different range format strategy)? If so, I would be glad to implement it myself.

stevearc commented 8 months ago

I still think that the injected formatter sounds like it's a closer match for your use case than range formatting. The reason I didn't go with the kind of range formatting you're describing initially is it requires the user to highlight syntactically complete sections of code, since most formatters can't operate on fragments, and thus doesn't work in a lot of situations. I'm not opposed to having more than one way to do range formatting, but only if there's a real need and solid benefit from it. I'm not yet convinced that this qualifies.

When you say "Rust's injections are bad," what is actually bad about them? What's the actual cause of the problem?

ftelnov commented 8 months ago

I think you a right and it might be solved with the injected formatter and we should go first for that solution, because my proposed one seems like a hack, not like an established algo. Maybe I need a help with understanding the injected formatter code, so I can contribute and maybe fix rust injections as well.

Can you please answer the following questions, or provide a link to a community where I can share the questions with the people: 1) According to the treesitter [docs](https://neovim.io/doc/user/treesitter.html#LanguageTree%3Aparse()), if we pass false at this place, it won't run injections. Maybe I'm wrong, but I have empty included_regions() output, and it solves if I pass true instead. 2) I want to inspect the tree to see whether Rust injections are correct. However, somewhy :InspectTree command results in error for the simple Rust macro. Maybe it is because :InspectTree is broken, but I doubt it. Please, take a look at attached screenshot:

image

Error on command run:

Error detected while processing FileType Autocommands for "*"..function <SNR>1_LoadFTPlugin[20]..script /opt/homebrew/Cellar/neovim/HEAD-17f3a3a
/share/nvim/runtime/ftplugin/query.lua:
E5113: Error while calling lua chunk: ...f3a3a/share/nvim/runtime/lua/vim/treesitter/language.lua:98: no parser for 'query' language, see :help
treesitter-parsers
stack traceback:
        [C]: in function 'error'
        ...f3a3a/share/nvim/runtime/lua/vim/treesitter/language.lua:98: in function 'add'
        ...a/share/nvim/runtime/lua/vim/treesitter/languagetree.lua:113: in function 'new'
        ...m/HEAD-17f3a3a/share/nvim/runtime/lua/vim/treesitter.lua:64: in function '_create_parser'
        ...m/HEAD-17f3a3a/share/nvim/runtime/lua/vim/treesitter.lua:131: in function 'get_parser'
        ...m/HEAD-17f3a3a/share/nvim/runtime/lua/vim/treesitter.lua:467: in function 'start'
        ...eovim/HEAD-17f3a3a/share/nvim/runtime/ftplugin/query.lua:12: in main chunk
        [C]: in function '__newindex'
        ...AD-17f3a3a/share/nvim/runtime/lua/vim/treesitter/dev.lua:163: in function 'set_dev_properties'
        ...AD-17f3a3a/share/nvim/runtime/lua/vim/treesitter/dev.lua:340: in function 'inspect_tree'
        ...m/HEAD-17f3a3a/share/nvim/runtime/lua/vim/treesitter.lua:504: in function 'inspect_tree'
        ...r/neovim/HEAD-17f3a3a/share/nvim/runtime/plugin/nvim.lua:18: in function <...r/neovim/HEAD-17f3a3a/share/nvim/runtime/plugin/nvim.lua
:9>

3) Even when I pass true as an argument, I still got a problematic included_regions. Consider code given above and pasted bellow:

thread_local! {
    static A: usize =                5;
}

fn main() {
}

What I get in the regions variable(and, well, similar results are in it's source - included_regions()) is as follows: { { "rust", 2, 38, 3, 1 }, { "rust", 2, 19, 2, 37 }, { "rust", 2, 12, 2, 14 }, { "rust", 1, 14, 2, 11 } }. Which is not what I expect - one of the regions, according to the logs, becomes: Injected format lines { ";", "}"}. Which is obviously not a correct Rust code.

So, to summarize, I predict that there is either problem with the injected formatter itself(also we should consider that it doesn't work without true argument in this case), or with the Rust's injections. Please, inspect the problem with your pro eye, I might have missed something..

stevearc commented 8 months ago
  1. According to the treesitter [docs](https://neovim.io/doc/user/treesitter.html#LanguageTree%3Aparse()), if we pass false at this place, it won't run injections. Maybe I'm wrong, but I have empty included_regions() output, and it solves if I pass true instead.

This was left over from when I was calculating the injected regions using a more hacky method, but it didn't require nested parsing. I missed this during the refactor because that parameter only takes effect in nightly, so 0.9 continued to work. I've fixed it to pass in true now.

  1. I want to inspect the tree to see whether Rust injections are correct. However, somewhy :InspectTree command results in error for the simple Rust macro.

You can use I inside :InspectTree to display the language of various regions (which allows you to spot injections). We can see that all of this example file is rust, there is no injection. Looking at nvim-treesitter, there is only one injection defined for a macro_invocation: https://github.com/nvim-treesitter/nvim-treesitter/blob/fcf843bf14adaeee53aad1b28fabba1d3b62fc8d/queries/rust/injections.scm#L20C3-L23

And it only executes if the language is html. This language is not html, so there is no injection. The ERROR then makes sense, because we're continuing to parse the file using the rust treesitter parser, but inside that chunk it's no longer rust, it's some sort of macro language syntax.

Hopefully that explains the rest of your questions as well

ftelnov commented 8 months ago

Thank you for the response, let me summarize it a little - for now it's a problem with Rust injections. If you don't mind, I want to get a little more knowledge from you.

You can use I inside :InspectTree to display the language of various regions (which allows you to spot injections). We can see that all of this example file is rust, there is no injection

I thought injections could be the same language as source file. Isn't it so? If so, then I(and playground in general in such case) shows us nothing - it just shows the language of the node. Can we really spot the injections via playground, through some key maybe? By the way, if the language of the node becomes html due to injection, I in the playground doesn't show it, so we can't rely on the I.

Looking at nvim-treesitter, there is only one injection defined for a macro_invocation

You point to the source code of treesitter repository. I thought this one is used instead for Rust injections: https://github.com/tree-sitter/tree-sitter-rust/blob/master/queries/injections.scm It is mentioned here: https://github.com/nvim-treesitter/nvim-treesitter#supported-languages I think it is being used instead of the one you provided link to, because it is mentioned as a language support. However I can't really check it. And in this one, language of the injections is set to rust, so it must be ok.

stevearc commented 8 months ago

Hmmm...you're right the queries in the upstream parser appear to get merged with the queries defined in nvim-treesitter. Based on that, I don't actually know what is causing the parse error in the tree. Might be worth posting that minimal file to the tree-sitter-rust repo and asking about it.

I suspect that at least some of the issues with the injected formatter operating on that block are because of the treesitter parser error. It's possible there are other problems, but it will be hard to tell while the parser error remains.

ftelnov commented 8 months ago

Got it, thank you! Please don't close the issue, I'll try asking on the tree-sitter-rust repo, and be right back!