Skip to content
This repository has been archived by the owner on Apr 25, 2020. It is now read-only.

file map line pragmas #906

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Conversation

lierdakil
Copy link
Collaborator

This turned out a bit more messy than I would like due to Literate Haskell files. That said, still better than the mess we have currently I guess?

Updated tests pass on my machine, but I didn't check on anything but GHC 8.0.

/cc @wz1000 -- I believe this was your idea?

NOTE: As of now, this introduces a behavior change. Mapped files specified with file path are handled as temp.files now, which means changes made to redirected files made after initial load are not visible to ghc-mod. Not sure if anyone relied on earlier behavior. Should be possible to reload those each time they're needed, but that's a bit of an overhead. Feedback from someone actually using file redirection (as opposed to loading from memory/stdin) would be appreciated. @voanhduy1512?

@wz1000
Copy link

wz1000 commented Aug 23, 2017

We tried this but it breaks stuff which uses withMappedFile and doesn't rely on GHC to parse the file(textual operations like diffs etc.). So with these changes, we would have to another version of the file in HIE ourself. Right now, we can depend on ghc-mod to manage document truth for us.

Also, why have you removed mkRevRedirMapFunc?

@lierdakil
Copy link
Collaborator Author

lierdakil commented Aug 23, 2017 via email

@wz1000
Copy link

wz1000 commented Aug 23, 2017

Can you confirm that all the entries in the AST(parsed, renamed, typechecked etc.) have the correct FilePaths everywhere after this? If that is the case, then I guess we shouldn't need mkRevDirMapFunc anymore.

We don't make changes to mapped files, but we do feed them to hlint, HaRe, apply-refact, brittany etc. and generate the edit the client should apply by diffing the mapped file with the new file source we get from those tools.

Basically, in HIE, whenever a tool wants to read the source file, we give it the file using withMappedFile. This gives us a single way to manage document truth.

@voanhduy1512
Copy link

Actually i hit by this file redirect when working on this PR dense-analysis/ale#846. I am not sure how this changes can affect the linter (my guess is it doesn't). I will build a new binary from your pull and check if it break anything when using with ale.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants