Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Capture output of eval plugin and some girls scout changes #4726

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
dschrempf wants to merge 6 commits into haskell:master
base: master
Choose a base branch
Loading
from dschrempf:dom/girl-scout

Conversation

Copy link
Collaborator

@dschrempf dschrempf commented Sep 12, 2025
edited
Loading

This PR contains some girl-scout changes as well as changes to how the eval plugin treats stdout and stderr.

Eval plugin

We now capture output to stdout and stderr induced as a possible side effect by the statement being evaluated. This is fragile because the output may be scrambled in a concurrent setting when HLS is writing to one of these file handles from a different thread.

Fixes #1977, most likely also #4390 (although this is debatable, we could do something more sophisticated).

-- >>> putStrLn "Hello"
-- Hello
-- >>> hPutStrLn stderr "Hello"
-- Hello
-- >>> putStrLn "Hello," >> pure "World!"
-- Hello,
-- "World!"

Girl scout changes

  • Docs: Fix some links in eval plugin in-source documentation
  • Clarify documentation of expected test failure
  • Update Nix Flake (Darwin issues seem to be solved)
  • Update some tests and avoid mentioning GHC 9.2
  • Remove comment referring to GHC 9.2 and fix code

fendor reacted with thumbs up emoji
@dschrempf dschrempf marked this pull request as draft September 12, 2025 15:20
@dschrempf dschrempf changed the title (削除) Minima girls scout changes removing code referring to deprecated GHC versions (削除ここまで) (追記) Remove code referring to GHC versions not anymore supported by HLS (追記ここまで) Sep 12, 2025
@dschrempf dschrempf force-pushed the dom/girl-scout branch 2 times, most recently from 199c13f to 6d59fcd Compare September 13, 2025 08:42
@dschrempf dschrempf changed the title (削除) Remove code referring to GHC versions not anymore supported by HLS (削除ここまで) (追記) Some girls scout changes related to GHC versions not anymore supported by HLS (追記ここまで) Sep 14, 2025
@dschrempf dschrempf force-pushed the dom/girl-scout branch 2 times, most recently from fabcd5e to 11a0f3f Compare September 14, 2025 11:27
@dschrempf dschrempf marked this pull request as ready for review September 14, 2025 14:38
@dschrempf dschrempf changed the title (削除) Some girls scout changes related to GHC versions not anymore supported by HLS (削除ここまで) (追記) Capture output of eval plugin and some girls scout changes (追記ここまで) Sep 14, 2025
Copy link
Collaborator

@fendor fendor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a proper test in hls-eval-plugin, but otherwise looks good to me, thank you for the clean up! Feel free to merge once you've added a test :)

Comment on lines 429 to 438
Variable not in scope: cls :: t0 -> t
Data constructor not in scope: C
WAS Variable not in scope: cls :: t0 -> t
WAS Data constructor not in scope: C
NOW Illegal term-level use of the class `C'
NOW defined at <interactive>:1:2
NOW In the first argument of `cls', namely `C'
NOW In the expression: cls C
NOW In an equation for `it_a5Zks': it_a5Zks = cls C
NOW Variable not in scope: cls :: t0_a5ZlS[tau:1] -> t1_a5ZlU[tau:1]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slightly weird, probably needs to be fixed one way or another?

Copy link
Collaborator Author

@dschrempf dschrempf Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't realize the eval plugin changed some other results. Thanks for picking that up. I will investigate.

Copy link
Collaborator Author

@dschrempf dschrempf Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed those WAS NOW errors, one question though:

Does the plugin also evaluate all actions in the same comment block in your case?

If I evaluate one action in this comment block, all of them get evaluated and a lot of WAS NOW messages get added. Some of them are wrong. I am not sure if this PR added this discrepancy, or if the problem was already there before. I will check.

Copy link

theGhostJW commented Sep 25, 2025
edited
Loading

This could be an interesting surprise for some users. I often eval expressions that produce > 1000 lines of IO. In my case detailed pretty printed logs. If this was to suddenly be rendered in the code editor I'd need to change my workflow. If i wasn't expecting it, it would be quite confusing.

Copy link
Collaborator Author

Yes, if this is your workflow. In theory, we could limit the amount of output. The underlying library redirects output to a temporary file and then reads this temporary file. We could amend the function so that it only takes the first N lines of this file (or bytes, or whatever).

theGhostJW reacted with thumbs up emoji

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fendor fendor fendor approved these changes

@July541 July541 Awaiting requested review from July541 July541 is a code owner

@wz1000 wz1000 Awaiting requested review from wz1000 wz1000 is a code owner

@berberman berberman Awaiting requested review from berberman berberman is a code owner

@michaelpj michaelpj Awaiting requested review from michaelpj michaelpj is a code owner

@guibou guibou Awaiting requested review from guibou guibou is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Capture stdout and stderr in eval plugin

AltStyle によって変換されたページ (->オリジナル) /