This repository has been archived by the owner on Jun 28, 2022. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 89
Bug 1128988 #76
Open
joewalker
wants to merge
42
commits into
pr76-base
Choose a base branch
from
pr76-tip
base: pr76-base
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Bug 1128988 #76
Conversation
This file contains 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
In many cases there is a gcli global that is a system object. We want to make it clearer what we're working with and to get rid of the global The changes to cli.js rid us of a useless function and make it clearer how system is used inside a Requisition. Signed-off-by: Joe Walker <[email protected]>
Its only use of context is in a function into which a context is passed so we can just remove it as a member variable and all is good. Signed-off-by: Joe Walker <[email protected]>
Previously Promise was synchronous, so an optional 'async' made sense. Signed-off-by: Joe Walker <[email protected]>
Signed-off-by: Joe Walker <[email protected]>
Signed-off-by: Joe Walker <[email protected]>
If we're asked for the default connector, we used to just use the first in the list, but it makes more sense to use the last one we asked for by name. Signed-off-by: Joe Walker <[email protected]>
Nothing fancy, but given JSON.stringify it would be rude not to. Signed-off-by: Joe Walker <[email protected]>
Better comments for the existing skips and remove a skip that was no longer needed. Signed-off-by: Joe Walker <[email protected]>
If a promise rejected with null/undefined, then we previously made a bad thing worse. Now we protect ourselves. Signed-off-by: Joe Walker <[email protected]>
Signed-off-by: Joe Walker <[email protected]>
Checking that an option is hidden involves messing in properties on the value right now (which isn't a good idea really) we really should be marking that on the option, so this encapsulates the problem Signed-off-by: Joe Walker <[email protected]>
We didn't catch rejected promises, and just silently failed to reply. Also we're less noisy about logging what's going on now. Signed-off-by: Joe Walker <[email protected]>
terminal is available on the options object, so it's not needed. Signed-off-by: Joe Walker <[email protected]>
I'm not sure why we ever thought a post-creation mutation was required but it looks from the code that it once was. Not any more however. Signed-off-by: Joe Walker <[email protected]>
Delegated types are used when we're not sure a load time what the type should be. An example is the 'pref set' command where the new pref value is determined by the pref being set. The addition of remoting made this asynchronous, but we've not had time to make all the required adjustments yet. This just assumes a 'blank' type if we can't ask. Signed-off-by: Joe Walker <[email protected]>
There is no reason that a value should be JSONable, and in some cases they're not, so strip them out. Practically we shouldn't need the values on the other side of the remote interface anyway. Signed-off-by: Joe Walker <[email protected]>
Chance some places where promises were being dropped on the floor. Signed-off-by: Joe Walker <[email protected]>
Now systems are created explicitly with createSystem, and the startup sequence is less magic. Signed-off-by: Joe Walker <[email protected]>
Signed-off-by: Joe Walker <[email protected]>
Signed-off-by: Joe Walker <[email protected]>
Several commands were fairly pointless or in various stages of broken so we removed everything that wasn't good, and included everything that was. Signed-off-by: Joe Walker <[email protected]>
The various places in GCLI where we created a new system all had their own list of items in their own groups, that were mostly the same sets over again. So we broke them into the logical groups to make maintenance easier, and importing simpler. Signed-off-by: Joe Walker <[email protected]>
Signed-off-by: Joe Walker <[email protected]>
Signed-off-by: Joe Walker <[email protected]>
Signed-off-by: Joe Walker <[email protected]>
Adding a 'location' to a system makes it ignore commands that don't have a matching runAt property. This is principly for client/server setups where we import commands from the server to the client, so a system with `{ location: 'client' }` will silently ignore commands with `{ runAt: 'server' }`. Any system without a location will accept commands with any runAt property (including none). Signed-off-by: Joe Walker <[email protected]>
When an async command was run, the input element didn't get cleared until the async command had completed. This clears it as soon as return is pressed. Signed-off-by: Joe Walker <[email protected]>
Signed-off-by: Joe Walker <[email protected]>
I think GCLI should probably use protocol.js, and using a similar interface is a step in the right direction. Also it simplifies calling remote interfaces. Signed-off-by: Joe Walker <[email protected]>
It's still there in the mozmaster branch, but there's really no point in keeping it here because it doesn't work, and will never work. Signed-off-by: Joe Walker <[email protected]>
isRemote is a testing flag to allow tests to be skipped if the commands are being run on a remote system. Previously there was some hacky voodoo which (predictably) broken, so now we're explicit about it. We also line up some comments in index.html. Signed-off-by: Joe Walker <[email protected]>
- Better error reporting in server.js - Remove unused require statements in direct.js - Add missing bind statements in remoted.js - Format a comment to help WebStorm do type detection Signed-off-by: Joe Walker <[email protected]>
2 separate methods make more sense than just one, and we're better off fixing this before it is a 'published' interface in Firefox. Signed-off-by: Joe Walker <[email protected]>
There was connectors.connect, but that wasn't a great home for it, so we move it to a better place, and tidy it up a bit. Signed-off-by: Joe Walker <[email protected]>
GCLI in general should rely on a front rather than a connector which is a much lower level thing. Signed-off-by: Joe Walker <[email protected]>
When registering modules it's bad to take the nuclear option because then nothing works on a small error, so we log instead. Signed-off-by: Joe Walker <[email protected]>
See #75 Signed-off-by: Joe Walker <[email protected]>
GCLI has allowed (by turning a blind eye) to commands and other items that have additional properties. This is used by Firefox to allow commands to be placed in the toolbox and toolbar buttons with custom icons. With remote GCLI it's important that these custom properties are transferred along with the standard command metadata. This change allows the 'specs' function to take an array of custom properties which should be transferred. Signed-off-by: Joe Walker <[email protected]>
joewalker
referenced
this pull request
in joewalker/gecko-dev
Feb 18, 2015
This is an import of all the changes made in the GCLI repo. See here: https://github.com/joewalker/gcli/commits/master The changes are those from 4 Feb 2015 (d59796b) to 12 Feb 2015 (fd63021)
I thought that catch() died with end(), but apparently not. This has the downside that Eclipse/JSDT mistakes the function 'catch' for the keyword of the same name, so all parsing fails, but I don't think we should hold back code for the sake of broken editors. Signed-off-by: Joe Walker <[email protected]>
See review comment: dba9008 Signed-off-by: Joe Walker <[email protected]>
See review comment at 7a95a19 Signed-off-by: Joe Walker <[email protected]>
See review comment: c7d092b#commitcomment-9884204 Signed-off-by: Joe Walker <[email protected]>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.