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

Add support for brittany (needs aeson-2) and floskell with ghc-9.0.1 #2551

Merged
merged 41 commits into from Jan 12, 2022

Conversation

jneira
Copy link
Member

@jneira jneira commented Dec 27, 2021

Copy link
Collaborator

@michaelpj michaelpj left a comment

Choose a reason for hiding this comment

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

A few ideas: I think you could mostly get away without the CPP. The only one I'm stuck on is the adjustJson call...

ghcide/src/Development/IDE/GHC/Orphans.hs Outdated Show resolved Hide resolved
hls-plugin-api/src/Ide/Plugin/Properties.hs Outdated Show resolved Hide resolved
hls-plugin-api/src/Ide/Plugin/Properties.hs Outdated Show resolved Hide resolved
hls-plugin-api/src/Ide/Plugin/Properties.hs Outdated Show resolved Hide resolved
hls-plugin-api/src/Ide/Plugin/ConfigUtils.hs Outdated Show resolved Hide resolved
hls-plugin-api/src/Ide/Plugin/ConfigUtils.hs Outdated Show resolved Hide resolved
(SomePropertyKeyWithMetaData SNumber MetaData {..}) ->
s A..= defaultValue
fromString s A..= defaultValue
Copy link
Member Author

Choose a reason for hiding this comment

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

I had to inline fromString cause ghc was not able to infer the type (tips on how to convince it appreciated)

Copy link
Member

@Ailrun Ailrun left a comment

Choose a reason for hiding this comment

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

Thank you for the update!

cabal-ghc901.project Outdated Show resolved Hide resolved
cabal-ghc901.project Outdated Show resolved Hide resolved
@jneira jneira marked this pull request as ready for review December 30, 2021 21:35
@jneira
Copy link
Member Author

jneira commented Jan 10, 2022

shake-bench also needs aeson-2 compatibility 🤦

@michaelpj
Copy link
Collaborator

i think i got it

yeah, looks great!

@michaelpj
Copy link
Collaborator

I'll take a look at shake-bench.

@jneira
Copy link
Member Author

jneira commented Jan 10, 2022

hmm dont know why but the error only is shown in the stack build:


shake-bench                       > /root/build/shake-bench/src/Development/Benchmark/Rules.hs:501:33: error:
shake-bench                       >     • Couldn't match type ‘Text’ with ‘Data.Aeson.Key.Key’
shake-bench                       >       Expected: GHC.Exts.Item
shake-bench                       >                   aeson-2.0.2.0:Data.Aeson.Types.Internal.Object
shake-bench                       >         Actual: (Text, Value)
shake-bench                       >     • In the pattern: (name, String gitName)
shake-bench                       >       In the pattern: [(name, String gitName)]
shake-bench                       >       In the pattern: toList -> [(name, String gitName)]
shake-bench                       >     |
shake-bench                       > 501 |   parseJSON (Object (toList -> [(name, String gitName)])) =
shake-bench                       >     |                                 ^^^^^^^^^^^^^^^^^^^^^^
shake-bench                       > 
shake-bench                       > /root/build/shake-bench/src/Development/Benchmark/Rules.hs:503:33: error:
shake-bench                       >     • Couldn't match type ‘Text’ with ‘Data.Aeson.Key.Key’
shake-bench                       >       Expected: GHC.Exts.Item
shake-bench                       >                   aeson-2.0.2.0:Data.Aeson.Types.Internal.Object
shake-bench                       >         Actual: (Text, Value)
shake-bench                       >     • In the pattern: (name, Object props)
shake-bench                       >       In the pattern: [(name, Object props)]
shake-bench                       >       In the pattern: toList -> [(name, Object props)]
shake-bench                       >     |
shake-bench                       > 503 |   parseJSON (Object (toList -> [(name, Object props)])) =
shake-bench                       >     |                                 ^^^^^^^^^^^^^^^^^^^^
shake-bench                       > 
shake-bench                       > /root/build/shake-bench/src/Development/Benchmark/Rules.hs:515:37: error:
shake-bench                       >     • Couldn't match type ‘Text’ with ‘Data.Aeson.Key.Key’
shake-bench                       >       Expected: GHC.Exts.Item
shake-bench                       >                   aeson-2.0.2.0:Data.Aeson.Types.Internal.Object
shake-bench                       >         Actual: (Text, Value)
shake-bench                       >     • In the expression: (n, String gitName)
shake-bench                       >       In the first argument of ‘fromList’, namely ‘[(n, String gitName)]’
shake-bench                       >       In the second argument of ‘($)’, namely
shake-bench                       >         ‘fromList [(n, String gitName)]’
shake-bench                       >     |
shake-bench                       > 515 |       Just n  -> Object $ fromList [(n, String gitName)]
shake-bench                       >     | 

@michaelpj
Copy link
Collaborator

Probably because we don't have benchmarks: true in cabal.project, so shake-bench gets built only if you ask for it.

@jneira
Copy link
Member Author

jneira commented Jan 10, 2022

Probably because we don't have benchmarks: true in cabal.project, so shake-bench gets built only if you ask for it.

i just did a cabal build --enable-benchmarks --project-file .\cabal-ghc901.project and it did not build shake-bench but a cabal build shake-bench --project-file .\cabal-ghc901.project worked. 🤷

@jneira
Copy link
Member Author

jneira commented Jan 10, 2022

Probably because we don't have benchmarks: true in cabal.project, so shake-bench gets built only if you ask for it.

i just did a cabal build --enable-benchmarks --project-file .\cabal-ghc901.project and it did not build shake-bench but a cabal build shake-bench --project-file .\cabal-ghc901.project worked. 🤷

due to haskell/cabal#6259 🤦

@michaelpj
Copy link
Collaborator

jneira#63

@jneira
Copy link
Member Author

jneira commented Jan 10, 2022

Wow that was fast, many thanks

@jneira jneira changed the title Add support for brittany and floskell with ghc-9.0.1 (needs aeson-2) Add support for brittany (needs aeson-2) and floskell with ghc-9.0.1 Jan 10, 2022
@@ -68,16 +68,19 @@ module Development.Benchmark.Rules

import Control.Applicative
import Control.Monad
import Control.Lens ((^.))
Copy link
Member

Choose a reason for hiding this comment

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

This one is a fairly minor thing but the formatting is not applied

haskell-language-server.cabal Show resolved Hide resolved
test/functional/FunctionalCodeAction.hs Show resolved Hide resolved
@jneira
Copy link
Member Author

jneira commented Jan 11, 2022

the new parsing in benchmarks is slighty different from the old one:

Error: Artifact path is not valid: /unprofiled/lsp-types/"upstream"/code_actions.csv. Contains the following character: Double quote "

@michaelpj
Copy link
Collaborator

the new parsing in benchmarks is slighty different from the old one

Ugh, I screwed up my string conversions trying to make them generic. I'll try again.

@michaelpj
Copy link
Collaborator

jneira#64

@jneira
Copy link
Member Author

jneira commented Jan 12, 2022

jneira#64

many thanks, lets see if ci is green here too ;-)

@jneira
Copy link
Member Author

jneira commented Jan 12, 2022

Ok it seems is ok to be merged, @michaelpj @Ailrun many thanks!

@jneira jneira added the merge me Label to trigger pull request merge label Jan 12, 2022
@michaelpj
Copy link
Collaborator

What a marathon lol

@michaelpj
Copy link
Collaborator

Mergify doesn't want to merge for some reason, but the checks are all green, so I'll hit the button.

@michaelpj michaelpj merged commit 2625689 into haskell:master Jan 12, 2022
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.

Build with aeson >= 2.0
4 participants