Last Comment Bug 725405 - Move the inline-output comment to the next line (not at the end of the current line)
: Move the inline-output comment to the next line (not at the end of the curren...
Status: RESOLVED FIXED
[good first bug][mentor=msucan][lang=js]
:
Product: Firefox
Classification: Client Software
Component: Developer Tools: Scratchpad (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 13
Assigned To: Spyros Livathinos
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-02-08 10:47 PST by Paul Rouget [:paul]
Modified: 2012-02-27 01:15 PST (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Proposed fix. (1.29 KB, patch)
2012-02-20 02:20 PST, Spyros Livathinos
mihai.sucan: feedback+
Details | Diff | Splinter Review
Proposed fix with fixed tests. (5.69 KB, patch)
2012-02-21 05:20 PST, Spyros Livathinos
mihai.sucan: review+
Details | Diff | Splinter Review

Description Paul Rouget [:paul] 2012-02-08 10:47:03 PST

    
Comment 1 Panos Astithas [:past] 2012-02-08 11:17:51 PST
One unexpected (for me) advantage of the current behavior is that I can sort of simulate autocomplete by typing "object.property" then Cmd-L, then Backspace, then ".property2", then Cmd-L, then Backspace, etc.
Comment 2 Paul Rouget [:paul] 2012-02-08 20:36:28 PST
(In reply to Panos Astithas [:past] from comment #1)
> One unexpected (for me) advantage of the current behavior is that I can sort
> of simulate autocomplete by typing "object.property" then Cmd-L, then
> Backspace, then ".property2", then Cmd-L, then Backspace, etc.

The "code -> ctrl-l -> backspace" thing can still work if, in the selection, the "\n" is included.
Comment 3 Panos Astithas [:past] 2012-02-09 01:44:51 PST
(In reply to Paul Rouget [:paul] from comment #2)
> (In reply to Panos Astithas [:past] from comment #1)
> > One unexpected (for me) advantage of the current behavior is that I can sort
> > of simulate autocomplete by typing "object.property" then Cmd-L, then
> > Backspace, then ".property2", then Cmd-L, then Backspace, etc.
> 
> The "code -> ctrl-l -> backspace" thing can still work if, in the selection,
> the "\n" is included.

Yes, that would be nice.
Comment 4 Spyros Livathinos 2012-02-20 02:20:57 PST
Created attachment 598803 [details] [diff] [review]
Proposed fix.

Added a newline character in front of the comment block that is generated
by the writeAsComment function of the Scratchpad editor.

The preceding newline character is being selected aswell when a user presses cmd+L, so the method that past described above still works.
Comment 5 Mihai Sucan [:msucan] 2012-02-20 12:40:56 PST
Spyros: thank you for taking this bug and submitting a patch!
Comment 6 Mihai Sucan [:msucan] 2012-02-20 12:50:52 PST
Comment on attachment 598803 [details] [diff] [review]
Proposed fix.

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

Great patch!

Now this patch doesn't need a new test, but Scratchpad has tests that are now broken by the change in writeAsComment(). Please run the Scratchpad tests and fix the failing tests in browser/devtools/scratchpad/test/. I see:

Browser Chrome Test Summary
	Passed: 221
	Failed: 21

Looking forward for the updated patch!
Comment 7 Spyros Livathinos 2012-02-21 05:20:21 PST
Created attachment 599117 [details] [diff] [review]
Proposed fix with fixed tests.

Thanks for the feedback! Fixed the two tests that were complaining due to the changes in writeAsComment() - Tests passed now: 242/242
Comment 8 Mihai Sucan [:msucan] 2012-02-22 09:53:12 PST
Comment on attachment 599117 [details] [diff] [review]
Proposed fix with fixed tests.

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

This is as good as it can get. Thanks Spyros for your contributions! Much appreciated. Keep up the good work coming! ;)
Comment 9 Rob Campbell [:rc] (:robcee) 2012-02-24 06:52:02 PST
https://hg.mozilla.org/integration/fx-team/rev/738d8ec39166
Comment 10 Tim Taubert [:ttaubert] 2012-02-27 01:15:57 PST
https://hg.mozilla.org/mozilla-central/rev/738d8ec39166

Note You need to log in before you can comment on or make changes to this bug.