Skip to content
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

Add support for disabling redirected output in the REPL driver for usage in worksheets in the Scala Plugin for IntelliJ IDEA #16810

Merged
merged 2 commits into from Feb 7, 2023

Conversation

ghost
Copy link

@ghost ghost commented Feb 2, 2023

  • Calling setOut/setErr in a concurrent environment without any synchronization (such as the Scala compile server in the Scala Plugin for IntelliJ IDEA, which is used to execute Scala 3 worksheets) can lead to unpredictable outcomes where the out/err streams are not restored properly after changing.
  • This change adds a new default method redirectOutput which can be overriden by others to control the redirecting behavior of the REPL driver.

…age in worksheets in the Scala Plugin for IntelliJ IDEA

- Calling `setOut/setErr` in a concurrent environment without any synchronization (such as the Scala compile server in the Scala Plugin for IntelliJ IDEA, which is used to execute Scala 3 worksheets) can lead to unpredictable outcomes where the out/err streams are not restored properly after changing.
- This change adds a new default method `redirectOutput` which can be overriden by others to control the redirecting behavior of the REPL driver.
@ghost
Copy link
Author

ghost commented Feb 2, 2023

I signed the Scala CLA, please run another check.

@ghost ghost marked this pull request as ready for review February 3, 2023 11:43
@ghost
Copy link
Author

ghost commented Feb 3, 2023

We would appreciate if this change lands in Scala 3.3.0, as it is supposed to be an LTS release. Thanks.

@prolativ
Copy link
Contributor

prolativ commented Feb 3, 2023

3.3.0 is now in the phase of testing release candidates and only critical bug fixes and fixes of new regressions are allowed in, so these changes would apply only in 3.3.1 and later on. However, to make things clear: LTS actually refers to the entire minor release 3.3.x line, not to a particular patch version as the general idea of LTS is about backporting fixes and some essential improvements from newer major release lines. So until 3.4.0 is out at some point in the future, there's no distinction between LTS and the mainstream.

Copy link
Member

@dwijnand dwijnand left a comment

Choose a reason for hiding this comment

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

I'm fine with merging this. But I don't want to give the impression that we're giving guarantees on any compatibility of the API with this. As in, we won't intentionally break it but we also won't stop the world and/or consider it a critical regression if some API is changed and it happened to be utilised externally.

That said, and not withstanding, could you add some details as scaladoc to redirectOutput to the effect of what you wrote, in terms of control/concurrency/synchronization/predictability.

@Kordyjan Kordyjan added the backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. label Feb 6, 2023
@Kordyjan Kordyjan added this to the 3.3.0 backports milestone Feb 6, 2023
@Kordyjan Kordyjan merged commit 723d140 into scala:main Feb 7, 2023
@Kordyjan Kordyjan added backport:accepted This PR needs to be backported, once it's been backported replace this tag by "backport:done" and removed backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. labels Feb 7, 2023
Kordyjan added a commit that referenced this pull request Feb 17, 2023
…ver for usage in worksheets in the Scala Plugin for IntelliJ IDEA" (#16947)

Backports #16810
@Kordyjan Kordyjan added backport:done This PR was successfully backported. and removed backport:accepted This PR needs to be backported, once it's been backported replace this tag by "backport:done" labels Feb 17, 2023
@Kordyjan Kordyjan modified the milestones: 3.3.0 backports, 3.3.0 Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:done This PR was successfully backported.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants