-
-
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
Read command #736
Read command #736
Conversation
@@ -233,6 +233,7 @@ | |||
"postinstall": "node ./node_modules/vscode/bin/install && gulp init" | |||
}, | |||
"dependencies": { | |||
"child-process": "^1.0.2", |
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.
Indent :)
Thanks for taking things that are currently not on our list :) Just drop some comments and can you update the roadmap in your PR as well, especially add |
I've updated the roadmap and dropped one comment at least :) |
} | ||
|
||
async getTextToInsertFromFile() : Promise<string> { | ||
// TODO: Substitute utf8 with current file encoding, couldn't find this anywhere in vscode's api. |
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.
Is there an issue tracking this with VSCode? Some initial Google says there's some config at the workspace level where you can set the encoding. Maybe we can read that config?
@dominic31 awesome work! dropped a few comments. |
@jpoon thanks for the comments! I've left some replies. |
LGTM.
|
I've updated the unit test. About the encoding, there's an open issue on the VS Code project for supporting encoding related functions through the api. It hasn't got past the backlog yet. Shall I open an issue here that refers to that issue? |
Yes please! One last thing, can you merge with master and resolve the conflicts? |
Sorry, I've gotten things a little muddled up here, the encoding parameter in fs.readFile() is of course the encoding of the external file that gets read in, not the current file in VS Code. Looking at the vim docs for read (link in the first comment), the encoding can be specified in the ++opt arguments that haven't yet been implemented. I'll update the comment in getTextToInsertFromFile(). Should I open an issue for implementing ++opt args, or just mention it in the roadmap? |
I'm okay with this not having ++opt, that can be a separate task. :) |
Initial support for read and read!.
Vim documentation, read
Vim documentation, read!
Range and opt arguments are not yet implemented.