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

improve magic command completions #3700

Merged
merged 3 commits into from
Oct 8, 2024

Conversation

jonsequitur
Copy link
Contributor

This fixes a number of cases with magic command completions.

return subcommandCompletions.Concat(baseCompletions).ToArray();
completions.AddRange(subcommandCompletions);

if (Parent is KernelDirective parent)
Copy link
Contributor

Choose a reason for hiding this comment

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

Does order in which the completions are added to the list matter? If yes, should the directive-based ones be added first?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't matter. At this level, the final list hasn't been assembled, so ordering doesn't matter yet, nor does e.g. filtering for partially-typed completions.


case DirectiveNameNode nameNode when subcommandNode is not null:
subcommandNode.Add(nameNode);
case DirectiveParameterNode parameterNode when subcommandNode is null:
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a duplicate of the case on line 188 above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is! Nice catch.

@jonsequitur jonsequitur enabled auto-merge (squash) October 8, 2024 17:52
@jonsequitur jonsequitur merged commit 3895c8c into dotnet:main Oct 8, 2024
4 checks passed
@colombod colombod added the Area-Language Services IntelliSense, LSP, and related label Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants