Bug 657595 #1
Closed
Bug 657595 #1
Conversation
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
Some changes that are tangentially related to this bug. i.e. not a direct part of the fix, but a helpful precursor. - Add requisition.context, and easy way to get at an ExecutionContext from a requisition - Remove the data passed to the resolve of the output object. The data exists on the output object itself, and it has proved to be confusing in a number of cases. - Commands that return View objects (it be templated) should declare a returnType of 'view' rather than 'html' - Rename outputData to output in output_terminal. It's called output everywhere else. (also fix some indentation) - Remove function names from pref.js - We don't use them anywhere else and the debugger does this for us now. Also remove some superfluous returns - Refactor the bugs command to use promises in a better way. Signed-off-by: Joe Walker <jwalker@mozilla.com>
Don't default the returnType to string for new commands, if the returnType is not specified, we can do better with a dynamic 'typeof x'. Add a typedData function to Execution context to allow commands to set the type of the output at runtime with "context.typedData(data, type)" Have Output store the type of the data using the following sources: - Dynamically set type info (i.e context.typedData) - Command meta-data (i.e. returnType in the command spec) - "typeof x" from the actual data Signed-off-by: Joe Walker <jwalker@mozilla.com>
The new 'converters.js' contains the meat of the change (and the meat of this branch) It does the job of Output.toDom and Output.toString but is configurable by adding new converters (so we expose addConverter and removeConverter to the outside world) Terminal, helpers and the server all need updating to use the new way of converting the output, and we move the link retargeting function to util. Signed-off-by: Joe Walker <jwalker@mozilla.com>
Both now separate the production of data from the conversion to a view so the Output object holds the raw data and can convert it as required. Signed-off-by: Joe Walker <jwalker@mozilla.com>
Previously we were using console.error as the handler for rejected promises, but this is broken on chrome (console functions not bound) and it doesn't show stack traces on node, so we introduce a custom error handler to fix this. Also completer was handling promises badly, by just logging on error rather than passing the errors on to the promises that it creates. So we also update the error handling there. Signed-off-by: Joe Walker <jwalker@mozilla.com>
I'm not sure if converters will need to be asynchronous in the future. It is entirely possible that some conversion might need to load some resource, or lookup some state, so I'm allowing it now since it's not many extra lines of code. Signed-off-by: Joe Walker <jwalker@mozilla.com>
intro.js was previously indented, unlike all other significant modules in gcli. Signed-off-by: Joe Walker <jwalker@mozilla.com>
Signed-off-by: Joe Walker <jwalker@mozilla.com>
Collaborator
Author
|
I'm unclear why this is closed. Going to re-open |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.