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

Stylish Haskell: CPP parse issues #3199

Merged
merged 2 commits into from
Sep 19, 2022

Conversation

andys8
Copy link
Collaborator

@andys8 andys8 commented Sep 19, 2022

Change

  • CPP language extension
  • Manually fix some files with duplication
  • Fixed ignored filepaths in pre-commit config (ExactPrint was moved)

Result

Without this change

pre-commit run --show-diff-on-failure --all-files

plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction/ExactPrint.hs: <string>:7:1: error: parse error on input `#'
plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction/Util.hs: <string>:14:1: error: parse error on input `#'
plugins/hls-refactor-plugin/src/Development/IDE/GHC/Compat/ExactPrint.hs: <string>:5:1: error: parse error on input `#'
plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs: <string>:90:1: error: parse error on input `#'

With this change

image

image

System

stylish-haskell 0.14.2.0

Issue

@andys8
Copy link
Collaborator Author

andys8 commented Sep 19, 2022

@pepeiborra Any chance pre-commit CI job can be required in order to prevent this to happen again in the future? It's way easier to fix (or wiggle around) a format issue right when you work on a file, rather than fail all - or now some - other PRs that have nothing to do with it.

image

* CPP language extension
* Manually fix some files with duplication
It looks like `ExactPrint` was ignored before, but since absolute paths
are used and weren't adapted it wasn't ignored afterwards.
@andys8 andys8 force-pushed the improvement/stylish-haskell-cpp branch from 6a09342 to 79e47a6 Compare September 19, 2022 18:26
@pepeiborra
Copy link
Collaborator

@pepeiborra Any change pre-commit CI job can be required in order to prevent this to happen again in the future? It's way easier to fix (or wiggle around) a format issue right when you work on a file, rather than fail all - or now some - other PRs that have nothing to do with it.

@andys8 thanks a lot for this fix, it is very appreciated. The pre-commit hook was contributed by @lunaticare and accepted by @Ailrun, maybe they can comment.

@pepeiborra
Copy link
Collaborator

My thoughts:

We have some of the worst CPP code in Haskell-land, it's bad enough for contributors as it is. Having to fight CI to make stylish-haskell "parse" the CPP will make things worse.

Ideally the pre-commit-hook would ignore parse errors, and leave those files untouched. Perhaps the exit code can be used to tell a parse error apart from an unformatted file?

@pepeiborra pepeiborra enabled auto-merge (squash) September 19, 2022 19:32
@pepeiborra pepeiborra merged commit bd1d0a1 into haskell:master Sep 19, 2022
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.

2 participants