| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
Thanks for the PR! It looks like you've changed the TSServer protocol in some way. Please ensure that any changes here don't break consumers of the current TSServer API. For some extra review, we'll ping Sheetal Nandi (@sheetalkamat), Andrew Casey (@amcasey), Matt Bierner (@mjbvz), Mine Starks (@minestarks) for you. Feel free to loop in other consumers/maintainers if necessary |
Sorry, something went wrong.
|
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
Sorry, something went wrong.
|
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
Sorry, something went wrong.
|
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
Sorry, something went wrong.
There was a problem hiding this comment.
Looks good, just a couple small things, and I’ll try it out in the meantime!
Sorry, something went wrong.
|
TypeScript Bot (@typescript-bot) pack this |
Sorry, something went wrong.
|
Heya Andrew Branch (@andrewbranch), I've started to run the tarball bundle task on this PR at 318b59f. You can monitor the build here. |
Sorry, something went wrong.
There was a problem hiding this comment.
Protocol changes and the described behavior look good to me. Thanks for looking into this!
Sorry, something went wrong.
|
TypeScript Bot (@typescript-bot) pack this? |
Sorry, something went wrong.
|
Heya Andrew Branch (@andrewbranch), I've started to run the tarball bundle task on this PR at 318b59f. You can monitor the build here. |
Sorry, something went wrong.
|
Hey Andrew Branch (@andrewbranch), I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so: {
"devDependencies": {
"typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/110739/artifacts?artifactName=tgz&fileId=EBEE052423D2275D919A988BA0A41126E538F286E254B7D4FC5DE13958A35E4202&fileName=/typescript-4.5.0-insiders.20210916.tgz"
}
}
and then running npm install. There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/pr-build@4.5.0-pr-45903-8".; |
Sorry, something went wrong.
There was a problem hiding this comment.
I forgot one thing!
Sorry, something went wrong.
|
I think it can be much smarter for boolean attributes. <input checke||It should provide the following completions:
|
Sorry, something went wrong.
|
Jack Works (@Jack-Works) We can only provide with one completion for each attribute, so in the case of "auto" we have decided that the best approach for booleans is to complete as little as possible. In you're example, that would be the third option: checked. By the time this completions is provided, we don't fully know the intention of the user, so providing with a different option might be extra work for the user if it wants to have characters removed. For users who always wants to have braces on their attributes, they will be able to configure the options with "braces" to achieve that. |
Sorry, something went wrong.
This isn’t true, but it is typical of current completions. I did think about an approach like Jack Works (@Jack-Works) mentioned, but I personally didn’t find it compelling. It takes very few keystrokes to get from checked to checked={|cursor here|}, and moving my fingers away from the home row to the arrow keys is more disruptive to my flow than typing those characters. More importantly, if VS Code ends up auto-selecting one of the longer options when you really want the short option, it’s super annoying. This is partly beyond our control since VS Code has their own settings and logic that can influence the pre-selected completion item. I do think it’s worth exploring these kinds of completions as we expand our snippet support, and I’m happy to hear other opinions, but I am initially biased against this one. Like Armando said, we’re starting out filling in the minimal high-confidence completion because typing code you do want is generally preferable to backspacing code you don’t want but got anyway, at least up to a point. |
Sorry, something went wrong.
|
From checked to checked={|cursor_here|} it need 3 key press: =, shift, { (and IDE will add }). From checked={|cursor_here|} to checked only needs two: backspace (delete pair of {}), backspace (delete =). |
Sorry, something went wrong.
|
Jack Works (@Jack-Works) We discussed offline a little bit about the snippet you propose and we decided to merge it as is to gather user feedback. I would suggest to submit this improvement as a suggestion to vscode and we can analyze further the best options. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Implements suggestion #38891.