Include samples in completions when the document is empty #2009
Conversation
vscode/src/completion.ts
Outdated
| // Include the samples list for empty documents | ||
| return document.lineCount > 0 ? results : results.concat(this.samples); | ||
| return !isEmptyOrWhitespace ? results : results.concat(this.samples); |
There was a problem hiding this comment.
I'm not sure this is the right approach. What if I start a file with some TODO comments (which I often do) or a license header (which is common)? Or I import some items knowing I'll be using them as I expand on the sample I'm starting from?
I think it's reasonable to only show when the user is on a blank line though.
There was a problem hiding this comment.
Actually, in the spirit of the 'correctness', being that the samples are all 'top level' code, could we just show them in situations when at the top level (i.e. the places where we currently show 'namespace').
I'd rather limit the 'magic' of when samples do or don't appear in a completion list that may not be intuitive to users. Have them appear only at top level seems to fit in the with correctness goal of only having completions show where they are valid.
There was a problem hiding this comment.
That's doable, but takes the difficulty level of this PR from a 1 to about a 6, since this code has no awareness of the AST, and the code that does know about the AST has no awareness of samples. I'm not sure I can justify that work right now... I can at least trivially ignore comments (since they can be textually detected without the AST). Or I can go back to the way it was before I broke it (always include samples). My inclination is the former (ignore comments) if that's ok with you too.
There was a problem hiding this comment.
i.e. the places where we currently show 'namespace'
Oh, now I think I see what you were getting at. I'll just check for the presence of namespace in the list, should be trivial and fairly accurate :)
There was a problem hiding this comment.
Per offline discussions. Will change "namespace" to "operation" in anticipation of #2023
minestarks
force-pushed
the
minestarks/fix-samples
branch
from
ea150fc
to
52b5805
Compare
Fixing the behavior that regressed in #1947 when I was attempting to limit samples to empty documents only.