-
Notifications
You must be signed in to change notification settings - Fork 369
#427 Add an ":exit" command to the REPL #823
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
Conversation
Added new IRepl interface that inherits from IScriptExecutor and adds a new method named Quit Updated the Execute method of IRepl to accept an instance of IRepl instead of IScriptExecutor Updated Repl to implement IRepl and the new Quit method, which calls Terminate() and Environment.Exit(0) Updated the Execute() method of the existing REPL commands to accept an instance of IRepl instead of IScriptExecutor Added a new ExitCommand that uses the new Repl.Quit() method to exit the REPL Updated RuntimeServices to bind IRepl to Repl Updated existing REPL command tests to pass in a mock of IRepl to IReplCommand.Execute instead of a mock of IScriptExecutor Added new tests to cover the new ExitCommand
Removing "Contracts" from "Contracts.IRepl" from updated tests
The PromptsUserBeforeExiting() unit test was eternal looping since the Console.ReadLine() method was never stubbed to return a value
| public void Quit() | ||
| { | ||
| Terminate(); | ||
| Environment.Exit(0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Environment.Exit should be moved into the IConsole abstraction - there is no guarantee which console is in use, so exiting the REPL might be different based on those
|
very good PR, thanks a lot. Creating Changing the signature on I also left some comments inline. |
|
Hi Filip, thank you for taking the time to look at my pull request. I have a question about #434, by "inject" do you mean move the construction of the Repl object to CommandFactory and pass it to the ExecuteReplCommand constructor as a parameter? Also, would it be appropriate to have an IRepl.Quit() method that would proxy to the IConsole.Quit() method? |
|
sorry I was sure I replied earlier! |
|
@munumafia really sorry for leaving this out there for that long. If you notice, we have merged a bunch of changes related to REPL infrastructure and the commands themselves. If you are still up for the issue, can you please rebase? thanks |
|
superseded by #959. @munumafia I rebased your branch so that your commits show up in the history |
Added new IRepl interface that inherits from IScriptExecutor and adds a new method named Quit
Updated the Execute method of IRepl to accept an instance of IRepl instead of IScriptExecutor
Updated Repl to implement IRepl and the new Quit method, which calls Terminate() and Environment.Exit(0)
Updated the Execute() method of the existing REPL commands to accept an instance of IRepl instead of IScriptExecutor
Added a new ExitCommand that uses the new Repl.Quit() method to exit the REPL
Updated RuntimeServices to bind IRepl to Repl
Updated existing REPL command tests to pass in a mock of IRepl to IReplCommand.Execute instead of a mock of IScriptExecutor
Added new tests to cover the new ExitCommand