699411 - GCLI should use a sensible naming convention for its class names

Status: RESOLVED FIXED

(DevTools :: Console, defect)

Comment 1

[Joe Walker [:jwalker]]

Attachment: upload 1

Comment 2

[Joe Walker [:jwalker]]

See https://tbpl.mozilla.org/?tree=Try&rev=2a2808e08d26

Comment 3

[Joe Walker [:jwalker]]

Attachment: the full css, post patch

Comment 4

[Joe Walker [:jwalker]]

This bug and bug 692742 account for quite a bit of churn to gcli.css. It might help to look at attachment 573188 [details] to see how things end up rather than wade through the patches.

Also you can look at/comment on https://github.com/mozilla/gcli/pull/44

Comment 5

[Paul Rouget [:paul]]

Comment on attachment 573173 [details] [diff] [review]
upload 1

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

f+ if gcliVALID is replaced with "gcli-in-valid".

::: browser/devtools/webconsole/gcli.jsm
@@ +5583,4 @@
>      if (!this.element) {
>        this.elementCreated = true;
>        this.element = dom.createElement(this.document, 'div');
> +      this.element.className = 'gcli-in-complete gcliVALID';

Did you mean "gcli-in-valid"?

Comment 6

[Joe Walker [:jwalker]]

(In reply to Paul Rouget [:paul] from comment #5)
> Comment on attachment 573173 [details] [diff] [review] [diff] [details] [review]
> upload 1
> 
> Review of attachment 573173 [details] [diff] [review] [diff] [details] [review]:
> -----------------------------------------------------------------
> 
> f+ if gcliVALID is replaced with "gcli-in-valid".
> 
> ::: browser/devtools/webconsole/gcli.jsm
> @@ +5583,4 @@
> >      if (!this.element) {
> >        this.elementCreated = true;
> >        this.element = dom.createElement(this.document, 'div');
> > +      this.element.className = 'gcli-in-complete gcliVALID';
> 
> Did you mean "gcli-in-valid"?

Yes. Good spot. It's fixed here: https://github.com/joewalker/gcli/commit/74b0d8fcb4d365313ababf2730558dc79819362a
Which is part of bug 699411

Comment 7

[Joe Walker [:jwalker]]

The fix above isn't part of attachment 573173 [details] [diff] [review], but it will be part of  bug 702621.

The reason for me wanting to avoid making new patches is that I fixed a bug in dryice (our build tool) which creates some churn in the way templates are serialized. Undoing this would cause quite a bit of patch churn, so unless there is actual breakage, I'd like to delay the fixes until the churn has landed.

Comment 8

[Joe Walker [:jwalker]]

Comment on attachment 573173 [details] [diff] [review]
upload 1


Dave: See also https://github.com/mozilla/gcli/pull/44

Comment 9

[Joe Walker [:jwalker]]

Attachment: upload 2

Rebase to fix paths.

Dao: Please could you review this patch - it's taking a long time to land. Thanks.

Comment 10

[Dão Gottwald [:dao]]

Comment on attachment 575165 [details] [diff] [review]
upload 2

I have no idea what "af" and "in" mean...

Comment 11

[Joe Walker [:jwalker]]

(In reply to Dão Gottwald [:dao] from comment #10)
> Comment on attachment 575165 [details] [diff] [review] [diff] [details] [review]
> upload 2
> 
> I have no idea what "af" and "in" mean...

I think that probably doesn't worry me. If you knew that af stood for argfetch, would you be any the wiser? Either can be found easily in a search for those that want to know more.

'in' stands for input, by the way.

Comment 12

[Joe Walker [:jwalker]]

https://tbpl.mozilla.org/?tree=Try&rev=47353dce1691

Comment 13

[Joe Walker [:jwalker]]

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

Comment 14

[Rob Campbell [:rc] (:robcee)]

https://hg.mozilla.org/integration/fx-team/rev/9049e597d6ba

Comment 15

[Rob Campbell [:rc] (:robcee)]

https://hg.mozilla.org/mozilla-central/rev/9049e597d6ba