-
-
Notifications
You must be signed in to change notification settings - Fork 388
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Retrie: implement high-level refactorings including renaming #282
Comments
Does LSP support "asking user for a name"? |
@sureyeaah I think that depends on LSP client, and LSP itself does not specify how to get a name for renaming. |
@Ailrun right but does lsp support getting a string from a user, besides via a selection in the text buffer? Edit: This is in context of extract refactoring (via codeaction) |
At least LSP-mode (Emacs) supports that. IDK other clients though. |
Is there something like that in the LSP spec? |
The LSP showMessageRequest supports some interaction with the user. If this is not enough, extract could assign a dummy name and let the user fix it later with a rename refactoring |
IIRC, show message only accepting input from labeled buttons, and doesn't allow accepting arbitrary text. |
textDocument/rename request accepts arbitrary text, and I think we should support this if we want to add the renaming feature. |
For rename I think the editor asks the user |
The only way to get a name is the rename command. So the general workflow required is to say extract a function and give it some assigned name ( |
* Support preprocessors * Add a preprocessor for testing * Add a preprocessor test
Hello, I was hoping to tackle this project for this year's GSoC! Could you please suggest what might be a good approach to step 1 (renaming in place)? I initially had written some code to replace all references to the symbol in the current file: descriptor :: PluginId -> PluginDescriptor IdeState
descriptor pluginId = (defaultPluginDescriptor pluginId) {
pluginHandlers = mconcat
[ mkPluginHandler STextDocumentCodeLens codeLensProvider
, mkPluginHandler STextDocumentRename renameProvider
]
}
renameProvider :: PluginMethodHandler IdeState TextDocumentRename
renameProvider
state
_pluginId
(RenameParams tdi@(TextDocumentIdentifier uri) pos _progToken name)
= do
locs <- getTextEdits state tdi pos name
return $ Right (WorkspaceEdit {
_changes=Just $ singleton uri (List locs),
_documentChanges=Nothing,
_changeAnnotations=Nothing
})
getTextEdits :: IdeState
-> TextDocumentIdentifier
-> Position
-> T.Text
-> LspT c IO [TextEdit]
getTextEdits state tdi@(TextDocumentIdentifier uri) pos name
= do
mbLocs <- references state $ ReferenceParams tdi pos Nothing Nothing (ReferenceContext False)
case mbLocs of
Right (List locs) -> return [TextEdit range name | Location uri' range <- locs, uri == uri']
_ -> return [] but when renaming types, this seems to replace too much code. Am I correct to think Retrie.Replace will be better suited? |
Back when I opened this ticket However now that we have What retrie lacks is the semantic awareness, and what
|
Thanks for the detailed explanation! References do seem to be a simple solution to most of the problem, but I'll need to consider how to manage some of the less obvious renames like qualified imports. |
One of the points where renaming could be useful would be renaming on a diagnostic about duplicate definition (see #1532) |
High-level refactorings should be doable by combining minimal code generation with retrie commands.
Rename
To rename
foo
tobar
, proceed as follows:Rename
foo
tobar
in placeGenerate a new declaration:
foo
Move
Similar to rename, but in step 1 we move
bar
to the target module, while in step 2 we generatefoo
in the source module.Extract definition
The text was updated successfully, but these errors were encountered: