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

Implement initial and cleanup commands as well as value bindings for REPL #3007

Closed
felixmulder opened this issue Aug 22, 2017 · 10 comments
Closed

Comments

@felixmulder
Copy link
Contributor

@3shv
Copy link

3shv commented Aug 29, 2017

While I was working on this, few doubts popped up.

  1. Can we assume initial and cleanup commands are always parsed successfully?
    -- If yes, probably we can make case class Silent extends Parsed
  2. If not, is the case class Silent always one of Parsed, SyntaxErrors, Newline, SigKill, Command?
    Hope I made sense.

@felixmulder
Copy link
Contributor Author

You're making sense :)

  1. No
  2. Yes

One way to do this is to say:

case class Silent(underlying: ParseResult) extends ParseResult {
  require(!underlying.isInstanceOf[Silent])
}

that way we can unpack it to the underlying result.

A more type safe way to do this is to introduce a marker trait extending ParseResult that denotes something concrete e.g:

          ParseResult
               |
   +-----------+-----------+
   |                       |
Silent             ConcreteParseResult
                           |
                           +--------- Parsed, SyntaxErrors, NewLine etc

And then making Silent be:

case class Silent(underlying: ConcreteParseResult) extends ParseResult

Perhaps there's a better name than concrete though 😄

@3shv
Copy link

3shv commented Aug 31, 2017

Thanks for the help. I've submitted a WIP PR, to check if I'm going in right direction. If I understood marker traits correctly, I need to declare a trait, say Concrete and attach it to all Parsed, SyntaxErrors, NewLine etc right?
Say,
case class Parsed(...) extends ParseResult with Concrete

Also I'm getting this warning, even though I am unpacking the inner object and required Silent can't inherit Silent
[warn] It would fail on the following input: Silent(_) [warn] parseResult match {

@SzymonPajzert
Copy link
Contributor

Is this feature still requested? As I understand, it wasn't implemented in the master in the end.

@Blaisorblade
Copy link
Contributor

Is this feature still requested?

Yes.

As I understand, it wasn't implemented in the master in the end.

We didn't stop liking the feature. The help wanted label means "outside contributions welcome" ;-).

@SzymonPajzert
Copy link
Contributor

I have implemented the changes, they seem to be much easier after refactoring, but I haven't added yet any tests to check the results. Where should I put them?

@Blaisorblade
Copy link
Contributor

Blaisorblade commented Aug 13, 2018

Under sbt-dotty/sbt-test/ we have tests that use sbt-scripted (https://www.scala-sbt.org/1.0/docs/Testing-sbt-plugins.html); sbt-dotty/sbt-test/sbt-dotty/example-project/test seems closest, and 061fdb1 and f503ad3 seem about it (but I'm not sure on the status, and I think @allanrenucci is on vacation).
EDIT: but while we wait, feel free to experiment with them!

@allanrenucci
Copy link
Contributor

For reference, here is where it is done for scalac.

initialCommands and cleanupCommands are not interpreted in a silent mode. If a command in initialCommands fails, the REPL exits with "Interpreter encountered errors during initialization!"

Only value binding should run in a silent mode

@SzymonPajzert
Copy link
Contributor

@allanrenucci, could you please also tell me about the status of testing of the feature? I tried to uncomment console in sbt-dotty/sbt-test/sbt-dotty/example-project/test, but received: java.lang.IllegalStateException: Unable to create a system terminal

@Blaisorblade
Copy link
Contributor

Blaisorblade commented Aug 26, 2018

@SzymonPajzert Yeah that doesn't work on CI, there's a fix at #5018. EDIT: looking at #4933, it looks like #5018 is enough to fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants