-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
fix #690 and other toggle case issues #698
Conversation
const text = TextEditor.getText(range); | ||
|
||
let newText = ""; | ||
for (var i = 0; i < text.length; i++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you refactor out this duplicated code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea I was waiting for this comment, I figured that this was better than polluting the other one with VisualBlock specific stuff.
Where would a function like this to DRY it up go?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haha, yeah, you know it had to happen. Maybe in a utilities file somewhere? Or if you can't find/dont want to make one, then a static method on a class can work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 static method
@@ -3537,20 +3537,55 @@ class ToggleCaseOperator extends BaseOperator { | |||
|
|||
public async run(vimState: VimState, start: Position, end: Position): Promise<VimState> { | |||
const range = new vscode.Range(start, new Position(end.line, end.character + 1)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you also change this to: new vscode.Range(start, end.getRight());
.
Probably doesn't matter but I'd sleep better at night with the position checking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do, insomnia is no fun
Pending @johnfn's comment. LGTM. |
Should be good to go |
@xconverge once you merge latest from |
Toggle case wasn't toggling interWoVen cases...
Also made it repeatable with dot and correctly