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

Status: RESOLVED FIXED

(DevTools Graveyard :: Scratchpad, defect)

Comment 1

[Panos Astithas (he/him) [:past] (please ni?)]

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

(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 (he/him) [:past] (please ni?)]

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

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

Spyros: thank you for taking this bug and submitting a patch!

Comment 6

[Mihai Sucan [:msucan]]

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]

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

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)]

https://hg.mozilla.org/integration/fx-team/rev/738d8ec39166

Comment 10

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/738d8ec39166