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

fix #12981 add REPL show diagnostics level warn | err #13000

Merged
merged 11 commits into from Jul 7, 2021

Conversation

bjornregnell
Copy link
Contributor

@bjornregnell bjornregnell commented Jul 2, 2021

See #12981

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.

The change seems reasonable to me - just need to delete the helper scripter IMO


@main
def run_insert_error(isModify: Boolean): Unit =
visitDirRecursively(replTestDir, isModify)
Copy link
Member

Choose a reason for hiding this comment

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

Nicely done :)

But we don't need to merge this into the repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! 🥰

The reason that I committed it to the repo was that I realized that any change in REPL output will require similar hacks, to avoid a very tedious manual walkthrough of all tests, so this might be needed several times again, as the REPL output, err messages etc will improve over time to get in feature parity with the Scala 2 REPL.

I have moved the util to this repo for now so it is still on the internet if someone needs it again:
https://github.com/lunduniversity/introprog/tree/master/util/repl-output-update-tool

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just pushed the delete and the checks are running...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me know if there is a better place for this little repl-output-update hack to be discoverable to this community (?) and I'll contribute it there.

Copy link
Contributor

Choose a reason for hiding this comment

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

There is --update-checkfiles which is passed via -D which should (be made to) work for REPL tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, feel free to use my util code for that if relevant.

compiler/src/dotty/tools/repl/Rendering.scala Outdated Show resolved Hide resolved
Co-authored-by: Jamie Thompson <bishbashboshjt@gmail.com>
@bishabosha bishabosha enabled auto-merge July 6, 2021 10:51
@bjornregnell
Copy link
Contributor Author

bjornregnell commented Jul 6, 2021

@bishabosha Coloring looks nice (after temporarily commenting out fatal warnings in Build.sbt):
image

@bishabosha
Copy link
Member

@bishabosha Coloring looks nice (after temporarily commenting out fatal warnings in [Build.sbt]

you can also use this line in sbt shell to reset the compiler settings for the repl task:

set `scala3-compiler-bootstrapped` / Compile / console / scalacOptions := Seq() // or whichever settings you want

@bjornregnell
Copy link
Contributor Author

The community_build_a just failed here (but it went through before the color highlighting change, I think, so I'm curious if that really is the problem or the fail is unrelated?)

@smarter
Copy link
Member

smarter commented Jul 6, 2021

That looks like flaky scalatest's tests failing:

[error] org.scalatest.ParallelTestExecutionSpec

I tried disabling them in 7556fd5 but somehow they're still being run?

@bjornregnell
Copy link
Contributor Author

Aha. As long as their tests don't depend on color highlighting of REPL output then we're good :)

@bishabosha bishabosha dismissed dwijnand’s stale review July 7, 2021 14:56

the helper script was removed

@bishabosha bishabosha merged commit a58a48a into scala:master Jul 7, 2021
@Kordyjan Kordyjan added this to the 3.0.2 milestone Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

scala 3 repl does not show if a message is an error or a warning (regression from scala 2)
6 participants