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

Move line up drops a line #29457

Closed
jrieken opened this issue Jun 26, 2017 · 4 comments
Closed

Move line up drops a line #29457

jrieken opened this issue Jun 26, 2017 · 4 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug editor-contrib Editor collection of extras important Issue identified as high-priority verified Verification succeeded
Milestone

Comments

@jrieken
Copy link
Member

jrieken commented Jun 26, 2017

See attached recording and sample snippets

  • select the whitespaceCharacter-array with an empty selection on its last liine
  • move up into be new class
  • 💥 the last line gets lost half way
const regexpOracle = new class {

}

function regexpMatchMightContainWhitespace(regexp: RegExp): boolean {
	const { source } = regexp;

	const whitespaceCharacters = [
		'.', '\\s', ' ',
		'\f', '\n', '\r', '\t', '\v',
		'\u00a0', '\u1680', '\u180e', '\u2000', '\u2001', '\u2002', '\u2003', '\u2004', '\u2005', '\u2006', '\u2007', '\u2008', '\u2009', '\u200a', '\u2028', '\u2029', '\u202f', '\u205f', '\u3000', '\ufeff'
	];



	let idx = source.indexOf('.');
	if (idx >= 0 && source[idx - 1] !== '\\') {
		return true;
	}

}

jun-26-2017 15-40-37

@vscodebot vscodebot bot added the bug Issue identified by VS Code Team member as probable bug label Jun 26, 2017
@jrieken jrieken added the important Issue identified as high-priority label Jun 26, 2017
@alexdima alexdima removed their assignment Jun 26, 2017
@rebornix
Copy link
Member

It's not a new issue actually, which means it has nothing to do with auto indent.

Code Stable
movelines-boundary

The root cause is we won't expand the selection if the endColumn of the selection is 1. cc @alexandrudima

@rebornix rebornix added editor-contrib Editor collection of extras under-discussion Issue is under discussion for relevance, priority, approach and removed bug Issue identified by VS Code Team member as probable bug important Issue identified as high-priority labels Jun 26, 2017
@jrieken
Copy link
Member Author

jrieken commented Jun 26, 2017

@rebornix In the gif I have shared the line reading ]; moves a few lines up (from 97 to 94) and then sticks there. I don't think that is to be discussed but a bug. The behaviour must be consistent, no matter how it is. See, it's not me that makes an empty selection but indent/outdent logic.

@jrieken jrieken added bug Issue identified by VS Code Team member as probable bug important Issue identified as high-priority and removed under-discussion Issue is under discussion for relevance, priority, approach labels Jun 26, 2017
@rebornix
Copy link
Member

@jrieken make sense, I'm convinced :)

movelines-boundary

@rebornix rebornix added this to the June 2017 milestone Jun 29, 2017
@rebornix
Copy link
Member

Revisit this issue as my previous fix for this issue actually leads to #29713 and #29682 .

The underlying problem of this issue is how we handle the empty ending line

  • Select several lines, and make sure select only whitespaces of last line
  • Move Lines to trigger auto indent
  • Auto indent adjusts the indentation and in some cases, the whitespaces selected in the last line is shrinked to one single cursor
  • Move Lines again. The last line is not treated as part of the moving block (see Move line up drops a line #29457 (comment)).

We have this behavior from the very beginning. It helps a lot as when users select several lines, they may happen to select the beginning of one line below. My first fix is if you have auto indent, then I'll always see this line as part of the moving block, which is a breaking change.

Now I reverted this change and the new way to mitigate this issue is

  • Move Lines
  • If the indentation of the last line shrinks to empty, and we no longer select anything in the last line, we automatically select one more character, just to make sure this line is still in the moving block.

It's not a clean fix but as our Move Lines command is stateless, this fix solves most of the problem. Let me know if you have better ideas on this and look forward to feedback of all of you.

rebornix added a commit that referenced this issue Jun 29, 2017
@jrieken jrieken added the verified Verification succeeded label Jun 29, 2017
@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug editor-contrib Editor collection of extras important Issue identified as high-priority verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

3 participants