-
-
Notifications
You must be signed in to change notification settings - Fork 369
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 #2598
Enable rename plugin #2598
Conversation
@@ -18,6 +18,8 @@ You can watch demos for some of these features [below](#demos). | |||
- [Call hierarchy support](#call-hierarchy) | |||
- [Qualify names from an import declaration](#qualify-imported-names) in your code | |||
- [Suggest alternate numeric formats](#alternate-number-formatting) | |||
- [Renaming](#renaming) definitions automatically across modules of *the same component* (library, executable, test suite, benchmark). |
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 is not correct. Renaming works across all modules that have been indexed (in this or previous sessions).
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.
Aha, thanks, please, could you note in which cases the lack of multicomponent support could affect the plugin behaviour? does it work for the components currently loaded? so if you open files from components forcing their loading, the renaming would work across those loaded components (and no others)?
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.
could you note in which cases the lack of multicomponent support could affect the plugin behaviour?
I already said: the plugin will rename all occurrences that have been indexed in this or previous IDE sessions.
One example where the plugin would fail to find some occurrences is the first time a project has been loaded in the IDE ever, and only a subset of the components are loaded.
does it work for the components currently loaded?
It works for all the components indexed, whether they are loaded or not.
so if you open files from components forcing their loading, the renaming would work across those loaded components (and no others)?
If you open a file from a component, that will cause the component to be fully indexed. So assuming that you wait until indexing is done, the plugin will work for that component.
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.
Ok so indexed as indexed in hiedb, i guess, thanks for the clarification
Afaiu, as we have the option haskell.checkProject
, hls by default would index the entire the project opening any file from it, right?
If it is set to false the best way to ensure the last version of files are indexes would open all files where the symbol to berenamed i present.
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.
Afaiu, as we have the option haskell.checkProject, hls by default would index the entire the project opening any file from it, right?
If by project you mean component, then yes.
If it is set to false the best way to ensure the last version of files are indexes would open all files where the symbol to berenamed i present.
If it is set to false, components won't be indexed in full on open. So you could just open all the files in the workspace.
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.
approve modulo Pepe being happy about us getting the caveats right.
I think that the caveat should be that it only works for single-component projects. It can have issues with single-components within multi-component projects. Indexing all components of files where the symbol is referenced before renaming should work (I did this manually when recording the example gif). |
Hmm, i would prefer to have a partial but consistent behaviour. If it depends on what files has been opened by the user, and the renaming is partial and potentially different between sessions the ux could be very bad. @OliverMadine so the fact hls checks the entire project would not make renaming work without opening all the files with symbol occurences?
|
I've just tested by deleting my cache and trying to rename. It does not seem to index the entire project, but only the components for open files. |
Thanks, so i am not sure anymore if we can enable it at compile time without disabling by default at runtime. It would be acceptable ask for open all files with the symbol to make it work? i am sure there will be uses surprised to see the renaming works some times and other only partially. @michaelpj thoughts? |
Okay, I give in, that does seem bad enough that we shouldn't do it. Do we have any path forwards for fixing the underlying problem? |
Can we write a rule that indexes everything in the project and then have rename depend on that? It'll be expensive but at least it will work. |
It seems hls does not know about all project components beforehand. The components are loaded via cradles on demand, while files are being opened. |
Yes. I think that if we assume that the So as you said, we just need #2009. |
dont have time to continue working on this, feel free to reuse it when enabling definitle the plugin |
//cc @OliverMadine