lsp
lsp copied to clipboard
use OsPath in NormalizedFilePath
Closes #445. Current status:
- existing tests pass
- breaks backward compatibility due to the necessity to add MonadThrow to the type of some functions
- use
filepath-compat
to avoid CPP, and always useOsPath
inNormalizedFilePath
- provide two sets of functions for
NormalizedFilePath
. One forFilePath
, another forOsPath
refresh
✅ Pull request refreshed
@Mergify refresh
refresh
✅ Pull request refreshed
I added a dimension to the test matrix, but Mergify is waiting for the old one. Strange.
@michaelpj After some rewrites, I'm finally happy with the current version. The most notable change is that now we always store a UTF-8 encoded ShortByteString
in NormalizedFilePath
, so that toNormalizedFilePath :: FilePath -> NormalizedFilePath
is still total.
And please only approve if you feel this is good. Let Mergify squash and merge it for us.
Oops, I missed the label! oh well, if you want to address any of the comments you can do a follow-up
Looking plausible! I worry that maybe this extra encoding/decoding might lose some of the benchmark improvements you were getting?
Does this still use old directory/base for file operations? Because all of them do encoding/decoding at the FFI boundary. The only thing you'll gain is lower memory footprint due to avoiding String.
Can't say which one has the bigger impact.
Does this still use old directory/base for file operations?
NormalizedFilePath
in lsp-types
doesn't use directory
or some FilePath
based IO functions. In HLS, we still use the old directory/base. (HLS can not upgrade before GHC 9.6)