Add scripting, using Cub #75
Conversation
| @@ -30,6 +30,10 @@ extension String { | |||
|
|
|||
| } | |||
|
|
|||
| let executor = CommandExecutor() | |||
|
|
|||
| var activeVC: ViewController! | |||
There was a problem hiding this comment.
Both of the above lines conflict with the Tabs change, which will introduce multiple ViewController instances, and moves executor to the TerminalView.
To get the visible view controller, there is an API on the TerminalTabViewController called visibleViewController
OpenTerm/Util/Cub.swift
Outdated
|
|
||
| DispatchQueue.main.async { | ||
|
|
||
| callback(.number(Double(code))) |
There was a problem hiding this comment.
Do we not care about the output of the command, just the return code?
There was a problem hiding this comment.
Maybe it would be better to create a class conforming to CommandExecutorCommand, then create your own TerminalExecutor and become its delegate, instead of using the visible view controller's executor.
OpenTerm/Util/Cub.swift
Outdated
|
|
||
| do { | ||
|
|
||
| activeVC.terminalView.isExecutingScript = true |
There was a problem hiding this comment.
The isExecutingScript and isWaitingForCommand should probably be put into an enum TerminalState.
Looks like they are used only to determine whether or not to write the prompt, so some refactoring would probably be able to remove the need for these anyways.
| @@ -46,6 +46,8 @@ class CommandExecutor { | |||
| /// Context from commands run by this executor | |||
| private var context = CommandExecutionContext() | |||
|
|
|||
| private var commandExecutedCallback: ((ReturnCode) -> Void)? | |||
There was a problem hiding this comment.
IMO, we should either use delegate pattern or callback pattern, not both.
#71 moves the "command finished" delegate method into the Parser class as parserDidEndTransmission, so this will conflict.
This was done because the command will return a status code, but stdout will most likely come into the pipe afterwards, then is parsed on a different thread, and finally output will be printed to the text view. Only after that should a new prompt be drawn, so the parserDidEndTransmission is called when the Parser finishes a command, which is after the command exits.
If you only care about the exit code (as you currently do), then adding a callback here would be fine, as it would be called at a different time than the Parser delegate parserDidEndTransmission, but if you want to capture output, more work will be required, and putting a callback here would not be smart.
# Conflicts: # OpenTerm.xcodeproj/project.pbxproj # OpenTerm/Controller/Scripting/ScriptEditViewController.swift # OpenTerm/Controller/ViewController.swift # OpenTerm/Util/Execution/CommandExecutor.swift # OpenTerm/View/TerminalView.swift # Podfile # Podfile.lock # Pods/Manifest.lock # Pods/Pods.xcodeproj/project.pbxproj # Pods/Target Support Files/Pods-OpenTerm/Pods-OpenTerm-frameworks.sh # Pods/Target Support Files/Pods-OpenTerm/Pods-OpenTerm.debug.xcconfig # Pods/Target Support Files/Pods-OpenTerm/Pods-OpenTerm.release.xcconfig
|
fyi: I've already added Cub and SavannaKit in master. Makes the diff from this PR a bit nicer (as well as fixing merge conflicts). |
# Conflicts: # OpenTerm.xcodeproj/project.pbxproj
|
|
||
| func didChangeText(_ syntaxTextView: SyntaxTextView) { | ||
| save() | ||
| // autoCompleteManager.reloadData() |
There was a problem hiding this comment.
I wouldn't check in the commented out lines in this file, but it's up to you.
OpenTerm/Util/Cub.swift
Outdated
|
|
||
| print("run command: \(commandStr)") | ||
|
|
||
| // executor.dispatch(commandStr, callback: { (code) in |
There was a problem hiding this comment.
What will you end up doing here for execution? Will you need to add a callback?
There was a problem hiding this comment.
I've added CubCommandExecutor, which conforms to CommandExecutorDelegate and ParserDelegate. It takes a TerminalView and temporarily sets itself as the delegates.
OpenTerm/Util/Cub.swift
Outdated
| DispatchQueue.main.async { | ||
|
|
||
| activeVC.terminalView.isExecutingScript = false | ||
| // activeVC.terminalView.isWaitingForCommand = false |
There was a problem hiding this comment.
PR #82 adds an enum state to the terminal, currently running or idle. Use that instead of adding this boolean.
# Conflicts: # OpenTerm/View/TerminalView.swift
OpenTerm/Util/Cub.swift
Outdated
| self.terminalView.stderrParser.delegate = nil | ||
| self.terminalView.stdoutParser.delegate = nil | ||
|
|
||
| self.terminalView.executor.delegate = nil |
There was a problem hiding this comment.
Not sure if it's safe to modify these delegates. This will have a conflict with PR #85, which moves the parsers down to a lower level.
|
Scripting is now stable, further work will happen on master. |
Not ready to be merged yet, but having this pull request is a nice way of detecting any merge conflicts.