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

Initial Retrie plugin #266

Merged
merged 13 commits into from
Aug 3, 2020
Merged

Initial Retrie plugin #266

merged 13 commits into from
Aug 3, 2020

Conversation

pepeiborra
Copy link
Collaborator

@pepeiborra pepeiborra commented Aug 1, 2020

Supports RULES, functions and type synonyms

Future work:

Closes #103

/cc @xich

@pepeiborra pepeiborra requested a review from alanz August 1, 2020 12:41
Supports RULES, functions and type synonyms

Future work:

- Handling names properly (when retrie/#10 is fixed)
- Suggestions for pattern synonyms (when retrie/#15 is released)
- Refactorings: rename, extract, move, etc..
- Automatically add imports when unfolding
- Proper support for workspace folders
@pepeiborra pepeiborra force-pushed the retrie branch 3 times, most recently from d754eb2 to e6ad0a5 Compare August 1, 2020 15:23
README.md Outdated Show resolved Hide resolved
#define GHC_API_VERSION_H

#ifdef GHC_LIB
#define MIN_GHC_API_VERSION(x,y,z) MIN_VERSION_ghc_lib(x,y,z)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this mean we can build hls with -fghc-lib now? That might solve the hlint ghc-lib mismatch issue @jneira

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't know about that, I have never tried to build ghcide with ghc-lib. Ping @cocreature

Copy link
Collaborator

@alanz alanz left a comment

Choose a reason for hiding this comment

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

LGTM

My only concern is that it provides code actions for the whole file, rather than in the context.

I wonder if we shouldn't split this into diagnostics and code actions, much as hlint (in hie) does.

@pepeiborra
Copy link
Collaborator Author

LGTM

My only concern is that it provides code actions for the whole file, rather than in the context.

I wonder if we shouldn't split this into diagnostics and code actions, much as hlint (in hie) does.

The intent is for the code actions to be attached to the relevant lines, not to the whole file. Is that not what you see?

I don't follow your second comment, what diagnostics?

@alanz
Copy link
Collaborator

alanz commented Aug 3, 2020

The intent is for the code actions to be attached to the relevant lines, not to the whole file. Is that not what you see?

TBH, I have not run the code, just looked at it. And I see now that the range is converted to a pos and then used as a filter.

I don't follow your second comment, what diagnostics?

The normal process is that a plugin provides diagnostics, which show up as markings in the file and a list of issues to address. When the cursor is placed on a line with a diagnostic, this is returned to the code action provider which can propose fixes.

It seems to me that you will not know that retrie has a rewrite suggestion unless the cursor is on a line with a rewrite in it?

If so, my suggestion is to run retrie in an unconstrained way, on the whole file, and turn the suggestions into diagnostics. This is how we process hlint, it runs to generate diagnostics about possible changes, and if you go to the line you get an action to actually change it.

@pepeiborra
Copy link
Collaborator Author

If so, my suggestion is to run retrie in an unconstrained way, on the whole file, and turn the suggestions into diagnostics. This is how we process hlint, it runs to generate diagnostics about possible changes, and if you go to the line you get an action to actually change it.

Retrie does not generate suggestions. It takes code modding commands as inputs, e.g. "unfold foo" and applies them.

@alanz
Copy link
Collaborator

alanz commented Aug 3, 2020

Ok, any reason not to merge then?

@pepeiborra
Copy link
Collaborator Author

Ok, any reason not to merge then?

Merge away, I'll open issues for the future work items and any TODOs left in the code.

@alanz alanz merged commit ba46353 into haskell:master Aug 3, 2020
pepeiborra pushed a commit that referenced this pull request Dec 27, 2020
haskell-lsp 0.19 has started to normalise file paths completely so we
need to make sure that NormalizedFilePath agrees with that, otherwise,
we get a bunch of test failures on the daml repo (they are not
specific to DAML, but atm ghcide CI does not run windows).
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 plugin
5 participants