Skip to content

Migration to text-2.* #2583

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
pepeiborra opened this issue Jan 13, 2022 · 9 comments
Closed

Migration to text-2.* #2583

pepeiborra opened this issue Jan 13, 2022 · 9 comments
Labels
old_type: distribution performance Issues about memory consumption, responsiveness, etc.

Comments

@pepeiborra
Copy link
Collaborator

pepeiborra commented Jan 13, 2022

There are at least two dependencies that block this migration:

  1. lsp-types: Migration to text 2.0 lsp#391
  2. bytestring-encoding: Support text 2.0 msakai/bytestring-encoding#6

The first one is obviously a hard requirement, but for the second one there's only one call site so we could consider dropping it:

persistentHieFileRule :: Rules ()
persistentHieFileRule = addPersistentRule GetHieAst $ \file -> runMaybeT $ do
res <- readHieFileForSrcFromDisk file
vfs <- asks vfs
encoding <- liftIO getLocaleEncoding
(currentSource,ver) <- liftIO $ do
mvf <- getVirtualFile vfs $ filePathToUri' file
case mvf of
Nothing -> (,Nothing) . T.decode encoding <$> BS.readFile (fromNormalizedFilePath file)
Just vf -> pure (Rope.toText $ _text vf, Just $ _lsp_version vf)
let refmap = Compat.generateReferencesMap . Compat.getAsts . Compat.hie_asts $ res
del = deltaFromDiff (T.decode encoding $ Compat.hie_hs_src res) currentSource
pure (HAR (Compat.hie_module res) (Compat.hie_asts res) refmap mempty (HieFromDisk res),del,ver)

@jneira jneira added old_type: distribution performance Issues about memory consumption, responsiveness, etc. type: refactor labels Jan 13, 2022
@drsooch
Copy link
Collaborator

drsooch commented Jan 19, 2022

Looks like retrie needs it's text bounds updated as well?

@Bodigrim
Copy link
Contributor

bytestring-encoding is avoidable: T.decode encoding <$> BS.readFile is just Data.Text.IO.readFile, and T.decode encoding below can be replaced with T.decodeUtf8, because Haskell sources are always in UTF-8.

I've successfully built HLS against text-2.0 with this patch: https://github.com/Bodigrim/haskell-language-server/commit/e14ea011f6386911f678223e87c0b519f7843a3e

@pepeiborra
Copy link
Collaborator Author

@Bodigrim would you be able to send a PR? I will gladly accept it

@pepeiborra
Copy link
Collaborator Author

Looks like retrie needs it's text bounds updated as well?

Opened facebookincubator/retrie#42

@Bodigrim
Copy link
Contributor

@pepeiborra sorry, I'm trying really hard to keep myself away from diving into one more project ;) All you need is this:

+import qualified Data.Text.IO                                 as T
 import           Data.Time                                    (UTCTime (..))
 import           Data.Tuple.Extra
 import           Development.IDE.Core.Compile
@@ -530,10 +530,10 @@ persistentHieFileRule = addPersistentRule GetHieAst $ \file -> runMaybeT $ do
   (currentSource,ver) <- liftIO $ do
     mvf <- getVirtualFile vfs $ filePathToUri' file
     case mvf of
-      Nothing -> (,Nothing) . T.decode encoding <$> BS.readFile (fromNormalizedFilePath file)
+      Nothing -> (,Nothing) <$> T.readFile (fromNormalizedFilePath file)
       Just vf -> pure (Rope.toText $ _text vf, Just $ _lsp_version vf)
   let refmap = Compat.generateReferencesMap . Compat.getAsts . Compat.hie_asts $ res
-      del = deltaFromDiff (T.decode encoding $ Compat.hie_hs_src res) currentSource
+      del = deltaFromDiff (T.decodeUtf8 $ Compat.hie_hs_src res) currentSource

@pepeiborra
Copy link
Collaborator Author

Sent #2628

@Bodigrim
Copy link
Contributor

Bodigrim commented Mar 5, 2022

I've been able to build HLS with text-2.0 and GHC 9.0, using this setup:

constraints:
    text >= 2.0,
    haskell-language-server -brittany -floskell -fourmolu -stylishHaskell

@pepeiborra
Copy link
Collaborator Author

pepeiborra commented Mar 5, 2022

I think we can close this issue and declare victory.

@michaelpj
Copy link
Collaborator

I agree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
old_type: distribution performance Issues about memory consumption, responsiveness, etc.
Projects
None yet
Development

No branches or pull requests

5 participants