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

Support substring and replacement #8 #9

Merged
merged 10 commits into from
Jul 2, 2021

Conversation

pombredanne
Copy link
Collaborator

@pombredanne pombredanne commented Jun 30, 2021

This PR adds minimal support for some Bash idioms for #8

  • simple replacement as in ${foo/bar/baz}
  • substrings as in ${foo:4:2}

In addition:

  • add support for plain $parameter substitution including
    some simple nesting.
  • apply some minor code cleanups

Signed-off-by: Philippe Ombredanne pombredanne@nexb.com

pombredanne and others added 7 commits June 29, 2021 17:00
"Substring Expansion" is a bash'ism to extract slices from a string
in the form of:
 ${parameter:offset}
 ${parameter:offset:length}

This is a minimal implementation that only supports simple slices on
strings and does not support arrays

See https://www.gnu.org/software/bash/manual/html_node/Shell-Parameter-Expansion.html

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Replacing with "/" is a bash'ism to replace a pattern with a string in
the parameter being expanded with the form of:
 ${parameter/pattern/string}

This is a simple and minimal implementation:
- supports only plain strings patterns (and not actual glob patterns).
- does not handle #, %, @ and * pattern modifiers.

See https://www.gnu.org/software/bash/manual/html_node/Shell-Parameter-Expansion.html

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Using expand(env=None) was failing as the os module was not imported

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
This performs a simple expansion for plain $parameters (without curly
braces) and is done as a pre-processing step in expand(). This also
allows for some minimal level of parameter nesting as a $parameter can
be nested in a ${parameter}.

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Also update module documentation

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
pombredanne referenced this pull request in quepop/scancode.io Jul 1, 2021
fix #191
Signed-off-by: Mateusz Perc <m.perc@samsung.com>
@kojiromike kojiromike self-assigned this Jul 1, 2021
@pombredanne
Copy link
Collaborator Author

Note that I have tried to keep each commit mostly self-standing if you want to cherry pick some

simple_test_cases = {
# string, env, result
"-${parameter/$aa/$bb}-": (
dict(
Copy link
Owner

Choose a reason for hiding this comment

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

Why dict() instead of a literal dict?

Copy link
Collaborator Author

@pombredanne pombredanne Jul 2, 2021

Choose a reason for hiding this comment

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

I feel it makes it stand out more clearly as being a mapping of environment variables... but I am fine either way

Copy link
Owner

@kojiromike kojiromike left a comment

Choose a reason for hiding this comment

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

Very nice, thanks. I am approving this as-is. You can implement my additional suggestions, or just merge as-is.

Leaving off the final slash is valid in a substitution as in:
$ parameter=aa/bb/cc; echo "-${parameter/aa}-"
-/bb/cc-

Reported-by: Michael A. Smith <michael@smith-li.com>
Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
@kojiromike kojiromike merged commit 3efeb4e into kojiromike:main Jul 2, 2021
@pombredanne pombredanne deleted the bashisms branch July 2, 2021 13:10
@pombredanne
Copy link
Collaborator Author

Thank you for the swift merge.

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.

2 participants