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

Allow users to use their own cursor style for insert from editor.cursorStyle #1399

Merged
merged 7 commits into from
Mar 25, 2017
Merged

Allow users to use their own cursor style for insert from editor.cursorStyle #1399

merged 7 commits into from
Mar 25, 2017

Conversation

xconverge
Copy link
Member

No description provided.

return vscode.TextEditorCursorStyle.Block;
} else if (cursorStyle === 'underline') {
return vscode.TextEditorCursorStyle.Underline;
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once we can upgrade to 1.10.0^ for the engine, I can add 2 more cases here for line-thin and underline-thin and it will work right away

@xconverge xconverge changed the title [WIP] Allow users to use their own cursor style for insert from editor.cursorStyle Allow users to use their own cursor style for insert from editor.cursorStyle Mar 15, 2017
@xconverge
Copy link
Member Author

@johnfn should be good to go. I tried to use vimState.editor when we had it available (in actions.ts for sure and some commands) this fixes #1356 and updates the minimum version of the latest extension to 1.10.0, now seems like an OK time due to some stability with the latest releases in my opinion

@xconverge
Copy link
Member Author

@johnfn can you take a peek at this when you get a chance

@johnfn
Copy link
Member

johnfn commented Mar 25, 2017

I was slightly intimidated by the sheer line count of changes, haha. It looks fine. Feel free to merge when you get to my one comment.

@xconverge
Copy link
Member Author

@johnfn might need to click finish review then ;) I dont see a comment

}

private cursorStyleFromString(cursorStyle: string): vscode.TextEditorCursorStyle {
if (cursorStyle === 'line') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you just make an object here like { line: vscode.TextEditorCursorStyle.Line, ... } etc?

I actually can't understand why that isn't already provided to us lol

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can, I actually copied this code from vscode since I was curious how they did it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hah really? But they don't expose that anywhere? Meh

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haha we should fix it and then submit it as a PR back to them 😁

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also refine their type string to be a union type amongst all the different cursor types... Haha, but you dont have to do that, we don't get much of an advantage from it unfortunately.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I think I changed it to what you meant...let me know

@johnfn
Copy link
Member

johnfn commented Mar 25, 2017

Grah, the number of times I've forgotten to click that button is too high.

@xconverge xconverge merged commit 84d209f into VSCodeVim:master Mar 25, 2017
@xconverge xconverge deleted the use-users-cursor-style branch March 25, 2017 18:29
This pull request was closed.
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

Successfully merging this pull request may close these issues.

2 participants