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

add support for bash Version 4 #4

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

Conversation

Falcosc
Copy link

@Falcosc Falcosc commented Jun 25, 2019

The colon fix was missing at the cur and prev assignment.

Now bash version 4 is supported for devices newer than 2009 ;)

On some distros you may need to install bash-completion package.
On Windows you already have bash4 and bash-completion with git bash mingw. On cygwin you may need to install bash-completion to get _get_comp_words_by_ref

@Falcosc Falcosc mentioned this pull request Jun 25, 2019
@Falcosc Falcosc force-pushed the master branch 2 times, most recently from 468acb2 to dfdb60c Compare June 25, 2019 07:32
@wadewegner
Copy link
Owner

This looks like a great update but sadly I don't know enough to know if this will break a lot of people. @Falcosc IIRC, this was the hardest part about getting it to work with bash. Do you have any suggestions on how I can best test this out to ensure I don't break people? Thank you!

@Falcosc
Copy link
Author

Falcosc commented Jul 6, 2019

Install bash v4 on your mac or use linux with bash v4 or a VM with windows and gitbash or if you would like to test even more, windows 10 with wsl but that should be same as linux.

At the end it can be only go better because current version is already broken and do only work with the 9 years old bash v3.

@gdemarcos-SFDC
Copy link

I just tested changing that one liner and working perfect under Windows 10 gitbash mingw64. Thank you! This tool is great. 👍😎

@SCWells72
Copy link

I just verified that this change works great on Bash 4 under Cygwin, Mingw, and Ubuntu (both WSL and a dedicated install), but it also breaks Bash 3 which is the default on Mac OS X. The change should be conditional based on the version of Bash with the original behavior used on Bash 3 and the new behavior on Bash 4+.

@Falcosc
Copy link
Author

Falcosc commented Aug 7, 2019

You just need to install bash-completion on mac or any other environment which doesn't have this by default. If you don't have bash-completion you get just the message that you are missing functions from bash-completion.

It's bad practice to copy sourcecode from bash-completion into this script like this:
https://github.com/wadewegner/salesforce-cli-bash-completion/blob/master/sfdx.bash#L10

Better would be something like this:
if __ltrim_colon_completions or _get_comp_words_by_ref is missing: echo "Please install bash-completion"

I will not put any bash-completion source code copy into this pullrequest.

@baksale
Copy link

baksale commented Jun 25, 2020

master did not work on RHEL7.
This branch works perfect.

@juan-cs
Copy link

juan-cs commented Jun 26, 2020

Tested it on WSL2 and works 👍

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.

6 participants