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

Include samples in completions when the document is empty #2009

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

minestarks
Copy link
Member

Fixing the behavior that regressed in #1947 when I was attempting to limit samples to empty documents only.

// Include the samples list for empty documents
return document.lineCount > 0 ? results : results.concat(this.samples);
return !isEmptyOrWhitespace ? results : results.concat(this.samples);
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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.

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