Merged
Conversation
737f870 to
1d74cc0
Compare
It doesn't need to be an active model. A plain PORO can do here. Plus, a lot of cruft has been removed for both REPL and REPLSession. Also, rename REPL to better match its intent. A REPL didn't wrap an external REPL and wasn't one either. It just evaluated code and formatted its output. I think Evaluator fits its purpose better. REPLSession got out of date after the REPL rename also. Renamed it to simply Session. As a bonus, I have tried to document them a bit better.
1d74cc0 to
a8abee4
Compare
gsamokovarov
added a commit
that referenced
this pull request
Jan 2, 2015
Rework REPL and REPLSession
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
REPLandREPLSessiondoesn't hold that much for the current requirements. They have unused methods in their interfaces and do work we no longer need.REPLSessiondoesn't need to be an active model. It has a lot of cruft – methods that used to be used in web-console 0.2 are unused now.It also has a user facing problem. When a session is no longer available in memory, it is raising an exception that wasn't handled anywhere in the current code. This leaves the console seemingly hanging and raises an exception that can be seen in the server log. We can show a message to the user of the console instead. I have went with this route.
REPLreceived an overhaul as well. The prompt used to be dynamic, however now its not. We can just let the UI decide what to show and don't care in the server code.After changing
REPLandREPLSessionbehaviours, I renamed them to fit their intentions better.REPLdidn't wrap an externalREPLand wasn't one either. It just evaluated code and formatted its output. I thinkEvaluatorfits its purpose better.REPLSessiongot renamed it to simplySession.I also added tests along the way for the code evaluation and binding switching behaviour.
Happy new year, folks! 🎆