Closed Bug 957072 Opened 6 years ago Closed 6 years ago

Add Pretty Print to Scratchpad Menu


(DevTools Graveyard :: Scratchpad, defect)

Not set


(Not tracked)

Firefox 30


(Reporter: darkowlzz, Assigned: lviknesh)


(Whiteboard: [good-first-bug][][lang=js][engineering-friend])


(1 file, 3 obsolete files)

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] [] [lang=js]
Version: unspecified → Trunk
Whiteboard: [chrome-debug] [good-first-bug] [] [lang=js] → [good-first-bug] [] [lang=js]
I am newbie i would like to work on this Bug
To learn about building firefox and hacking on devtools, go here:

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`.

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] [] [lang=js] → [good-first-bug] [] [lang=js]
Congratulations Nick on becoming a mentor for this bug!
Attached patch patch1.patch (obsolete) — Splinter Review
"Bug 957072 Changes made-->
I added Pretty Print to the menu but got this error when i click it . 

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

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?
Attached patch patch2.patch (obsolete) — Splinter Review
I have uploaded the second patch . It works fine now .Please review it
Flags: needinfo?
Attached patch patch3.patch (obsolete) — Splinter Review
Updated the white space in 3rd patch .Please review it
Attachment #8363150 - Flags: review?(rcampbell)
Comment on attachment 8363150 [details] [diff] [review]

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"
Flags: needinfo?
Whiteboard: [good-first-bug] [] [lang=js] → [good-first-bug] [] [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]

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)
Attached patch pprint.patchSplinter Review
Rebased.  :)
Keywords: checkin-needed

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.
Keywords: checkin-needed
Whiteboard: [good-first-bug] [] [lang=js][land-in-fx-team] → [good-first-bug][][lang=js][fixed-in-fx-team]
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [good-first-bug][][lang=js][fixed-in-fx-team] → [good-first-bug][][lang=js]
Target Milestone: --- → Firefox 30
Whiteboard: [good-first-bug][][lang=js] → [good-first-bug][][lang=js][engineering-friend]
Product: Firefox → DevTools
Product: DevTools → DevTools Graveyard
You need to log in before you can comment on or make changes to this bug.