Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Unexpected whitespace insertion and/or cursor placement when creating new line #5068

Closed
redmunds opened this issue Sep 5, 2013 · 8 comments
Assignees
Milestone

Comments

@redmunds
Copy link
Contributor

redmunds commented Sep 5, 2013

This behavior has caused me to have lines of code with unknown and undesired whitespace at the end. I finally realized what is happening...

  1. Find some indented code where there's an "empty" line, where "empty" line has whitespace to match the indentation of the code. For example:
        // last line of previous block of code

        // first line of next block of code
  1. Place cursor at start of "empty" line (i.e. column 0)
  2. Press Enter
  3. New line is added, cursor is moved to column 0 of new line.
  4. So far so good, now press Tab to indent code to match existing code
  5. Continue blissful coding...

Results:
After step 3, the code is already correctly indented, but cursor is placed a column 0, so there's whitespace after the cursor. Also, there is no longer whitespace on original empty line. Pressing Tab in step 5 doesn't move cursor to end of existing whitespace, it adds more whitespace!

Expected:
After step 3, I expect both lines to be indented, and cursor to be placed in column 8.

@redmunds
Copy link
Contributor Author

redmunds commented Sep 6, 2013

Looking at this again, the whitespace after the cursor in step 4 was not auto-inserted -- it's the same whitespace that was after the cursor following step 1. I guess this is why I have always tried to avoid whitespace on empty lines. Would be nice if CodeMirror was smarter, but I guess this is not a bug. Closing.

@redmunds redmunds closed this as completed Sep 6, 2013
@njx
Copy link
Contributor

njx commented Sep 6, 2013

This bites me all the time too. It's understandable why it happens, but I wonder if it's worth adding a heuristic that would eliminate the whitespace in these cases. For example, maybe in step 4, if we detect that the line is all whitespace, just delete it all before inserting the indentation whitespace.

I'm going to reopen this since I think it really is a bug, even though it's sort of "as designed". I wonder whether other text editors have special handling.

@njx njx reopened this Sep 6, 2013
@ghost ghost assigned njx Sep 16, 2013
@njx
Copy link
Contributor

njx commented Sep 16, 2013

Reviewed. Opening to me, medium priority, to see if there's a cheap solution. If not, we might want to create a user story for better whitespace handling in general (including on paste, etc.) and add this as a special case.

@njx
Copy link
Contributor

njx commented Sep 19, 2013

Nominating for sprint 32.

@pthiess
Copy link
Contributor

pthiess commented Sep 30, 2013

Moving to sprint 33

njx added a commit that referenced this issue Oct 22, 2013
@njx
Copy link
Contributor

njx commented Oct 22, 2013

There are two issues here.

(1) In CodeMirror, if you do an indent operation and the cursor is before/inside the whitespace at the beginning of the line, CM moves the cursor after the indenting whitespace, except if there was already the correct amount of whitespace on the line, in which case it leaves it alone. That seems inconsistent--it should always move the cursor to the end of the whitespace. I submitted codemirror/codemirror5#1905 to fix that.

(2) In Brackets, after asking CodeMirror to do an autoindent, if the amount of whitespace didn't change, we assume that the indentation was already correct and the user just wants to insert another tab. But that assumption doesn't seem right if the user was inside the indentation whitespace to begin with. Most likely, the user just wants to jump to the end of the indented whitespace so they can start typing. Branch nj/issue-5068 has a fix for this. In conjunction with the above CM pull request, this should produce much better behavior.

After both changes, the new behavior feels much better. Hitting tab in whitespace before content on the current line will always fix the indentation on the current line and move the cursor after the indentation whitespace. Also, since hitting return also does autoindent, if you hit return on a line before whitespace that happens to be of the correct indentation level, we'll now move the cursor to the end of that whitespace on the new line, so you'll always end up at the right place.

We can't merge this until sprint 34 since we need to take a new CM. So I'm going to remove this from the milestone for now and set it to sprint 34 when that milestone is created.

@njx
Copy link
Contributor

njx commented Oct 23, 2013

Added to sprint 34

@redmunds
Copy link
Contributor Author

redmunds commented Nov 3, 2013

Confirmed. Closing.

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

No branches or pull requests

3 participants