Closed
Bug 792063
Opened 12 years ago
Closed 10 years ago
define $_ to mean the previous command result
Categories
(DevTools :: Console, defect, P3)
DevTools
Console
Tracking
(firefox39 fixed)
RESOLVED
FIXED
Firefox 39
Tracking | Status | |
---|---|---|
firefox39 | --- | fixed |
People
(Reporter: dangoor, Assigned: bgrins, Mentored)
References
(Blocks 1 open bug)
Details
(Whiteboard: [console-papercuts])
Attachments
(1 file, 5 obsolete files)
8.50 KB,
patch
|
past
:
review+
|
Details | Diff | Splinter Review |
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)
Comment 1•12 years ago
|
||
Cool idea!
Updated•11 years ago
|
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?
Comment 3•11 years ago
|
||
ConsoleAPIListener is the constructor of the object, owner is null. This patch is incomplete. Please test your patch before submitting feedback requests.
Flags: needinfo?
Comment 6•11 years ago
|
||
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!
Comment 8•11 years ago
|
||
(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
Comment 10•11 years ago
|
||
Comment 11•11 years ago
|
||
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.)
Updated•10 years ago
|
Mentor: past
Comment 13•10 years ago
|
||
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!
Updated•10 years ago
|
Whiteboard: [console-papercuts]
Comment 15•10 years ago
|
||
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)
Assignee | ||
Comment 16•10 years ago
|
||
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.
Comment 17•10 years ago
|
||
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)
Assignee | ||
Comment 19•10 years ago
|
||
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 20•10 years ago
|
||
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+
Assignee | ||
Comment 21•10 years ago
|
||
Assignee | ||
Comment 22•10 years ago
|
||
Whiteboard: [console-papercuts] → [fixed-in-fx-team][console-papercuts]
Comment 23•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-firefox39:
--- → fixed
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team][console-papercuts] → [console-papercuts]
Target Milestone: --- → Firefox 39
Comment 24•10 years ago
|
||
(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.
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•