657595 - GCLI type conversion should be a type->type affair not just arg->type

Status: RESOLVED FIXED

(DevTools Graveyard :: Graphic Commandline and Toolbar, defect, P3)

Description

[Joe Walker [:jwalker]]

This will help while converting data for output display.

Comment 1

[Joe Walker [:jwalker]]

Moving GCLI bugs to Developer Tools: Console. Filter on 'baked beans are off'.

Comment 2

[Joe Walker [:jwalker]]

Triage. Filter on PEGASUS.

Comment 3

[Joe Walker [:jwalker]]

GCLI Triage.

Comment 4

[Joe Walker [:jwalker]]

Triage, filter on TEABAGS

Comment 5

[Joe Walker [:jwalker]]

New component triage. Filter on "Lobster Thermidor aux crevettes with a Mornay sauce"

Comment 6

[Joe Walker [:jwalker]]

https://github.com/MikeRatcliffe/gcli/pull/1

Comment 7

[Joe Walker [:jwalker]]

The last pull request got closed: https://github.com/MikeRatcliffe/gcli/pull/2

Comment 8

[Joe Walker [:jwalker]]

Attachment: v1

The changes to gcli.jsm should be familiar to you.

Comment 9

[Joe Walker [:jwalker]]

https://tbpl.mozilla.org/?tree=Try&rev=3e7f38b6ccfc

Comment 10

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Comment on attachment 727626 [details] [diff] [review]
v1

Review of attachment 727626 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me, r+.

::: browser/devtools/commandline/gcli.jsm
@@ +10903,5 @@
>  
>      promisedDirectTabText.resolve(directTabText);
>      promisedArrowTabText.resolve(arrowTabText);
>      promisedEmptyParameters.resolve(emptyParameters);
> +  }.bind(this), onError);

Should this not be:
}.bind(this), util.errorHandler);
?

Comment 11

[Joe Walker [:jwalker]]

(In reply to Michael Ratcliffe [:miker] [:mratcliffe] from comment #10)
> Comment on attachment 727626 [details] [diff] [review]
> v1
> 
> Review of attachment 727626 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good to me, r+.
> 
> ::: browser/devtools/commandline/gcli.jsm
> @@ +10903,5 @@
> >  
> >      promisedDirectTabText.resolve(directTabText);
> >      promisedArrowTabText.resolve(arrowTabText);
> >      promisedEmptyParameters.resolve(emptyParameters);
> > +  }.bind(this), onError);
> 
> Should this not be:
> }.bind(this), util.errorHandler);
> ?

Good spot, but in this case no. We don't want to see the error message, we want to pass it on to the promises. onError is a function to do that:

  // If anything goes wrong, we pass the error on to all the child promises
  var onError = function(ex) {
    promisedDirectTabText.reject(ex);
    promisedArrowTabText.reject(ex);
    promisedEmptyParameters.reject(ex);
  };

Thanks

Comment 12

[Joe Walker [:jwalker]]

Attachment: v2

Try breakage was down to forgetting to copy the helpers.js changes across.

Comment 13

[Joe Walker [:jwalker]]

Attachment: v3

Comment out a failing test.

That's OK we're fixing it in bug 784790 which is following this one down the landing strip.

Comment 14

[Joe Walker [:jwalker]]

https://tbpl.mozilla.org/?tree=Fx-Team&rev=f952eebd4cbd
https://hg.mozilla.org/integration/fx-team/rev/f952eebd4cbd

Comment 15

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/f952eebd4cbd