-
-
Notifications
You must be signed in to change notification settings - Fork 367
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
Enable rename plugin #2809
Enable rename plugin #2809
Conversation
- 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
There was a problem hiding this comment.
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?
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 |
I would suggest putting the behaviour in this PR behind a plugin option, enabled by default if that's what we want. |
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. |
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. |
I mean that it's not possible to turn this change off, period. Anyone can do |
I totally agree that we shouldn't actually lose the old functionality! |
Thanks for the feedback @pepeiborra @michaelpj! I’ve added a plugin option so that we can maintain the old functionality. |
I believe that an additional PR to https://github.com/haskell/vscode-haskell will be needed to expose the config option |
|
There was a problem hiding this comment.
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!
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:
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:
Closes #2804
Closes haskell/vscode-haskell#560
Relevant issues: #2598