Add Pretty Print to Scratchpad Menu

VERIFIED FIXED in Firefox 30

Status

()

Firefox
Developer Tools: Scratchpad
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: darkowlzz, Assigned: vikneshwar)

Tracking

Trunk
Firefox 30
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good-first-bug][mentor=fitzgen@mozilla.com][lang=js][engineering-friend])

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

4 years ago
Pretty Print is available only by clicking the button in Scratchpad.
It should also be available as a menu item under some menu with shortkey visible.
Hardware: x86 → All
Whiteboard: [chrome-debug] [good-first-bug] [mentor=bbenvie@mozilla.com] [lang=js]
Version: unspecified → Trunk
Whiteboard: [chrome-debug] [good-first-bug] [mentor=bbenvie@mozilla.com] [lang=js] → [good-first-bug] [mentor=bbenvie@mozilla.com] [lang=js]
(Assignee)

Comment 1

4 years ago
I am newbie i would like to work on this Bug
To learn about building firefox and hacking on devtools, go here: https://wiki.mozilla.org/DevTools/Hacking

Benvie can give you more of the specifics for this bug.
Assignee: nobody → lviknesh
(In reply to Sunny [:darkowlzz] from comment #0)
> Pretty Print is available only by clicking the button in Scratchpad

and Ctrl+P (Windows/Linux) or Command+P (Mac)

@benvie Shouldn't be [lang=xul]?
You'r(In reply to Zulkarnain K. [:LouCypher] from comment #3)
> @benvie Shouldn't be [lang=xul]?

It's half and half. Adding the menu is a XUL change, but updating the test is JS.


vikneshwar, you'll need modify a couple files [1][2]. In the .xul file there's already a <command> and <key> for pretty print (search "pprint"), you'll need to add a new menuitem. In the .js file (the test) you just need to add a new item to `methodsAndItems`.

[1] http://mxr.mozilla.org/mozilla-central/source/browser/devtools/scratchpad/scratchpad.xul
[2] http://mxr.mozilla.org/mozilla-central/source/browser/devtools/scratchpad/test/browser_scratchpad_ui.js
Can I have this bug assigned to me? I am willing to work on this bug.
what is the menu item are we supposed to add in the xul file?
what is the menu item are we supposed to add in the xul file?
hi Anup!

Nick should be able to help you out.

have fun!
Whiteboard: [good-first-bug] [mentor=bbenvie@mozilla.com] [lang=js] → [good-first-bug] [mentor=fitzgen@mozilla.com] [lang=js]
Congratulations Nick on becoming a mentor for this bug!
(Assignee)

Comment 10

4 years ago
Created attachment 8363054 [details] [diff] [review]
patch1.patch

"Bug 957072 Changes made-->  http://pastebin.com/EcUCW9CN.
I added Pretty Print to the menu but got this error http://pastebin.com/tWpjgm2q when i click it . 

Please check the patch and help me fix the error .
Flags: needinfo?
Comment on attachment 8363054 [details] [diff] [review]
patch1.patch

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

just a couple of minor nits.

::: browser/devtools/scratchpad/scratchpad.xul
@@ +116,5 @@
>    <key id="sp-key-errorConsole"
>         key="&errorConsoleCmd.commandkey;"
>         command="sp-cmd-errorConsole"
>         modifiers="accel,shift"/>
> +  <key id="sp-key-hideSidebar" keycode="VK_ESCAPE"

no need to move this.

::: browser/locales/en-US/chrome/browser/devtools/scratchpad.dtd
@@ +66,5 @@
>  <!ENTITY display.key                  "l">
>  
>  <!ENTITY pprint.label                 "Pretty Print">
>  <!ENTITY pprint.key                   "p">
> +<!ENTITY pprint.accesskey             "P">

should have a blank line between this line and the next.
Attachment #8363054 - Flags: feedback+
Flags: needinfo?
(Assignee)

Comment 12

4 years ago
Created attachment 8363081 [details] [diff] [review]
patch2.patch

I have uploaded the second patch . It works fine now .Please review it
Flags: needinfo?
(Assignee)

Comment 13

4 years ago
Created attachment 8363150 [details] [diff] [review]
patch3.patch

Updated the white space in 3rd patch .Please review it
(Assignee)

Updated

4 years ago
Attachment #8363150 - Flags: review?(rcampbell)
Comment on attachment 8363150 [details] [diff] [review]
patch3.patch

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

looks good!
Attachment #8363150 - Flags: review?(rcampbell)
Attachment #8363150 - Flags: review+
Attachment #8363150 - Flags: checkin?
just a note about patch formatting, typically we'll set the status message to include the Bug # and the description of what the patch does. Ultimately, with the name of the reviewer. makes it easier to checkin.

e.g., hg qref -m "Bug 957072 - Add Pretty Print to Scratchpad Menu; r=robcee"
Status: NEW → ASSIGNED
Flags: needinfo?
Whiteboard: [good-first-bug] [mentor=fitzgen@mozilla.com] [lang=js] → [good-first-bug] [mentor=fitzgen@mozilla.com] [lang=js][land-in-fx-team]
Attachment #8363081 - Attachment is obsolete: true
Attachment #8363054 - Attachment is obsolete: true
Comment on attachment 8363150 [details] [diff] [review]
patch3.patch

Doesn't apply cleanly to fx-team. Please rebase. Also, please just use the checkin-needed bug keyword. It's easier to use with our automated tools.
Attachment #8363150 - Flags: checkin?
Hi! Can you please rebase this so we can land it? Thank you ;)
Flags: needinfo?(lviknesh)
Flags: needinfo?(lviknesh)
(Assignee)

Comment 18

4 years ago
Created attachment 8372823 [details] [diff] [review]
pprint.patch

Rebased.  :)
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
Attachment #8363150 - Attachment is obsolete: true
https://hg.mozilla.org/integration/fx-team/rev/cd69b4b6d544

You really don't need the {land-in-fx-team] on the whiteboard. Also, please note that your commit message should be saying what your patch is doing overall, not what you changed since the last time around.
https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F
Keywords: checkin-needed
Whiteboard: [good-first-bug] [mentor=fitzgen@mozilla.com] [lang=js][land-in-fx-team] → [good-first-bug][mentor=fitzgen@mozilla.com][lang=js][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/cd69b4b6d544
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [good-first-bug][mentor=fitzgen@mozilla.com][lang=js][fixed-in-fx-team] → [good-first-bug][mentor=fitzgen@mozilla.com][lang=js]
Target Milestone: --- → Firefox 30
(Reporter)

Updated

4 years ago
Whiteboard: [good-first-bug][mentor=fitzgen@mozilla.com][lang=js] → [good-first-bug][mentor=fitzgen@mozilla.com][lang=js][engineering-friend]

Updated

4 years ago
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.