[cmv4] Odd indent behavior with multiple cursors

[dangoor]

Perhaps this is to be expected, but it's not what I expected.

To reproduce:

1. Have at least one cursor in the middle of a line
2. Add a cursor at the beginning of the line
3. Press tab
4. Four spaces added appropriately in the middles of the lines and at the beginning of a line
5. Press the left arrow key
6. All of the cursors are now back where they were with the 4 spaces following
7. Press tab
8. The two middle of the line cursors have no change, the cursor at the beginning of a line has the indentation removed

The beginning of the line behavior is consistent with pressing the tab key in the single select case. The middle of the line behavior is different because in the single select case another four spaces would be added.

[njx]

I think in order to reproduce your steps, you have to do step 2 first and then step 1 (i.e., first cursor is at the left edge, second cursor is in the middle). The issue is that our indent code is only really looking at the primary selection in order to figure out what to do. There's a similar issue around soft tabs, which is partly related: #7002.

I agree that this is pretty low priority since it's probably not a really common case, but while fixing #7002 I can see if there's an obvious fix for this as well.

[dangoor]

I definitely did it in the order listed... I had 2 cursors in the middles of lines and then added a 3rd at the beginning of another line.

If this proves to be non-trivial to fix, I think we just close it because it is such an edge case.

[njx]

Oops, forgot to look at this with #7002 :) I'll look at it now.

[njx]

Oh, I know why your case is different...the line that you added the cursor at the beginning of must have had no indentation to begin with.

[njx]

This is generally tricky for the same reason that #7002 is tricky; the CM commands we currently use to indent operate on all the selections, not on an individual selection. However, for some cases where it makes sense, we could do the individual operations ourselves.

I'm thinking that (similar to #7002) it's better to generally apply the same operation to all selections anyway, rather than try to treat each selection individually, because it will be less confusing. So, my proposal is:

1. if any of the selections are multiline, just add one indent level to the beginning of all lines that intersect any selection
2. otherwise, if any of the selections is after the first non-whitespace character in a line:
   • if indentation is set to tabs, just insert a hard tab at each selection
   • if indentation is set to spaces, insert the appropriate number of spaces at each selection to get it to its next soft tab stop
3. otherwise (all selections are cursors or single-line, and are in the whitespace before their respective lines), try to autoindent; if none of the cursors moved, then add one indent level to the beginning of all lines

[njx]

I'm thinking of modifying this slightly from the existing behavior. Currently, if you have a multi-line range selected, we add an indent level to the beginning of each line. However, if you have a range within a single line selected, and it's in the middle of the content, we replace the range with a tab (or spaces). That behavior has always felt a little weird to me (and the comments in Editor._handleTab() suggest that it should only be doing that if the selection is a cursor rather than a range, though they don't specify what the desired behavior is in the range case). I'm tempted to make it so that it inserts a tab/spaces at the beginning of the selection rather than replacing it. (In other words, we would treat it like case 2 above, but with the insertion being at the beginning of the range.)

@dangoor / @redmunds - any thoughts on this case?

[redmunds]

I've always expected that behavior. I think it's more consistent to always indent (as opposed to replace in some cases).

[njx]

@redmunds Ah ok - just to be clear, you mean that you would expect my proposed new behavior?

[redmunds]

@njx Yes, I have always expected your new proposed behavior.

[redmunds]

If I select a range in a single line and hit Tab, then an indent is inserted before the range, which is expected. But, if I hit Shift-Tab, then indent is removed from beginning of line -- I expected it to be removed from immediately before the range (same place it's inserted). This is useful when using Alt-drag to select a box of text, and then use Tab/Shift-Tab to move it to the desired indent level within some other text.

[njx]

That makes sense, though it's new behavior. Does this sound reasonable for the behavior of Shift-Tab if a range is selected in the middle of a line:

1. if using tabs for indentation:
   (a) if there is a tab immediately before the selection, delete it
   (b) otherwise do the standard Shift-Tab behavior (unindent whole line)
2. if using spaces for indentation with soft tabs:
   (a) if there are spaces before the range, delete backwards to the next virtual tab stop, or to the content, whichever is closer
   (b) otherwise do the standard Shift-Tab behavior (unindent whole line)

2a is kind of like our original soft tab behavior for backspace in the middle of the line (which we removed). One issue is what we should do if there are multiple selections - do we look at each one individually and handle it separately (so you might get different deletion amounts for each selection), or do we find the minimal deletion across all selections? It gets kind of complicated (which is one reason I wanted to get rid of it originally :))

[redmunds]

(Oops -- my comment was supposed to be in PR).

This only applies to a selection which does not contain a newline. Original comment was only about a range in a line, but may also apply to a cursor.

I, personally, think Shift-Tab should only remove indents between selection and previous non-whitespace char. This means stop when there are no more indents to remove, and don't remove indents from beginning of line. I don't like the idea of Shift-Tab removing indents from beginning of line when selection is in middle of line because Tab will never insert an indent at beginning of line (when selection is within line).

For multi-selection with different amounts of whitespace before each selection, I think each selection should be handled separately. This is how it works if you select a bunch of lines of text with different indention -- you keep hitting Shift-Tab and eventually everything will be left-aligned (each line is handed separately).

[njx]

That makes sense (as long as the range is not in the whitespace at the beginning of a line). It was a little weird for Delete to do different things on each selection, but I think it's less weird for Shift-Tab.

If you're using spaces, and the space before one of the selections is less than a tab stop, should it just delete up to the previous content? That sounds right, although I bet in most cases you want to keep one space there...but then you can just type the space afterwards.

[peterflynn]

Are the above statements intended to apply for the degenerate case of a cursor, or just to a selected range? For cursors, I think it's fairly common to have the cursor in the midst of the line's text when you hit Shift+Tab to unindent. Having it try to remove mid-line whitespace and no-op failing that would feel like a notable regression, IMO. But if we don't do that, will it fell weird for the behavior to differ for a cursor vs. a range?

[dangoor]

+1 to what @peterflynn said. I'll add that since we changed cmd-[ and cmd-] to not be indent/outdent (a key combo I didn't realize my brain was programmed to use until we changed it), there would no longer be a way to change the indentation of a line when you're in the middle of the line.

[njx]

@peterflynn Good point. Also, if we make it work on cursors, but people are still expecting the Shift-Tab-to-unindent-line behavior, then they'll get weird cases where if they happen to be after a single space, it will delete the space instead of unindenting.

I'm tempted to say that we shouldn't change the Shift-Tab behavior now, since (even before these changes) Tab would insert a tab in the middle of a line and Shift-Tab wouldn't delete it, so it's not a new "inconsistency". @redmunds are you ok with that?

@dangoor - not sure what you mean; Cmd-[ and Cmd-] still seem to be indent/outdent in my build...maybe you have an extension that's conflicting?

[redmunds]

@njx I'm ok with that.

[dangoor]

@njx Oh, you're right. I didn't realize I had an extension doing that. That's definitely an argument for Brackets to know which things came from which extensions...

[redmunds]

FBNC to @dangoor

[njx]

Oops, I found a regression with my changes: if the cursor is at the beginning of a blank line, we used to autoindent to the correct level, but now we just insert a single indent. PR coming shortly.

[njx]

OK, that last issue is fixed, so should be ready to FBNC again.

[dangoor]

OK, the behavior is consistent now between the cursors in the middle of the line and the cursors at the beginning of the line.