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

Return nothing if tool is not on the PATH #309

Merged
merged 1 commit into from
Aug 11, 2020

Conversation

fendor
Copy link
Collaborator

@fendor fendor commented Aug 11, 2020

As discovered by #306 (comment), I forgot to actually check that a tool is on the path!
Therefore, currently HLS breaks for everyone who built it from source since the commit 0b12fcb and has not every tool installed.

@fendor fendor requested review from lukel97 and jneira August 11, 2020 09:27
@fendor
Copy link
Collaborator Author

fendor commented Aug 11, 2020

cc @horus does this fix it for you?

Copy link
Collaborator

@lukel97 lukel97 left a comment

Choose a reason for hiding this comment

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

Whoops, I had it in my head that readProcessWithExitCode would have returned a non zero exit code if it wasn't on the PATH. Too hasty code review lol

@fendor
Copy link
Collaborator Author

fendor commented Aug 11, 2020

@bubba I was thinking the same. The problems of having no tests for a new functionality :(

@fendor fendor merged commit 02a3bac into haskell:master Aug 11, 2020
@horus
Copy link

horus commented Aug 11, 2020

cc @horus does this fix it for you?

Yes. I can confirm this PR solved my problem.
Thanks for the quick fix.

@fendor
Copy link
Collaborator Author

fendor commented Aug 11, 2020

@horus Thank you for verifying!

pepeiborra pushed a commit to pepeiborra/ide that referenced this pull request Sep 6, 2020
pepeiborra added a commit that referenced this pull request Dec 27, 2020
* code action to insert new definitions
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.

3 participants