sublimelsp / LSP-rust-analyzer

Convenience package for rust-analyzer
MIT License
70 stars 11 forks source link

Automatically open rename dialog after extracting into a function #49

Open FichteFoll opened 2 years ago

FichteFoll commented 2 years ago

When using a code action to extract code into a function, 9 out of 10 times I need to rename the created function. LSP-rust-analyzer could make that much easier for me if it always opened the rename dialog after an extraction action, where I can then simply press esc to close it if I don't need it. I would even argue it makes sense to have this as the new default and not even make it configurable.

IntelliJ also has this behavior.

rchl commented 2 years ago

Note that ideally the actual server (https://github.com/rust-analyzer/rust-analyzer) should support that in the first place since it would have to send us a notification with a position of the new function symbol after refactoring. Otherwise I think we would be guessing where it is.

rchl commented 2 years ago

I've checked how it works and I suppose it might not be too hacky to just find the new function symbol if it's always called fun_name and it's always within the current selection.

Although then another issue is that there is no API for hooking into when specific code action is applied. So a custom notification from the server would still be nice.

rwols commented 2 years ago

There is a protocol extension that rust-analyzer defined where the text edits can contain snippet syntax. Perhaps it uses that?

rchl commented 2 years ago

Indeed it does make a difference here.

If I set:

    "experimental_capabilities": {
        "snippetTextEdit": true,
    }

in LSP-rust-analyzer.sublime-settings I get a response like this:

{
  "edit": {
    "documentChanges": [
      {
        "edits": [
          {
            "insertTextFormat": 2,
            "newText": "self.fun_name(idx);",
            "range": {
              "end": {
                "character": 65,
                "line": 17
              },
              "start": {
                "character": 8,
                "line": 17
              }
            }
          },
          {
            "insertTextFormat": 2,
            "newText": "\n\n    fn $0fun_name(&mut self, idx: usize) {\n        self.v.resize_with((idx + 1).max(self.v.len()), || None);\n    }",
            "range": {
              "end": {
                "character": 5,
                "line": 19
              },
              "start": {
                "character": 5,
                "line": 19
              }
            }
          }
        ],
        "textDocument": {
          "uri": "file:///Users/rafal/workspace/github/rust-analyzer/lib/arena/src/map.rs",
          "version": 6
        }
      }
    ]
  },
  "kind": "refactor.extract",
  "title": "Extract into function"
}

Though that still looks wrong since there are two edits and the first one doesn't have a placeholder so I'm not sure how an editor is supposed to handle that.

rchl commented 2 years ago

When applying such code action, the editor should insert snippet, with tab stops and placeholder. At the moment, rust-analyzer guarantees that only a single edit will have InsertTextFormat.Snippet.

This doesn't seem to be the case @matklad

https://github.com/rust-analyzer/rust-analyzer/blob/master/docs/dev/lsp-extensions.md

matklad commented 2 years ago

Huh, indeed -- in the above, we indeed have only snippet, but we wrongly set InsertTextFormat.Snippet. everywhere.

rchl commented 2 years ago

That's half of the problem though. There are also 2 edits. If the intention is that the user is able to easily change the function's name then that will work only partially - the user will only be able to edit the name in the function definition but not the place where it's called.