Closed Bug 725405 Opened 10 years ago Closed 10 years ago

Move the inline-output comment to the next line (not at the end of the current line)

Categories

(DevTools Graveyard :: Scratchpad, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 13

People

(Reporter: paul, Assigned: livathinos.spyros)

Details

(Whiteboard: [good first bug][mentor=msucan][lang=js])

Attachments

(1 file, 1 obsolete file)

No description provided.
Hardware: x86 → All
Whiteboard: [good first bug][mentor=msucan][lang=js]
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.
(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.
(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.
Attached patch Proposed fix. (obsolete) — Splinter Review
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.
Attachment #598803 - Flags: feedback?(mihai.sucan)
Spyros: thank you for taking this bug and submitting a patch!
Assignee: nobody → livathinos.spyros
Status: NEW → ASSIGNED
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!
Attachment #598803 - Flags: feedback?(mihai.sucan) → feedback+
Thanks for the feedback! Fixed the two tests that were complaining due to the changes in writeAsComment() - Tests passed now: 242/242
Attachment #598803 - Attachment is obsolete: true
Attachment #599117 - Flags: review?(mihai.sucan)
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! ;)
Attachment #599117 - Flags: review?(mihai.sucan) → review+
Whiteboard: [good first bug][mentor=msucan][lang=js] → [good first bug][mentor=msucan][lang=js][land-in-fx-team]
https://hg.mozilla.org/integration/fx-team/rev/738d8ec39166
Whiteboard: [good first bug][mentor=msucan][lang=js][land-in-fx-team] → [good first bug][mentor=msucan][lang=js][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/738d8ec39166
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [good first bug][mentor=msucan][lang=js][fixed-in-fx-team] → [good first bug][mentor=msucan][lang=js]
Target Milestone: --- → Firefox 13
Product: Firefox → DevTools
Product: DevTools → DevTools Graveyard
You need to log in before you can comment on or make changes to this bug.