Skip to content

Fix #21242: Add REPL flag to quit after evaluating init script #22636

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

Merged
merged 5 commits into from
Feb 28, 2025

Conversation

noti0na1
Copy link
Member

@noti0na1 noti0na1 commented Feb 21, 2025

Adding a new REPL flag --repl-quit-after-init: evaluate the init script and skip the interactive mode.

Fix the remaining part of #21242

Test with flag at REPL startup:

> ./bin/replQ --repl-quit-after-init --repl-init-script 'println("Hello from init script!"); val i = 2 * 2' 
Hello from init script!
val i: Int = 4
>

@Gedochao
Copy link
Contributor

Can we have a test case for this?
Just to verify the output, and that a sub-terminal doesn't get created.
Preferably with dotty.tools.repl.Main, rather than the legacy runner.

@noti0na1
Copy link
Member Author

Can we have a test case for this?

You can test it locally, the process terminates immediately, but I don't know how to create a test to verify that.

@Gedochao
Copy link
Contributor

You can test it locally, the process terminates immediately, but I don't know how to create a test to verify that.

I can see that it works the way it is now, but an automated anti-regression test would go a long way.
I believe it'd be sufficient to call the REPL main method with the --repl-init-script and --repl-eval args, and then verify the output on stdout.

@noti0na1
Copy link
Member Author

Oh, I find this test we may be able to reuse: compiler/test/dotty/tools/scripting/BashExitCodeTests.scala

@Gedochao
Copy link
Contributor

Oh, I find this test we may be able to reuse: compiler/test/dotty/tools/scripting/BashExitCodeTests.scala

Seems the test is failing 🤔

@Gedochao Gedochao added the release-notes Should be mentioned in the release notes label Feb 24, 2025
@dwijnand
Copy link
Member

Why --repl-eval? I really don't intuit "quit after evaluating init script" from that... Picking --repl-eval-init-script-and-quit is verbose but clear. But I don't know what the best name is... --repl-skip-interactive, --repl-eval-only is still iffy to me, dunno... 👨🏼‍🎨

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.

Other than the name, LGTM

@som-snytt
Copy link
Contributor

som-snytt commented Feb 25, 2025

I had the same reaction to naming, but I haven't followed the conversation, which mentions Ammonite as a model.

Regular scala 2 REPL:

➜  snips scala -e 'println("hello")' -I world.scala
world
hello
➜  snips

If I wanted to switch the order of evaluation, I'd be inclined to add -E.

Edit: quick musing: there's a set of such things I must always look up, such as how to turn off colors in sbt vs scalac. I kind of remember, but it's impractical to try out all the variants, color vs colors, no vs never, etc. I can try out -E or -e quickly, if I've forgotten.

In a problem space where I can't try it out (I could try out ls -R vs ls -r if I forgot), then I will never use it casually. Possibly, if I used it, I would only cut/paste, unless I finally had to go in search of doc.

Another example is -Wconf, where I often must -Wconf:help. (Warnings now issue extra help.)

@noti0na1
Copy link
Member Author

noti0na1 commented Feb 25, 2025

  • scala -e only evaluates the file and show the output;
  • --repl-eval will evaluate as the input to repl, so the output and result will be shown line by line.

Maybe @Gedochao can point out more usages 🤔

@noti0na1
Copy link
Member Author

Why --repl-eval? I really don't intuit "quit after evaluating init script" from that... Picking --repl-eval-init-script-and-quit is verbose but clear. But I don't know what the best name is... --repl-skip-interactive, --repl-eval-only is still iffy to me, dunno... 👨🏼‍🎨

Or --repl-quit-after-init?

@Gedochao
Copy link
Contributor

  • scala -e only evaluates the file and show the output;
  • --repl-eval will evaluate as the input to repl, so the output and result will be shown line by line.

Maybe @Gedochao can point out more usages 🤔

Generally speaking, it allows to interact with the REPL without spawning a sub-terminal, which will make testing REPL integrations (on top of which Scala CLI builds) much easier. That's also been the primary motivation for this change.
Other than that, it can be used rather similarly to scala -e.

Gedochao
Gedochao previously approved these changes Feb 26, 2025
Copy link
Contributor

@Gedochao Gedochao left a comment

Choose a reason for hiding this comment

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

LGTM

@Gedochao
Copy link
Contributor

@noti0na1 not merging yet, as to let you finalize the naming of the option.
I'm fine with both what's currently there and what was suggested.

@Gedochao Gedochao linked an issue Feb 26, 2025 that may be closed by this pull request
2 tasks
Copy link
Member

@hamzaremmal hamzaremmal left a comment

Choose a reason for hiding this comment

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

We should not ship the repl command, this diminishes SIP-46. No new script should go in the dist/bin folder without careful thinking (and it shouldn't have a generic name as repl).

@Gedochao
Copy link
Contributor

We should not ship the repl command, this diminishes SIP-46. No new script should go in the dist/bin folder without careful thinking (and it shouldn't have a generic name as repl).

Right, I didn't connect the dots that this script would get subsequently shipped alongside scala and scalac.
100% agreed that we shouldn't ship it.

@hamzaremmal hamzaremmal dismissed Gedochao’s stale review February 26, 2025 09:27

We all agree that we shouldn't ship a new script

@noti0na1
Copy link
Member Author

  • Rename the flag to --repl-quit-after-init so the behaviour is more clear by its name;
  • Removed the script from dist/bin.

@noti0na1 noti0na1 requested a review from Gedochao February 26, 2025 15:44
@noti0na1 noti0na1 enabled auto-merge February 28, 2025 14:45
@noti0na1 noti0na1 merged commit fc61936 into scala:main Feb 28, 2025
29 checks passed
@noti0na1 noti0na1 deleted the add-repl-eval-only-flag branch February 28, 2025 15:26
@WojciechMazur WojciechMazur added this to the 3.7.0 milestone Mar 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes Should be mentioned in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a setting/API to run a code snippet/script in the REPL on startup
6 participants