-
Notifications
You must be signed in to change notification settings - Fork 92
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
Use appropriate number types #366
Conversation
Many thanks for fixing this. Would need it some work to adapt clients (like hls)? could we do something here to make easier the upgrade? |
Yeah, it'll require some changes, but just a scattering of |
@@ -1,6 +1,7 @@ | |||
{-# LANGUAGE RankNTypes #-} | |||
{-# LANGUAGE TemplateHaskell #-} | |||
{-# LANGUAGE LambdaCase #-} | |||
{-# LANGUAGE GeneralizedNewtypeDeriving #-} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this needed? There is no other changes in the module
-- might contain the index of the change that failed. This property is | ||
-- only available if the client signals a `failureHandling` strategy | ||
-- in its client capabilities. | ||
, _failedChange :: Maybe Word32 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a not related change, maybe it worths to note it as a separate commit or in another way
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops thanks, just a small thing I noticed we missed!
lsp/test/JsonSpec.hs
Outdated
@@ -7,6 +7,9 @@ | |||
{-# OPTIONS_GHC -fno-warn-orphans #-} | |||
-- For the use of MarkedString | |||
{-# OPTIONS_GHC -fno-warn-deprecations #-} | |||
{-# LANGUAGE DerivingStrategies #-} | |||
{-# LANGUAGE StandaloneDeriving #-} | |||
{-# LANGUAGE GeneralizedNewtypeDeriving #-} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More pragmas with no module changes, maybe they are needed for upstream changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, many thanks for fixing that
This ensures that we use either: - `Int32` - `Word32` - `Float` In particular, this gets us: - Appropriate `Bounded` instances (see the original issue haskell/haskell-language-server#2169). - More picky `aeson` instances for the bounded types. Rather than use newtypes, we just use the existing appropriate Haskell numeric types for bounded integers. Fixes haskell#354.
Just a small missing field.
Since type representations changed.
138208f
to
493f2cd
Compare
This ensures that we use either:
Int32
Word32
Float
In particular, this gets us:
Bounded
instances (see the original issueUse maxBound of uinteger not Int. haskell-language-server#2169).
aeson
instances for the bounded types.Rather than use newtypes, we just use the existing appropriate Haskell
numeric types for bounded integers.
Fixes #354.
NOTE: I also bumped the
lsp-types
major version, since this is a type-representation change.