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

Only use nosort in bash <4.4 #1683

Merged
merged 2 commits into from
May 10, 2024
Merged

Only use nosort in bash <4.4 #1683

merged 2 commits into from
May 10, 2024

Conversation

martinml
Copy link
Contributor

@martinml martinml commented May 4, 2024

Today I updated git-delta via brew and started seeing the following error message in all my new shells:

-bash: complete: nosort: invalid option name

Turns out the bash_completion.d script included in delta uses nosort, which doesn't exist until Bash 4.4, which I don't have (native macOS bash is currently 3.2.57 for me).

So I added a check for the version, blatantly stolen heavily inspired from what cobra does.

if [[ ${BASH_VERSINFO[0]} -lt 4 || ( ${BASH_VERSINFO[0]} -eq 4 && ${BASH_VERSINFO[1]} -lt 4 ) ]]; then
complete -F _delta -o bashdefault -o default delta
else
complete -F _delta -o bashdefault -o default delta -o nosort
Copy link
Owner

Choose a reason for hiding this comment

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

Curious why you moved -o nosort after delta. I'm not saying it's incorrect but I think positional arguments (delta) typically go last.

Copy link
Contributor Author

@martinml martinml May 8, 2024

Choose a reason for hiding this comment

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

Whoops, my bad. I reordered them so the difference between the two lines was clearer, but it looks like I didn't realize delta was positional. Fixing it now.

Copy link
Owner

Choose a reason for hiding this comment

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

Thanks!

@dandavison dandavison merged commit 0f8a2f0 into dandavison:main May 10, 2024
13 checks passed
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.

None yet

2 participants