957072 - Add Pretty Print to Scratchpad Menu

Status: VERIFIED FIXED

(DevTools Graveyard :: Scratchpad, defect)

Description

[Sunny [:darkowlzz]]

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.

Comment 1

[vikneshwar]

I am newbie i would like to work on this Bug

Comment 2

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

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.

Comment 3

[Zulkarnain K. [:LouCypher]]

(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]?

Comment 4

[Brandon Benvie [:benvie]]

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

Comment 5

[Anup Kumar [:reznord]]

Can I have this bug assigned to me? I am willing to work on this bug.

Comment 6

[Anup Kumar [:reznord]]

what is the menu item are we supposed to add in the xul file?

Comment 7

[Anup Kumar [:reznord]]

what is the menu item are we supposed to add in the xul file?

Comment 8

[Rob Campbell [:rc] (:robcee)]

hi Anup!

Nick should be able to help you out.

have fun!

Comment 9

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

Congratulations Nick on becoming a mentor for this bug!

Comment 10

[vikneshwar]

Attachment: 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 .

Comment 11

[Rob Campbell [:rc] (:robcee)]

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.

Comment 12

[vikneshwar]

Attachment: patch2.patch

I have uploaded the second patch . It works fine now .Please review it

Comment 13

[vikneshwar]

Attachment: patch3.patch

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

Comment 14

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 8363150 [details] [diff] [review]
patch3.patch

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

looks good!

Comment 15

[Rob Campbell [:rc] (:robcee)]

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"

Comment 16

[Ryan VanderMeulen [:RyanVM]]

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.

Comment 17

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

Hi! Can you please rebase this so we can land it? Thank you ;)

Comment 18

[vikneshwar]

Attachment: pprint.patch

Rebased.  :)

Comment 19

[Ryan VanderMeulen [:RyanVM]]

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

Comment 20

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

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