Skip to content

Fixed none cradle files shown as failed #3270

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

Closed
wants to merge 2 commits into from

Conversation

Soupstraw
Copy link

@Soupstraw Soupstraw commented Oct 8, 2022

This PR introduces a hacky fix to filter out the ignored files when printing out the failed files.

closes #2692

@pepeiborra
Copy link
Collaborator

pepeiborra commented Oct 8, 2022

I don't think we want to merge this PR in its current state. What are we trying to accomplish here?

My recommendation would be to start by writing a failing test that demonstrates the issue that you are trying to fix. Define a sample project with a none cradle, and then write an lsp-test style test in the ghcide test suite that clearly displays the issue, if there is an issue at all.

@Soupstraw
Copy link
Author

Currently whenever a none cradle matches any files, haskell-language-server exits with a nonzero exit code. This PR keeps track of the files that match a none cradle and removes those files from the "failed" files list.

@pepeiborra
Copy link
Collaborator

Currently whenever a none cradle matches any files, haskell-language-server exits with a nonzero exit code. This PR keeps track of the files that match a none cradle and removes those files from the "failed" files list.

Could we have some comments documenting this purpose in the code? As well as a test that the change does what it claims and not more.

@pepeiborra
Copy link
Collaborator

Does this really close #2692? It's unclear to me whether the issue is just with the command line checker or there is more

@Soupstraw
Copy link
Author

I think this only happens with the command-line checker, since the code that collects the failed modules and prints them out was at the end of the Check branch of the case analysis in the main function.

@Soupstraw
Copy link
Author

I think I can also write some tests to ensure that a file matching a none cradle doesn't automatically cause hls to exit with a failure.

@fendor
Copy link
Collaborator

fendor commented Oct 8, 2022

Afaict, the linked issue is only about a "minor" display bug in the HLS cli. None-cradle is supported otherwise.

@fendor
Copy link
Collaborator

fendor commented Oct 8, 2022

However, I think the change should be local to the HLS cli and ideally not affect the LSP use in any way. Need to check again how difficult this is.

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.

none-cradle is not respected by HLS
4 participants