Skip to content
This repository was archived by the owner on Apr 20, 2023. It is now read-only.

Conversation

@larson-carter
Copy link

I did NOT find this solution, I know @mcornella is quite busy and I wanted to make sure that this was able to be formed into a PR.

This solves issue dotnet/sdk#9232

I did NOT find this solution, I know @mcornella is quite busy and I wanted to make sure that this was able to be formed into a PR.

This solves issue dotnet/sdk#9232
@mcornella
Copy link

Thanks @larson-carter but this is wrong, the whole file should be what I posted, not just the function body.

@mcornella
Copy link

Also for the future, it's better not to use GitHub mentions in git commits because GitHub sends a message every time the commit appears in a fork.

@larson-carter
Copy link
Author

Also for the future, it's better not to use GitHub mentions in git commits because GitHub sends a message every time the commit appears in a fork.

Oh wow I didn't know that.

Thanks @larson-carter but this is wrong, the whole file should be what I posted, not just the function body.

Also, I removed everything except what you committed. It's odd that there is no function body.

@rainersigwald
Copy link
Member

@larson-carter
Copy link
Author

@rainersigwald I had no clue that was a thing. Thank You!

@mcornella
Copy link

+1 to the current version. Should we do something about the filename? Normally completion files are of the form _<command> where <command> is the command for which the file provides completion, dotnet in this case.

@jongalloway
Copy link

jongalloway commented Jun 4, 2020

@dsplaisted @terrajobst
This file should be renamed from scripts/register-completions.zsh to /scripts/_dotnet to match zsh completions convention.

@larson-carter is willing to rename but wants to make sure that won't cause conflicts with other scripts. Any issues?

@omajid
Copy link
Member

omajid commented Jun 5, 2020

@jongalloway bash completions use the same filename convention: https://github.com/scop/bash-completion/tree/master/completions

To avoid conflict, I think we should keep the names as they are. We should configure the installer and the update the manual installation steps to tell users where the files need to go.

compctl -K _dotnet_zsh_complete dotnet
#compdef dotnet
local completions=("$(dotnet complete "$words")")
compadd "${(ps:\n:)completions}"

Choose a reason for hiding this comment

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

Hey there, coming back to this to add that it should be

compadd -- "${(ps:\n:)completions}"

to avoid dash options to be passed to compadd instead of being shown as completion entries.

Choose a reason for hiding this comment

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

Also we should only call compadd if there are $completions, i.e. if [[ -n "$completions" ]] is true. Otherwise a blank space is called each time a completion is triggered and there are no entries returned.

@mcornella
Copy link

Also, should this be submitted to dotnet/sdk instead?

@dsplaisted
Copy link
Member

Also, should this be submitted to dotnet/sdk instead?

Yes, dotnet/sdk would be the right repo now.

@marcpopMSFT
Copy link
Member

CLI repo has been archived and merged with https://github.com/dotnet/sdk/tree/master/src/Cli. Closing all old PRs in the cli repo to master.

@omajid
Copy link
Member

omajid commented Jan 27, 2021

@marcpopMSFT can we move this issue to dotnet/sdk instead? The issue still exists and there's even a PR open in the sdk repo that refers to this issue.

@marcpopMSFT
Copy link
Member

This is a PR. All issues we're previously moved to the SDK repo and as you noted there is a PR open for this and an issue tracking it already. I was just cleaning up old leftover PRs.

@omajid
Copy link
Member

omajid commented Jan 29, 2021

Oops. Thanks for correcting me!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants