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

Apply formats again #1824

Merged
merged 3 commits into from
May 13, 2021
Merged

Apply formats again #1824

merged 3 commits into from
May 13, 2021

Conversation

Ailrun
Copy link
Member

@Ailrun Ailrun commented May 12, 2021

I'm now considering adding CI check for styling, but one big issue is that Haskell formatters (including Stylish-haskell, Ormolu, and Brittany) are not able to parse some specific usage of CPP pragmas.
For example, formatters cannot parse this code:

  test
#if defined(SOME_MACRO)
    = definition1
#else
    = definition2
#endif

and give an error. (However, if we move test to both branches they do not give an error)

We have quite a lot modules using pragmas in this way, so adding CI check will cause an unnecessary problems to modifying these modules.

Any idea will be appreciated.

@Ailrun Ailrun requested review from pepeiborra and jneira May 12, 2021 17:38
@pepeiborra
Copy link
Collaborator

Could you disable the formatted in certain modules using a pragma?

@Ailrun
Copy link
Member Author

Ailrun commented May 12, 2021

@pepeiborra Yes, that's a way, but will the list maintenance cost be OK?

Maybe it's not that expensive though...

@jneira jneira added the merge me Label to trigger pull request merge label May 12, 2021
@jneira
Copy link
Member

jneira commented May 12, 2021

Fixing upstream our actual formatter? :-P

@jneira jneira removed the merge me Label to trigger pull request merge label May 12, 2021

# Temporarily ignored files
# Stylish-haskell (and other formatters) does not work well with some CPP usages in these files
"^ghcide/src/Development/IDE/GHC/Compat.hs$" "^plugins/hls-splice-plugin/src/Ide/Plugin/Splice.hs$" "^ghcide/test/exe/Main.hs$" "ghcide/src/Development/IDE/Core/Rules.hs" "^hls-test-utils/src/Test/Hls/Util.hs$"
Copy link
Member Author

Choose a reason for hiding this comment

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

Now these are added

@Ailrun Ailrun added the merge me Label to trigger pull request merge label May 13, 2021
@mergify mergify bot merged commit e0a4642 into haskell:master May 13, 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.

3 participants