define $_ to mean the previous command result

RESOLVED FIXED in Firefox 39

Status

defect
P3
normal
RESOLVED FIXED
7 years ago
Last year

People

(Reporter: dangoor, Assigned: bgrins, Mentored)

Tracking

(Blocks 1 bug)

Trunk
Firefox 39
Dependency tree / graph

Firefox Tracking Flags

(firefox39 fixed)

Details

(Whiteboard: [console-papercuts])

Attachments

(1 attachment, 5 obsolete attachments)

WebKit has added a handy shortcut, $_, that refers to the last expression result. (For example, type 2+2<enter>, then $_<enter> and the console will display 4 for each)
Cool idea!
OS: Mac OS X → All
Priority: -- → P3
Hardware: x86 → All
Version: 17 Branch → Trunk
Needs testing and before that a check if I made the right changes.
Flags: needinfo?
ConsoleAPIListener is the constructor of the object, owner is null. This patch is incomplete. Please test your patch before submitting feedback requests.
Flags: needinfo?
Posted patch Reworked patch 2 (obsolete) — Splinter Review
Flags: needinfo?(mihai.sucan)
Thanks for your patch, David. Next time please ask for feedback on the patch, not needinfo. Also, please mark old patches as obsolete when you add a new one.

The approach is not what we have in mind: you store the last window.console API method call, instead of storing the last result of the JavaScript input evaluation.

Summary how to fix this bug: look at toolkit/devtools/server/actors/webconsole.js search for WCA_onEvaluateJS(). Store the last eval result. Add the $_ jsterm helper.

Do note this is not a good first bug, however we are glad to take a working patch that meets the requirements. Please take the time to read the code, online docs, to understand what is happening. We will try to help when we have some available time. Thank you!
Flags: needinfo?(mihai.sucan)
Attachment #825411 - Attachment is obsolete: true
Attachment #825419 - Attachment description: Reworked patch 3 (forgot to remove one line) → Adding $_ to console helper functions - Reworked patch 3 (forgot to remove one line)
Attachment #825354 - Attachment is obsolete: true
Okay, thanks for your info, help and support (yes, it's one of my first bugs/tickets I try to solve here).

Should I save the last input evaluation (webconsole.js -> line 676 or whole return object) in WebConsoleActor.prototype?
Then I could get the evaluation via ConsoleAPIListener.prototype.owner (-> utils.js) and the method added by me in WebConsoleActor.prototype? Is that right how I could achieve it?

Thanks!
(In reply to David from comment #7)
> Okay, thanks for your info, help and support (yes, it's one of my first
> bugs/tickets I try to solve here).
> 
> Should I save the last input evaluation (webconsole.js -> line 676 or whole
> return object) in WebConsoleActor.prototype?

Save only the |result| object on the WCA prototype, yes. Not the whole return object.

> Then I could get the evaluation via ConsoleAPIListener.prototype.owner (->
> utils.js) and the method added by me in WebConsoleActor.prototype? Is that
> right how I could achieve it?

You do not need to involve the ConsoleAPIListener at all.

The $_ getter can use aOwner to access any property from WebConsoleActor, so you can get the last result object directly, and return it.
Attachment #825419 - Attachment is obsolete: true
Is this correct now?
If yes, should I also add tests?
Where could I find the file to add such tests?
(DXR Code finder doesn't allow to search for strings with just 2 chars since I though I could find the test file(s) that way.)
Mentor: past
Duplicate of this bug: 1026658
This seems to have slipped through the cracks. David if you want to ask for feedback please set the flag on the patch as Mihai suggested in comment 6. Without looking at the patch I can safely say that a test would be required for this. For general instructions on running the tests read this:

https://wiki.mozilla.org/DevTools/Hacking#DevTools_Mochitests

You should be able to add your tests to this file, which already deals with other command line helpers:

http://dxr.mozilla.org/mozilla-central/source/browser/devtools/webconsole/test/browser_webconsole_jsterm.js

You can run it with:

./mach test browser/devtools/webconsole/test/browser_webconsole_jsterm.js

Let us know if you need any help!
Whiteboard: [console-papercuts]
Is anyone still working on this? What's left?
Flags: needinfo?(past)
Nobody is active on this bug currently, so feel free to pick it up. See comment 13 for details on writing tests.
Flags: needinfo?(past)
FWIW Safari is using $1 - $n to represent the last $n executed results (as can be seen on this page: http://timothy.hatcher.name/console/).  This could be very useful, although it requires some frontend changes to show which variable each execution was assigned to.

I think starting with $_ would be great, and would love to see this land.
So I tried to apply the patches ( attachment 826303 [details] [diff] [review] and attachment 826304 [details] [diff] [review] ) to my queue. It looks like
> devtools/webconsole/utils.js 
> devtools/server/actors/webconsole.js
no longer exist. They've probably moved to
> toolkit/devtools/webconsole/utils.js
> toolkit/devtools/server/actors/webconsole.js
So the patches would definitely not apply.
I could go ahead and write the tests, but have no way of testing them. So shall I write the test nonetheless? Or do you want me to rebase the earlier patches?
Flags: needinfo?(past)
Yes, please rebase the old patches first.
Flags: needinfo?(past)
Includes test
Assignee: nobody → bgrinstead
Attachment #826303 - Attachment is obsolete: true
Attachment #826304 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8584165 - Flags: review?(past)
Comment on attachment 8584165 [details] [diff] [review]
dollar-underscore.patch

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

Nit: initialize _lastConsoleInputEvaluation in the WebConsoleActor.prototype to avoid monkey patching later which could make the object get deoptimized by the JIT.

We also want this in Valence, so if you feel like adding it there, go for it! Otherwise I'll get to it at some point.
Attachment #8584165 - Flags: review?(past) → review+
https://hg.mozilla.org/integration/fx-team/rev/697c11bd51f6
Whiteboard: [console-papercuts] → [fixed-in-fx-team][console-papercuts]
https://hg.mozilla.org/mozilla-central/rev/697c11bd51f6
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team][console-papercuts] → [console-papercuts]
Target Milestone: --- → Firefox 39
(In reply to Brian Grinstead [:bgrins] from comment #19)
> Created attachment 8584165 [details] [diff] [review]
> dollar-underscore.patch
> 
> Includes test

Awesome, thanks for picking this up.
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.