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

New rename plugin implementation #2108

Merged

Conversation

OliverMadine
Copy link
Collaborator

@OliverMadine OliverMadine commented Aug 17, 2021

A new PR for my GSoC rename plugin since I'm changing my approach to the solution.


After spending some time implementing a solution with Retrie, I've realised it would be better just to modify the source directly (using SYB).

The advantages are:

  • Compatibility with earlier ghc versions
  • No issues with Retrie compatibility on windows
  • We can rename non top-level names
  • Easier solution to maintain

I am leaving the plugin disabled by default since we still need all relevant files to be indexed to ensure correctness.
The work towards a full solution to this has been started, however, if we get #2009 merged, then I can look into making a temporary solution (by assuming the hie.yaml lists all components).

Example

Renaming 221 occurrences of the IdeState data type in HLS.
HLS rebuilt successfully after this rename!

rename-demo

Closes #282.

project structure
initial test cases
directly modify source to rename references
support ghc9
revert retrie version bump
disable feature by default
remove unnecessary flag in test.yml
consistent indentation
increase retrie version for ghc 9.0.1 (decreased by mistake)
…erMadine/haskell-language-server into rename-plugin-directly-parse-source
Copy link
Collaborator

@pepeiborra pepeiborra left a comment

Choose a reason for hiding this comment

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

Looks good to go with only minor comments!

plugins/hls-rename-plugin/hls-rename-plugin.cabal Outdated Show resolved Hide resolved
plugins/hls-rename-plugin/src/Ide/Plugin/Rename.hs Outdated Show resolved Hide resolved
@mpickering
Copy link
Contributor

What is the performance of this plugin like on a big project?

@OliverMadine
Copy link
Collaborator Author

What is the performance of this plugin like on a big project?

As a rough approximation, renaming 221 occurrences of the IdeState data type in HLS took around 15 seconds on my machine. I've attached a video example of this in the PR description.

@pepeiborra
Copy link
Collaborator

What is the performance of this plugin like on a big project?

As a rough approximation, renaming 221 occurrences of the IdeState data type in HLS took around 15 seconds on my machine. I've attached a video example of this in the PR description.

How does it compare to the retrie version of the plugin?

fix typo in indentation test
break lines at 100 chars
rename some vars
@samhh samhh mentioned this pull request Aug 18, 2021
16 tasks
remove Retire dependency by using exactprint directly
group references by file to increase speed of checking `elem` per file
add test cases (typeclass, let expression)
add some documentation
fix bug when looking up reference in HieDb
improve error messages
rename some function / vars
…ver into rename-plugin-directly-parse-source
@OliverMadine
Copy link
Collaborator Author

OliverMadine commented Aug 19, 2021

How does it compare to the retrie version of the plugin?

This version actually seems much faster; the Retrie version was taking over 1 min for this rename. Maybe there was a bug or some overhead from calling Retrie individually for each file on that branch.

@pepeiborra pepeiborra merged commit 28222e9 into haskell:master Aug 21, 2021
@pepeiborra
Copy link
Collaborator

This is merged. Congratulations @OliverMadine on landing the first version of the rename plugin!

@jneira
Copy link
Member

jneira commented Sep 13, 2021

I am not able to see the demo in my firefox, @OliverMadine could you attach a demo in gif format to add to ChangeLog and other docs? thanks!

@OliverMadine
Copy link
Collaborator Author

OliverMadine commented Sep 13, 2021

@jneira Here you go.

rename-demo

@Ciantic
Copy link

Ciantic commented Nov 12, 2021

Is this still disabled as mentioned in the parent post? I can't seem to get it working on the brand new stack new project. It's not visible in quick fixes (aka "Refactor..." command) nor it's available from my normal shortcut:

image

@OliverMadine
Copy link
Collaborator Author

OliverMadine commented Nov 12, 2021

Is this still disabled as mentioned in the parent post?

Unfortunately it is still disabled (#2193) :(

You can enable it for use on a single component projects by building the HLS with cabal build --flag="+rename" and setting the Server Executable Path in vscode to the resulting executable. More details here.

Also, note: once it is enabled you can use it through the rename command. It is not a code action.

Screenshot 2021-11-12 at 20 25 51

@Karthik-Dulam
Copy link

Has this been merged into the release builds?
To me at least rename is not supported by the LSP.
(Using nvim-lsp.)

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.

Retrie: implement high-level refactorings including renaming
6 participants