Code completion #209
Code completion #209
Conversation
alexfrasson
requested review from
allevato,
laurentlb and
thomasvl
as code owners
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
alexfrasson
force-pushed
the
code-completion
branch
from
6d92dd4
to
3ed18b5
Compare
|
@googlebot I signed it! |
|
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
alexfrasson
force-pushed
the
code-completion
branch
6 times, most recently
from
b2b88c6
to
1d108a0
Compare
alexfrasson
force-pushed
the
code-completion
branch
from
1d108a0
to
4dae04d
Compare
|
@allevato @laurentlb @thomasvl could I please get some feedback on this? 🙂 |
|
do you plan on reviewing and merging it? |
|
Is that project dead? |
|
Hey come on, let's not ignore this man's hard work... we'd all benefit from this! |
|
Tagging @philwo, since it looks like he's the Googler who was most recently active on the project. |
|
Hey folks, thanks for tagging me. Unfortunately I really do not have any experience with TypeScript at all, so I'm not a good reviewer. 😅 But my colleague @coeuvre might be able to review this. I'm happy to handle the releasing part later. |
| document: vscode.TextDocument, | ||
| position: vscode.Position, | ||
| ) { | ||
| let candidateTarget = GetCandidateTargetFromDocumentPosition( |
There was a problem hiding this comment.
candidateTarget could be undefined. Please check before use.
| import * as vscode from "vscode"; | ||
| import { queryQuickPickTargets } from "../bazel"; | ||
|
|
||
| function InsertCompletionItemIfUnique( |
There was a problem hiding this comment.
Function name should start with a lower case character to keep code style consistent. Please change this and following ones.
| } | ||
|
|
||
| const completionItems = new Array<vscode.CompletionItem>(); | ||
| this.targets.forEach((target) => { |
There was a problem hiding this comment.
this.targets could be undefined if refresh hasn't finished yet. Please check before use.
| implements vscode.CompletionItemProvider { | ||
| private targets: string[] | undefined; | ||
|
|
||
| public provideCompletionItems( |
There was a problem hiding this comment.
Can you please add comment to briefly explain how the completion works?
| return completionItems; | ||
| } | ||
|
|
||
| public async refresh() { |
There was a problem hiding this comment.
Can you please add comment to briefly explain how refresh works and why do we need it?
|
@alexfrasson, are you still around? (Bumping bc I got reminded of this PR, and he might not be after...almost a year. @coeuvre, any chance you'd be able to shepard this through if not?) |
|
Paging @alexfrasson :) |
|
@coeuvre, could I ask you to guide this through to merging? Looks like Alex is long gone, but it'd be really great to have. |
|
Sure, I will fork this patch and create another PR. |
|
@coeuvre Thank you for the review and sorry for not getting back to you sooner. I will have some free time next week and I could go over the PR again if you want. |
|
That's great. Thanks! |
|
@alexfrasson, any chance you'd still be down to take a pass through so this can get landed? |
|
@coeuvre, could we revert to your guiding this through to merging? It'd be awesome to have. |
|
Sure. I will look into this next week. |
|
This was merged via #246 - thanks everyone for your patience and help! |
|
Wahoo! Thank you, all! |
Enables Intellisense code completion for targets/packages within BUILD files.
#210