Add the 'Reformat code' action to mouse context menu

[terziele]

Closes #15891

Release Notes:

• Added "Reformat code' action to mouse context menu.

[WeetHet]

I don't think this is ready to be merged yet, as seemingly the Format action reformats the whole file and I would expect this to reformat only the selected part

[terziele]

@WeetHet I'm new to the project, can you guide me where I can find such functionality in the codebase to reuse? All I can find is a editor::Format which does full file formatting.

[WeetHet]

There's probably none, and I'm even unsure how to implement it, but in the current state the behaviour is counterintuitive imho

[terziele]

@WeetHet Maybe it would be better to rename the action into Reformat File and also add it to context menu in a file picker on the left panel? As a quick solution to the request, of course. As a second PR it would be a selection formatting. But I'm not even sure, that LSP supports that kind of functionality.

[WeetHet]

@WeetHet Maybe it would be better to rename the action into Reformat File and also add it to context menu in a file picker on the left panel?

That makes sense, but is it necessary? I don't know if I want even more options in any of the menus rn. But if the community wants it sure, it's just that I think it's not highly justified. @iamnbutler you know this better than me, any advice would be nice

[WeetHet]

Seemingly LSP support it: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_rangeFormatting, we can try looking in that direction

[terziele]

@WeetHet Yeah, I've prepared the PoC, but somehow can't test it on Rust code. Simply do not get to the right branch with range format. Rust analyzer supports range format though https://github.com/rust-lang/rust-analyzer/issues/7580

https://github.com/zed-industries/zed/commit/9468aab7aee788b5d40874eefae37c7999225db1

UPD: It seems the issue with rust analyzer is at &Formatter::LanguageServer { name: None },, because my prettier_settings are default without anything.

[WeetHet]

After thinking about it for a bit, we can do both. Let's add a Format buffer for now and a Format Selection later. If you have something in the works, I suggest you may make a draft PR for formatting selection and work towards merging this one with just the whole buffer formatting

On 20 August 2024 19:16:13 UTC, Ihnat @.***> wrote:

@terziele commented on this pull request.

@@ -174,6 +173,7 @@ pub fn deploy_context_menu( deployed_from_indicator: None, }), )

•            .action("Reformat code", Box::new(Format))
  

Should it format the whole file or a selection(multiselection maybe?) as well?

-- Reply to this email directly or view it on GitHub: https://github.com/zed-industries/zed/pull/16080#discussion_r1723855312 You are receiving this because you were mentioned.

Message ID: @.***>

[terziele]

@WeetHet OK. This PR is complete then. I'll make another PR for selection formatting.

[WeetHet]

@WeetHet OK. This PR is complete then. I'll make another PR for selection formatting.

I feel like right now the context menu becomes very top heavy. How about we do the following: Four sections:

1. Finding symbols
2. Editing file using lsp
3. Copy/Cut/Paste
4. Getting file location

Here's how I think it should be like:

[WeetHet]

@maxdeviant what do you think?

[maxdeviant]

@WeetHet OK. This PR is complete then. I'll make another PR for selection formatting.

I feel like right now the context menu becomes very top heavy. How about we do the following: Four sections:

1. Finding symbols
2. Editing file using lsp
3. Copy/Cut/Paste
4. Getting file location

Here's how I think it should be like:

It seems alright to me.

Maybe it could do with a reorganize, but I think that's best left to a separate discussion/PR.

[WeetHet]

@WeetHet OK. This PR is complete then. I'll make another PR for selection formatting.

I feel like right now the context menu becomes very top heavy. How about we do the following: Four sections:

1. Finding symbols
2. Editing file using lsp
3. Copy/Cut/Paste
4. Getting file location

Here's how I think it should be like:

It seems alright to me.

Maybe it could do with a reorganize, but I think that's best left to a separate discussion/PR.

Sure, I've made a PR

[terziele]

Have made a PR as well.

• https://github.com/zed-industries/zed/pull/16823