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

Low: bash can do find and replace #528

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

Conversation

grooverdan
Copy link

Ok, shelling out to do find and replace when sh does it so easily urked me enough to write a patch.

tested with ./lvm.sh meta-data

@davidvossel
Copy link
Contributor

is this a bashism?

@grooverdan
Copy link
Author

Nope, all sh,
substitution ${a//x/y} and removing prefix/suffixes ${a%%xx} / ${a#xx} in the other PRs

and aside from that #!/bin/bash is the header of each file here.

@lge
Copy link
Member

lge commented Dec 17, 2014

Note that while ${a#xx}, ##, %, %% are portable, ${a/x/y} is not.
try yourself: dash -c 'X=AAA; echo ${X//A/B}'

Which means these changes are ok for any resource agent with #!/bin/bash,
but not for those that announce themselves as #!/bin/sh

I did not check if you hit any of the latter, so your change may even be ok,
and only the statement "all sh" was wrong...

@grooverdan
Copy link
Author

thanks @lge for the corrections. Will keep that in mind. Good to know dash does a good test.

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.

4 participants