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

Try using haskell/lsp:master #1894

Closed
wants to merge 4 commits into from
Closed

Conversation

anka-213
Copy link
Contributor

@anka-213 anka-213 commented Jun 5, 2021

This PR is testing/demonstrating the test failures caused by: haskell/lsp#326
Issue #1895 is for fixing them.

See also #1649 (comment)

This pr is causing test failures:
haskell/lsp#326
@anka-213
Copy link
Contributor Author

anka-213 commented Jun 6, 2021

Huh, apparently those tests succeeded here. I was sure the failed locally. I wonder what changed.

No they didn't. It's just that ghcide failed a test before that.

@anka-213
Copy link
Contributor Author

anka-213 commented Jun 6, 2021

This is the ghcide test failure:

Starting LSP server...
If you are seeing this in a terminal, you probably should have run WITHOUT the --lsp option!
Started LSP server in 0.00s
setInitialDynFlags cradle: Cradle {cradleRootDir = "/home/runner/work/haskell-language-server/haskell-language-server/ghcide", cradleOptsProg = CradleAction: Cabal}
    notification handlers run sequentially: FAIL
      Exception: Timed out waiting to receive a message from the server.

@pepeiborra Do you know if this is related? I can't reproduce it on my local (MacOS) machine.

@pepeiborra
Copy link
Collaborator

I cannot repro either but it could be related as there are two usages of getCurrentDirectory in the ghcide codebase.
I'll prepare a PR with fixes and then you can rebase this one on top to test with lsp HEAD.

@anka-213
Copy link
Contributor Author

anka-213 commented Jun 7, 2021

It looks like #1897 did indeed fix the test failures!

@jneira
Copy link
Member

jneira commented Jul 14, 2021

@anka-213 could be this marked as ready for review?

@jneira jneira added the status: needs info Not actionable, because there's missing information label Jul 14, 2021
@anka-213
Copy link
Contributor Author

anka-213 commented Jul 14, 2021

Oh, this was never intended to be merged, it was just intended as a demonstration of #1895.
I believe we can just close this now. Unless you see a different reason for doing this?

@anka-213 anka-213 closed this Jul 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: needs info Not actionable, because there's missing information
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants