Skip to content

Capture stdout and stderr in eval plugin #1977

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

Open
anka-213 opened this issue Jun 25, 2021 · 6 comments
Open

Capture stdout and stderr in eval plugin #1977

anka-213 opened this issue Jun 25, 2021 · 6 comments
Labels
component: hls-eval-plugin type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc..

Comments

@anka-213
Copy link
Contributor

Steps to reproduce

Run the "Evaluate..." code lens on the following code

-- >>> putStrLn "Hello"

or

-- >>> hPutStrLn stderr "Hello"

Expected behaviour

It should put

-- Hello

in a comment below, just like it would do with normal return values.

Actual behaviour

In the stderr case, the output is sent to the debug output together with the logs.
In the stdout case I believe the output is sent in the LSP channel directly to the editor as is. If you're lucky, the editor's LSP parser will handle the error gracefully instead of just crashing or something.

@jneira jneira added component: hls-eval-plugin type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc.. labels Jun 25, 2021
@xsebek
Copy link
Contributor

xsebek commented May 20, 2022

I would like to try implementing the stdout/stderr capture 🙂

In #2775 it was suggested by @pepeiborra to use -fexternal-interpreter from GHC(i).

So far I could not figure out from GHC docs and code what happens to stdout when GHCi uses the external interpreter (which seems similar to our use case) and if it could be captured 😕

@anka-213
Copy link
Contributor Author

I wonder if a simple redirect of stdout using hDuplicateTo like silently does wouldn't actually work? As long as we make sure that all the output that should be sent to the actual stdout/stderr gets sent to the backup copy of the handles and not the ones currently named stdout/stderr. It should be easy to do for stdout, since (afaik) all the lsp communication is done at the same place, but might be more difficult to do for stderr, since code that writes to stderr is more spread out over the codebase.

There might also be an issue with this approach if we want to run multiple eval commands in parallel, since afaik, stdout is global per process.

hDuplicateTo is already done to prevent crashes from accidental prints:

, argsHandleOut = do
-- Move stdout to another file descriptor and duplicate stderr
-- to stdout. This guards against stray prints from corrupting the JSON-RPC
-- message stream.
newStdout <- hDuplicate stdout
stderr `hDuplicateTo'` stdout
hSetBuffering stdout NoBuffering
-- Print out a single space to assert that the above redirection works.
-- This is interleaved with the logger, hence we just print a space here in
-- order not to mess up the output too much. Verified that this breaks
-- the language server tests without the redirection.
putStr " " >> hFlush stdout
return newStdout

Maybe all that's needed is to redirect stderr to a temporary buffer during eval and hope that there are no stray prints to stderr/stdout during that time. But of course, if we can get a more robust implementation using -fexternal-interpreter, that's better.

@xsebek
Copy link
Contributor

xsebek commented May 21, 2022

Hey @anka-213, thanks for pointing the redirection out, I was not aware of it!

The "global per process" part is exactly why we would like to use the external interpreter process as GHC and GHCi do.
Their use-case is geared more toward Template Haskell evaluation, but presumably, the output of evaluated code is captured/redirected somewhere too.

But if we can hack in some stdout support without messing it up for others, then I am all for it. 🙂

@pepeiborra
Copy link
Collaborator

Using the external interpreter is the right fix, but I think the last time I tried it didn't support graceful restarts.

@fmehta
Copy link
Contributor

fmehta commented Mar 14, 2023

One of the reasons why this feature is good to have is to work around the allow one to print non-ASCII characters, which show always escapes.

Here is a workaround for this that I found that may be helpful until this gets fixed, or may be helpful in finding a solution.

It seems that going via the pretty function in the package prettyprinter has the desired behaviour:

import Prettyprinter

-- >>> pretty "λ \n λ"
-- λ 
--  λ

@dcastro
Copy link

dcastro commented Jul 14, 2023

Another reason why capturing stdout is indeed needed: The workaround mentioned in the docs (i.e. use error) is not compatible with doctest-parallel (and, I imagine, with other similar doctest implementations):

>>> prettyPrint [1..3]
[ 1
, 2
, 3
]
[ERROR  ] [ThreadId 58] failure in expression `prettyPrint [1..3]'
[ERROR  ] [ThreadId 58] expected: [ 1
[ERROR  ] [ThreadId 58]           , 2
[ERROR  ] [ThreadId 58]           , 3
[ERROR  ] [ThreadId 58]           ]
[ERROR  ] [ThreadId 58]  but got: *** Exception: [ 1
[ERROR  ] [ThreadId 58]           ^
[ERROR  ] [ThreadId 58]           , 2
[ERROR  ] [ThreadId 58]           , 3
[ERROR  ] [ThreadId 58]           ]
[ERROR  ] [ThreadId 58]           CallStack (from HasCallStack):
[ERROR  ] [ThreadId 58]             error, called at <interactive>:39:17 in interactive:Ghci5

(Now, I could add wildcards ... to make it work, but they would be gone every time I press "Refresh", so that's no good).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: hls-eval-plugin type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc..
Projects
None yet
Development

No branches or pull requests

6 participants