-
Notifications
You must be signed in to change notification settings - Fork 1
Rework array functions #2
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
base: master
Are you sure you want to change the base?
Conversation
This modifies the array functions to operate on a named array in-place, rather than passing the array as arguments and returning a new one. The reason is: there's no real way to "return" an array in bash. You can echo items by newline, but then you flatten any elements that may contain a newline of its own. `array_filter` has been substantially changed: it now accepts a user function that should return true or false for each element. To help in the common filtering cases, two functions have been added here: `match_string` and `match_regexp`.
rabbbit
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice (magic)
| # $1: The array value to remove | ||
| # $@: The array values, e.g. "${myarray[@]}" | ||
| # $1: name of the array variable | ||
| # $@: name of the function to call plus any arguments; element to test will be |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not call out func_name as a separate argument?
That's what the code does too.
| local func_name=$1; shift | ||
| local -a func_args=("$@") | ||
|
|
||
| # Escape func args |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
random question: should this be a separate function? or do you want to have them independent? I'd personally be fine with a re-use, I think.
Also, I don't understand why we need escaping, I thought ${arr_name}[@] will correctly contain elements we need?
pawel@pawel-C02V306VHTDH ~ $ cat test.sh
#!/bin/bash
set -eu
func() {
echo $#
echo $@
}
func 123 123 123
pawel@pawel-C02V306VHTDH ~ $ bash test.sh
3
123 123 123Note: general sense: you could also be OVERVERBOSE in your comments and explain why exactly things are happening?
| # $1: name of the array variable | ||
| # $2: index of element to remove | ||
| # | ||
| array_pop() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: pop makes me think about removing a single element: I was expecting a single element to be returned (first? last?)
what about filter_by_index?
Also, random thought, you could re-use your array_filter, right? Could be a nice example on how to pass in functions in to array_filter.
| return $ret | ||
| } | ||
|
|
||
| main() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not have it in bash.sh as "test_with_default", or something?
Realistically, when copying this bash, I'd like the whole function to be "copy-paste-able"?
| local arr_name=$1 | ||
| local item=$2 | ||
|
|
||
| eval "local -a _tmp_arr=(\"\${${arr_name}[@]}\")" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, but I think this should be explained in the comment.
Maybe a separate section, then a point to that explanation in each of the functions that uses it?
(yeah, I would repeat the comment in every test, I think)
This modifies the array functions to operate on a named array in-place,
rather than passing the array as arguments and returning a new one.
The reason is: there's no real way to "return" an array in bash.
You can echo items by newline, but then you flatten any elements that
may contain a newline of its own.
array_filterhas been substantially changed: it now accepts a userfunction that should return true or false for each element. To help in
the common filtering cases, two functions have been added here:
match_stringandmatch_regexp.