733783 - GCLI needs commands to support breakpoint stepping

Status: RESOLVED FIXED

(DevTools :: Console, defect, P2)

Description

[Joe Walker [:jwalker]]

See notes on bug 683503.

Comment 1

[Panos Astithas (he/him) [:past] (please ni?)]

Something to consider is emulating the relevant commands for GDB:

http://gcc.gnu.org/onlinedocs/gcc-4.1.2/gnat_ugn_unw/Introduction-to-GDB-Commands.html

We could also consider expanding the scope of the GDB emulation to add command aliases for all GDB commands supported by our debugger.

Comment 2

[Joe Walker [:jwalker]]

GCLI Triage.

Comment 3

[Victor Porof [:vporof][:vp]]

Attachment: v1

We'll need new strings for stepping because I took them from somewhere.

Comment 4

[Joe Walker [:jwalker]]

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

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

I do think we need to add some tests for these commands. We've skipped it in the past and got bitten.
It's not hard, because there is a framework to help.

Examples:
- browser/devtools/commandline/test/browser_gcli_commands.js
- browser/devtools/commandline/test/browser_gcli_edit.js

There is even documentation (which I should perhaps move to MDN):
- browser/devtools/commandline/test/head.js
  See the doc comments for DeveloperToolbarTest.checkInputStatus()
  and DeveloperToolbarTest.exec()

Thanks.

Comment 5

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

Unfortunately I need to add a breakpoint and this process is async ... so blocked by bug 773313

Comment 6

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

Attachment: Patch with working test

Works now so we should be fine to land depending on review.

Comment 7

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 641947 [details] [diff] [review]
Patch with working test

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

The only issue I found was a window leak I got when I ran the test. I believe using the registerCleanupFunction idea from the debugger mochitests would fix it.

::: browser/devtools/commandline/test/resources_dbg.html
@@ +27,5 @@
> +    function stepOverMe() {
> +      output.value = "step over";
> +    }
> +
> +    function checkTestPaused() {

Nit: this function looks unused.

Comment 8

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

Attachment: Addressed reviewers comments

http://tbpl.mozilla.org/?tree=Try&rev=9fb63e1299fc

Comment 9

[Joe Walker [:jwalker]]

https://tbpl.mozilla.org/?tree=Fx-Team&rev=6e2e2f904237

Comment 10

[Ed Morley [:emorley]]

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

Comment 11

[Francesco Lodolo [:flod]]

Commented in the wrong bug this morning.

> The skipped statements are executed, but not stepped through
Could you explain the meaning of this string?

Side note: copy&paste of l10n comments (clearly wrong, e.g. "A very short string used to describe thefunction of the dbg step over command.") is kind of worse than having no comments at all.

Comment 12

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

> > The skipped statements are executed, but not stepped through
> Could you explain the meaning of this string?

I can give it a shot ... it is easier to understand when compared with Step In.

Step In and Step Over differ in only one respect, the way they handle function calls. Either command instructs the debugger to execute the next line of code. If the line contains a function call, Step Into executes only the call itself, then halts at the first line of code inside the function. Step Over executes the entire function, then halts at the first line outside the function. Use Step Into if you want to look inside the function call. Use Step Over if you want to avoid stepping into functions.

Bug 774603 has been created so that we can fix any errors in gclicommands.properties, please log any other issues in that bug.