Skip to content

Enable rename plugin - #2809

Merged
pepeiborra merged 21 commits into
haskell:masterfrom
OliverMadine:rename-within-module
Apr 3, 2022
Merged

Enable rename plugin#2809
pepeiborra merged 21 commits into
haskell:masterfrom
OliverMadine:rename-within-module

Conversation

@OliverMadine

@OliverMadine OliverMadine commented Mar 29, 2022

Copy link
Copy Markdown
Collaborator

The rename plugin is disabled due to limited multi-component support in the HLS / hie-bios. This limitation causes renaming across components to work partially (depending which files have been indexed).


Within-module renaming

While the work on full multi-component support is continued, we can enable renaming of names within modules for use in both single- and multi-component projects.

I think that this is the better of the temporary solutions since it eliminates all partial behaviour, while still allowing the plugin to be of some use within multi-component projects.

The conditions required to perform a within-module rename are then:

  • the source of the name is the current module (not an imported name)
  • the current module must have an export list that does not contain the name

In the case that these are not met, we fail informatively.


Cross-module plugin option

This also adds plugin option for renaming across modules. This can be used in single-component projects or when ensuring that all components that reference the name are indexed.

By default cross-module renaming is disabled. It can be enabled with the following configuration option:

"haskell.plugin.rename.config.crossModule": true

Closes #2804
Closes haskell/vscode-haskell#560
Relevant issues: #2598

  - add context to error messages
  - remove unnecessary unwrapping of ParsedSource
  - use HashSet for references
  - consistent naming, whitespace, indentation, imports style
  - add rename package to hackage CI
  - set default build flag to True
@OliverMadine OliverMadine changed the title Rename within module Renaming within modules Mar 29, 2022

@pepeiborra pepeiborra left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This makes no sense to me, it cripples the plugin artificially in an unrecoverable way.

Sigma IDE (a custom LSP server built with HLS) includes the rename plugin since the Sigma codebase is single component. Isn't there a way to preserve the full feature set in single component projects?

@OliverMadine

OliverMadine commented Mar 29, 2022

Copy link
Copy Markdown
Collaborator Author

This makes no sense to me, it cripples the plugin artificially in an unrecoverable way.

Sigma IDE (a custom LSP server built with HLS) includes the rename plugin since the Sigma codebase is single component. Isn't there a way to preserve the full feature set in single component projects?

We could preserve the full feature set in single-component projects but I don't think we have a way of determining if we are in a multi-component project or not?

Also as @michaelpj commented I think this would rule out most users?

If we could enable full support for single-component projects, then we could use this "within module" renaming just for multi-component projects. Maybe we could regex on the hie.yaml as a quick way of determining if the project is multi-component?

@pepeiborra

Copy link
Copy Markdown
Collaborator

We could preserve the full feature set in single-component projects but I don't think we have a way of determining if we are in a multi-component project or not?

I would suggest putting the behaviour in this PR behind a plugin option, enabled by default if that's what we want.

@michaelpj

Copy link
Copy Markdown
Collaborator

Pepe, when you say it cripples it "unrecoverably", do you mean specifically that you can't turn it off for Sigma?

I agree that making it possible to get the "full" plugin is a good idea, and that probably means a plugin option.

@michaelpj

Copy link
Copy Markdown
Collaborator

I think this is an okay step and provides at least some useful renaming to users (as opposed to nothing, currently). As and when we can support multiple components, we can just stop failing on non-local identifiers.

@pepeiborra

Copy link
Copy Markdown
Collaborator

Pepe, when you say it cripples it "unrecoverably", do you mean specifically that you can't turn it off for Sigma?

I mean that it's not possible to turn this change off, period. Anyone can do cabal install haskell-language-server with the rename flag to link this plugin and use it to do multi-component renames as long as they have indexed all the components.

@michaelpj

Copy link
Copy Markdown
Collaborator

I totally agree that we shouldn't actually lose the old functionality!

@OliverMadine OliverMadine changed the title Renaming within modules Enable rename plugin Apr 2, 2022
@OliverMadine

Copy link
Copy Markdown
Collaborator Author

Thanks for the feedback @pepeiborra @michaelpj! I’ve added a plugin option so that we can maintain the old functionality.

Comment thread plugins/hls-rename-plugin/src/Ide/Plugin/Rename.hs
@pepeiborra

Copy link
Copy Markdown
Collaborator

I believe that an additional PR to https://github.com/haskell/vscode-haskell will be needed to expose the config option

@OliverMadine

Copy link
Copy Markdown
Collaborator Author

I believe that an additional PR to https://github.com/haskell/vscode-haskell will be needed to expose the config option

Done!

Comment thread ghcide/src/Development/IDE/GHC/Orphans.hs Outdated

@pepeiborra pepeiborra left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks @OliverMadine for accepting all our suggestions and getting the rename plugin enabled at last!

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.

Enable the rename plugin on single component projects Add support for using vscodes rename symbol feature

3 participants