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

Allow BooleanSettings to be set with a colon #16425

Merged
merged 1 commit into from
Dec 1, 2022

Conversation

KacperFKorban
Copy link
Member

e.g. "-explain:true" and "-explain:false" should be allowed

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.

LGTM. I did the same in Scala 2 a few years ago to make a -Y flag on by default, but allow it to be disabled. Could you add a test that proves that in (a) -foo:true -foo:false false wins and (b) that emits that "Flag $name set repeatedly" warning.

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.

❤️

@KacperFKorban KacperFKorban marked this pull request as ready for review December 1, 2022 12:32
@KacperFKorban KacperFKorban merged commit e16ff26 into scala:main Dec 1, 2022
@KacperFKorban KacperFKorban deleted the fix-bool-settings branch December 1, 2022 12:32
@som-snytt
Copy link
Contributor

In Scala 2, the facility was required to turn off lints. retronym 2014. But dotty has no grouped settings such as -Xlint or -opt.

scala/scala@55549bf

I'm still dubious about flags that default true, just because the options are "underdocumented" or "hard to find the docs". Apparently not everyone scalac -help | grep something to check defaults. Oh, I guess I don't do that on dotty because it's on stderr. The one I always forget is colorization, scalac -help 2>&1 | grep color.

@Kordyjan Kordyjan added this to the 3.3.0 milestone Aug 1, 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.

4 participants