Skip to content

Sanitize the setup of the default Ide.Config#1361

Merged
mergify[bot] merged 7 commits into
masterfrom
defaultHlsConfig
Feb 14, 2021
Merged

Sanitize the setup of the default Ide.Config#1361
mergify[bot] merged 7 commits into
masterfrom
defaultHlsConfig

Conversation

@pepeiborra

@pepeiborra pepeiborra commented Feb 13, 2021

Copy link
Copy Markdown
Collaborator

This adds a field in the defaultMain driver to specify the default config, replacing two callbacks and making things a bit saner.

The rest of the changes are to deal with the use of def in various places which had to be turned into a parameter.

@Ailrun Ailrun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@Ailrun

Ailrun commented Feb 13, 2021

Copy link
Copy Markdown
Member

It looks like HLint plugin requires some changes too.

@pepeiborra

Copy link
Copy Markdown
Collaborator Author

It looks like HLint plugin requires some changes too.

there isn't a sensible way to fix it, so I just plugged in the type error and let the lsp 1.0 PR take care of it

@pepeiborra pepeiborra added the merge me Label to trigger pull request merge label Feb 13, 2021
Comment thread plugins/hls-hlint-plugin/src/Ide/Plugin/Hlint.hs
@pepeiborra

Copy link
Copy Markdown
Collaborator Author

@jneira I'm dropping the "ghcide does not support update config" test. Shout if you want to keep it

@wz1000

wz1000 commented Feb 13, 2021

Copy link
Copy Markdown
Collaborator

@pepeiborra that test is already dropped in my lsp-1.0 PR.

@jneira

jneira commented Feb 13, 2021

Copy link
Copy Markdown
Member

I will not miss it, thanks for cleaning it out

@pepeiborra

Copy link
Copy Markdown
Collaborator Author

This keeps failing with:

image

@mergify
mergify Bot merged commit 57b78e7 into master Feb 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge me Label to trigger pull request merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants