725382 - Add feature to sourceeditor/scratchpad: delete line(s)

Status: RESOLVED WONTFIX

(DevTools :: Source Editor, defect, P5)

Description

[Johan C]

A quick way (menuitem + keyboard shortcut?) of deleting one or several lines without having to select every bit of text on said lines could be another great addition to the sourceeditor/scratchpad.

Use case 1:
1) Place caret cursor/marker on a line
2) Use "Delete Current Line" or the the keyboard shortcut.
3) The current line is deleted, leaving no empty line.

Use case 2:
1) Select some text on several lines.
2) Use "Delete Current Lines" or the the keyboard shortcut.
3) All lines on which some text is selected are deleted.

Comment 1

[Leonard Camacho [:lcamacho]]

Attachment: Delete lines

Comment 2

[Mihai Sucan [:msucan]]

Comment on attachment 597666 [details] [diff] [review]
Delete lines

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

Great work Leonard! Thanks a lot for your contributions!

One bug: when I select an entire line, the code is confused and it thinks I selected the second line as well.

::: browser/devtools/sourceeditor/source-editor-orion.jsm
@@ +654,5 @@
>    },
>  
>    /**
> +   * Delete the lines or line that have some text selected, 
> +   * if there's no selection delete the actual line.

Please add @private to mark the method as private.

@@ +656,5 @@
>    /**
> +   * Delete the lines or line that have some text selected, 
> +   * if there's no selection delete the actual line.
> +   */
> +  _deleteLines: function SE_deleteLines()

SE__deleteLines()

@@ +662,5 @@
> +    let selection = this.getSelection();
> +    let firstLine = this._model.getLineAtOffset(selection.start);
> +    let firstLineStart = this._model.getLineStart(firstLine);
> +    let lastLine = this._model.getLineAtOffset(selection.end);
> +    let lastLineEnd = this._model.getLineEnd(lastLine);

You need to include the line end: getLineEnd(lastLine, true). Currently Ctrl-D leaves an empty line.

@@ +664,5 @@
> +    let firstLineStart = this._model.getLineStart(firstLine);
> +    let lastLine = this._model.getLineAtOffset(selection.end);
> +    let lastLineEnd = this._model.getLineEnd(lastLine);
> +
> +    this.setText('', firstLineStart, lastLineEnd);

Please use double quotes.

At the end of the function please return true. This is needed for the Orion API: actions that execute successfully need to return true.

Comment 3

[Leonard Camacho [:lcamacho]]

Attachment: Delete lines V2

Comment 4

[Mihai Sucan [:msucan]]

Comment on attachment 598546 [details] [diff] [review]
Delete lines V2

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

Thanks for the patch update! This is working well. Please go ahead and write the test!

Looking forward to see the test!

::: browser/devtools/sourceeditor/source-editor-orion.jsm
@@ +667,5 @@
> +    let firstLine = this._model.getLineAtOffset(selection.start);
> +    let firstLineStart = this._model.getLineStart(firstLine);
> +    let lastLine = this._model.getLineAtOffset(selection.end);
> +    let lastLineEnd = this._model.getLineEnd(lastLine, true);
> +    let lastLineStart = this._model.getLineStart(lastLine);

nit: lastLineStart then lastLineEnd.

@@ +670,5 @@
> +    let lastLineEnd = this._model.getLineEnd(lastLine, true);
> +    let lastLineStart = this._model.getLineStart(lastLine);
> +
> +    if (selection.start != selection.end && lastLineStart == selection.end) {
> +      lastLine -= 1;

You can put this check immediately after you get lastLine.

Comment 5

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

Moving to Source Editor component.

Filter on CHELICERAE.