Leave selection unchanged when toggling line comment#51493
Leave selection unchanged when toggling line comment#51493dibarbet merged 1 commit intodotnet:masterfrom
Conversation
aaee915 to
632701f
Compare
|
thanks @ryzngard ! |
justcla
left a comment
There was a problem hiding this comment.
When applying ToggleLineComment to a selection, the start of the selection should not change. Current tests move the caret two places to the left to include the comment markers. This is not expected behaviour.
| void M() | ||
| { | ||
| [| ////var i = 1; | ||
| [|////var i = 1; |
There was a problem hiding this comment.
This has changed the selection by including the comment markers inside the selection. This is not the behaviour intended.
| void M() | ||
| { | ||
| [| //var i = 1; | ||
| [|//var i = 1; |
There was a problem hiding this comment.
The start marker of the selection should remain as it was - immediately left of "var".
| void M() | ||
| { | ||
| [| //var i = 1;|] | ||
| [|//var i = 1;|] |
There was a problem hiding this comment.
This has moved the caret. Suggest leaving the caret unchanged (as per spec).
Caret should be hard left against "var".
| void M() | ||
| { | ||
| [| //var i = 1; // A comment.|] | ||
| [|//var i = 1; // A comment.|] |
There was a problem hiding this comment.
This has moved the caret and changed the selection. Please leave the selection unchanged. Caret should remain hard left against "var", which is where it was before applying the action.
| void M() | ||
| { | ||
| [| //string s = '\\';|] | ||
| [|//string s = '\\';|] |
There was a problem hiding this comment.
This has changed the selection by adding extra chars at the beginning.
Please leave the selection unchanged by keeping the caret immediately to the left of "string".
| void M() | ||
| { | ||
| [| //var i = 1; // A comment. | ||
| [|//var i = 1; // A comment. |
There was a problem hiding this comment.
Selection start should remain the same - right next to "var".
|
@justcla thanks for the feedback. I'm not totally sure I agree with not including the comment marker. When there is a subsection of the line selected I think it makes sense to leave the selection as it was (so you can easily change a variable name or something similar). However having the selection include the comment marker when the whole line is selected seems to make sense to me. I'll try out your suggestion and play around with both options for an hour or two to see which one feels better. |
|
Thanks Dave. I agree with you. |
|
I still think there is an issue with adding comment markers anywhere but the beginning of the line. When you apply comment markers one line at a time in an indented section, the comments will be scattered. Applying line comment markers at the start of the line provides consistency. And furthermore... why not? The user has asked to comment the entire line. So why start a comment some 'random' number of chars in? |
Forked from #51411
Resolves #36349