This command allows the user to edit the web page, and see updates to their changes as the type. It does not attempt to save resources.

For now we're only thinking about CSS. We'll add editing of HTML in another bug, but I expect that we'll share the basic 'edit' command.

Moving conversation from email. Conversation so far: Cedric: > How do you envision the syntax of the edit command? Joe: > How about the resource type returns 'new-stylesheet' as one of it's options, > and that the edit command uses that as it's defaultValue? > That way the format of the command is very consistent > 'edit <thing to edit>'. Cedric: > Makes sense. > > Perhaps we could have a generic (additional) argument --new. > If you do: > > > edit --new foo.css > It would create a new stylesheet that is already (pre) named with name foo.css ? > > One could directly choose the full path where to save as well : > > > edit --new ~/websites/project42/css/theme-v2.css

If 'edit some-non-existant-file.css' automatically created the file, then we wouldn't need the --new would we?

(In reply to Joe Walker from comment #5) > If 'edit some-non-existant-file.css' automatically created the file, then we > wouldn't need the --new would we? That's right. And if it references an existing file:// URL, it could automatically import the file to the page (before letting the user edit it).

For what it's worth these are the strings to go with the example, which would live in /devtools/browser/locales/en-US/chrome/browser/devtools/gclicommands.properties nodeParseSyntax=Syntax error in CSS query nodeParseMultiple=Too many matches (%S) nodeParseNone=No matches editDesc=Tweak a page resource editManual=Edit one of the resources that is part of this page (or maybe any generic web resource?) editResourceDesc=URL to edit editPretend=Pretend this is an popup editor containing

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

marking as P1 as per joe. filter on pegasus

Cedric - Mike offered to take this on - I'm assuming that this is OK with you? Mike, I've written a 'resource' type which allows you to easily pick from the resources in a page. It probably makes sense to use that. I'll update this demo along with it: https://github.com/joewalker/gcli/blob/master/lib/demo/commands/experimental.js#L288

Bug 709223 brings in a resource type to help filtering css.

We also need to wait on bug 585563 as it holds the machinery needed to focus stylesheets in the style editor.

Done

Comment on attachment 592123 [details] [diff] [review] Working patch Review of attachment 592123 [details] [diff] [review]: ----------------------------------------------------------------- Thanks Mike - I like it. ::: browser/devtools/webconsole/GcliCommands.jsm @@ +132,5 @@ > + name: 'resource', > + include: 'text/css' > + }, > + description: gcli.lookup("editResourceDesc") > + } How easy would it be to add this?: { name: "line", type: { name: "number", min: 1, step: 10 }, description: "Line to Jump To" } This is probably too much but we might even do: { name: "line", type: { name: "number", min: 1, max: function() { var text = args.resource._domSheet.ownerNode.innerHTML; return text.match(/\r/g).length; } step: 10 } } @@ +137,5 @@ > + ], > + exec: function(args, context) { > + let hud = HUDService.getHudReferenceById(context.environment.hudId); > + let StyleEditor = hud.gcliterm.document.defaultView.StyleEditor; > + StyleEditor.openChrome(args.resource._domSheet); I think I should create a less hacky way to access the sheet. arg.resource.element? This would make sense for script resources too. What do you think?

Yes, that would make perfect sense

(In reply to Joe Walker from comment #14) > Comment on attachment 592123 [details] [diff] [review] > Working patch > > Review of attachment 592123 [details] [diff] [review]: > ----------------------------------------------------------------- > > Thanks Mike - I like it. > > ::: browser/devtools/webconsole/GcliCommands.jsm > @@ +132,5 @@ > > + name: 'resource', > > + include: 'text/css' > > + }, > > + description: gcli.lookup("editResourceDesc") > > + } > > How easy would it be to add this?: > > { > name: "line", > type: { > name: "number", > min: 1, > step: 10 > }, > description: "Line to Jump To" > } > Done > This is probably too much but we might even do: > > { > name: "line", > type: { > name: "number", > min: 1, > max: function() { > var text = args.resource._domSheet.ownerNode.innerHTML; > return text.match(/\r/g).length; > } > step: 10 > } > } > If only that would work with all stylesheets. Apart from args.resource not been available at this point, most stylesheets are not inline but are linked so we have no direct access to their text. > @@ +137,5 @@ > > + ], > > + exec: function(args, context) { > > + let hud = HUDService.getHudReferenceById(context.environment.hudId); > > + let StyleEditor = hud.gcliterm.document.defaultView.StyleEditor; > > + StyleEditor.openChrome(args.resource._domSheet); > > I think I should create a less hacky way to access the sheet. > arg.resource.element? > This would make sense for script resources too. > What do you think? That sounds sensible.

Comment on attachment 593378 [details] [diff] [review] Rebased according to Joe's changes to bug 709223 Review of attachment 593378 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/devtools/webconsole/GcliCommands.jsm @@ +154,5 @@ > + ], > + exec: function(args, context) { > + let hud = HUDService.getHudReferenceById(context.environment.hudId); > + let StyleEditor = hud.gcliterm.document.defaultView.StyleEditor; > + StyleEditor.openChrome(args.resource.element, args.line ? args.line - 1 : 0); This is a nit-pick, but worth mentioning: Rather than say args.line ? args.line - 1 : 0 we could add a default value to the parameter: > name: "line", > defaultValue: 1, > ... This is better for 2 reasons: GCLI won't be so insistent about getting a value, and also you won't have to check if it's set, so you can do: > StyleEditor.openChrome(args.resource.element, args.line - 1); Maybe this is too late, but shouldn't line numbers be 1-based, not 0-based?

(In reply to Joe Walker from comment #18) > Maybe this is too late, but shouldn't line numbers be 1-based, not 0-based? I believe this is a result of the SourceEditor's 0-based numbering. 1-based makes more sense to me and the debugger API is 1-based as well.

No longer index based and uses a default value of 1 by popular demand.

Comment on attachment 593919 [details] [diff] [review] [in-fx-team] Line number is now line number instead of index Bouncing request to Joe

Happy to put this in my landing queue just behind bug 709223.

Comment on attachment 593919 [details] [diff] [review] [in-fx-team] Line number is now line number instead of index Landed: https://hg.mozilla.org/integration/fx-team/rev/a6e809d5a446