Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CLOSED] [cmv4] Odd indent behavior with multiple cursors #6358

Open
core-ai-bot opened this issue Aug 30, 2021 · 22 comments
Open

[CLOSED] [cmv4] Odd indent behavior with multiple cursors #6358

core-ai-bot opened this issue Aug 30, 2021 · 22 comments

Comments

@core-ai-bot
Copy link
Member

Issue by dangoor
Thursday Feb 27, 2014 at 18:32 GMT
Originally opened as adobe/brackets#7022


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.

@core-ai-bot
Copy link
Member Author

Comment by njx
Thursday Feb 27, 2014 at 22:29 GMT


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.

@core-ai-bot
Copy link
Member Author

Comment by dangoor
Friday Feb 28, 2014 at 02:43 GMT


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.

@core-ai-bot
Copy link
Member Author

Comment by njx
Thursday Mar 06, 2014 at 19:29 GMT


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

@core-ai-bot
Copy link
Member Author

Comment by njx
Thursday Mar 06, 2014 at 23:47 GMT


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.

@core-ai-bot
Copy link
Member Author

Comment by njx
Friday Mar 07, 2014 at 00:14 GMT


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

@core-ai-bot
Copy link
Member Author

Comment by njx
Friday Mar 07, 2014 at 23:27 GMT


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?

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Saturday Mar 08, 2014 at 00:11 GMT


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

@core-ai-bot
Copy link
Member Author

Comment by njx
Saturday Mar 08, 2014 at 01:03 GMT


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

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Saturday Mar 08, 2014 at 01:33 GMT


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

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Tuesday Mar 11, 2014 at 01:19 GMT


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.

@core-ai-bot
Copy link
Member Author

Comment by njx
Tuesday Mar 11, 2014 at 01:28 GMT


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

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Tuesday Mar 11, 2014 at 01:59 GMT


(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).

@core-ai-bot
Copy link
Member Author

Comment by njx
Tuesday Mar 11, 2014 at 02:44 GMT


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.

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Tuesday Mar 11, 2014 at 05:58 GMT


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?

@core-ai-bot
Copy link
Member Author

Comment by dangoor
Tuesday Mar 11, 2014 at 13:12 GMT


+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.

@core-ai-bot
Copy link
Member Author

Comment by njx
Tuesday Mar 11, 2014 at 17:02 GMT


@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?

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Tuesday Mar 11, 2014 at 17:34 GMT


@njx I'm ok with that.

@core-ai-bot
Copy link
Member Author

Comment by dangoor
Tuesday Mar 11, 2014 at 17:42 GMT


@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...

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Tuesday Mar 11, 2014 at 21:12 GMT


FBNC to@dangoor

@core-ai-bot
Copy link
Member Author

Comment by njx
Tuesday Mar 11, 2014 at 22:09 GMT


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.

@core-ai-bot
Copy link
Member Author

Comment by njx
Wednesday Mar 12, 2014 at 01:08 GMT


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

@core-ai-bot
Copy link
Member Author

Comment by dangoor
Thursday Mar 13, 2014 at 15:13 GMT


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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant