Skip to content

Parent search bug fix #2030

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

Merged
merged 4 commits into from
Jan 23, 2023
Merged

Parent search bug fix #2030

merged 4 commits into from
Jan 23, 2023

Conversation

adamsitnik
Copy link
Member

Every symbol can have multiple parents. When getting the completions for commands, the global options defined by the parent commands should be included as well. The problem was that we were searching only the parents of current command (flat, one level above), rather going up in the hierarchy to reach the root command

Discovered by dotnet/sdk#29131

@KalleOlaviNiemitalo
Copy link

KalleOlaviNiemitalo commented Jan 23, 2023

This seems wrong to me. Instead of looking at statically defined ancestry of the Command, it should get the CommandResult objects from the ParseResult in order to know which intermediate commands were actually used.

I.e. if you have

  • RootCommand: subcommands { Mid1Command, Mid2Command }
  • Mid1Command: subcommands { LeafCommand }, options { GlobalOption1 }
  • Mid2Command: subcommands { LeafCommand }, options { GlobalOption2 }

and the user is asking for completions of "Mid2 Leaf", then it should offer only GlobalOption2 and not GlobalOption1.

@adamsitnik
Copy link
Member Author

@KalleOlaviNiemitalo thank you for excellent feedback! I've added a failing test based on it and provided the fix. PTAL again

@adamsitnik adamsitnik merged commit 6524142 into dotnet:main Jan 23, 2023
@adamsitnik adamsitnik deleted the parentSearchFix branch January 23, 2023 17:55
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.

3 participants