ghc-mod icon indicating copy to clipboard operation
ghc-mod copied to clipboard

file map line pragmas

Open lierdakil opened this issue 7 years ago • 4 comments

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?

lierdakil avatar Aug 23 '17 12:08 lierdakil

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?

wz1000 avatar Aug 23 '17 12:08 wz1000

mkRevDirMapFunc should be obsoleted by this change, since I'm relying on this to be handled downstream (e. g. by ghc). Also it's a bit of a mystery to me why would you use withMappedFile outside of ghc-mod... Intended use was always like

  1. Map file
  2. Do stuff
  3. Unmap file

Mapped files were never intended to be persistent. Certainly changing temp files after mapping is going to be problematic, since those are not tracked in World?

On ср, 23 авг. 2017 г., 15:33 wz1000 [email protected] wrote:

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?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/DanielG/ghc-mod/pull/906#issuecomment-324314751, or mute the thread https://github.com/notifications/unsubscribe-auth/AG8EZnzG0qxjb4DpuZ5QaW30EH4p7WYwks5sbBwTgaJpZM4O_7Dc .

lierdakil avatar Aug 23 '17 12:08 lierdakil

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.

wz1000 avatar Aug 23 '17 13:08 wz1000

Actually i hit by this file redirect when working on this PR https://github.com/w0rp/ale/pull/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.

voanhduy1512 avatar Aug 30 '17 14:08 voanhduy1512