Skip to content

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.

Sign up for GitHub

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

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

Rework array functions #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 99 additions & 12 deletions bash.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,40 +12,83 @@
#
# The only proper way to do this is by looping through each item. Other
# solutions claim to do this using string matching, but they are often
# dangerous because don't handle edge cases.
# dangerous because they don't handle edge cases, e.g. may be susceptible to
# partial match or may not properly handle elements containing newlines.
#

#
# Returns 0 if item is in the array; 1 otherwise.
#
# $1: The array value to search for
# $@: The array values, e.g. "${myarray[@]}"
# $1: name of the array variable to search
# $2: array item to search for
#
array_contains() {
local item=$1; shift
for val in "$@"; do
local arr_name=$1
local item=$2

eval "local -a _tmp_arr=(\"\${${arr_name}[@]}\")"
Copy link

@rabbbit rabbbit Jul 30, 2019

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)


for val in "${_tmp_arr[@]}"; do
if [ "$val" == "$item" ]; then
return 0
fi
done

return 1
}


#
# Returns 0 if regexp matches any item in the array; 1 otherwise.
#
# $1: name of the array variable to search
# $2: regexp to match
#
array_contains_regexp() {
local arr_name=$1
local item=$2

eval "local -a _tmp_arr=(\"\${${arr_name}[@]}\")"

for val in "${_tmp_arr[@]}"; do
if [[ "$val" =~ $item ]]; then
return 0
fi
done

return 1
}

# ### Array filter

#
# Return all elements of an array with the specified item removed.
# Remove elements from the named array. Iterate over each element and call
# named function. If the function returns false, the element is removed.
#
# $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
Copy link

@rabbbit rabbbit Jul 30, 2019

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.

# provided as the last arg
#
array_filter() {
local item=$1; shift
for val in "$@"; do
if [ "$val" != "$item" ]; then
echo $val
local arr_name=$1; shift
local func_name=$1; shift
local -a func_args=("$@")

# Escape func args
Copy link

@rabbbit rabbbit Jul 30, 2019

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 123

Note: general sense: you could also be OVERVERBOSE in your comments and explain why exactly things are happening?

for i in "${!func_args[@]}"; do
func_args[$i]=\'"${func_args[@]}"\'
done

eval "local -a _tmp_arr=(\"\${${arr_name}[@]}\")"

local -a filtered_arr=()
for val in "${_tmp_arr[@]}"; do
if ! eval "$func_name" "${func_args[@]}" "$val"; then
filtered_arr+=("$val")
fi
done

eval "${arr_name}=(\"\${filtered_arr[@]}\")"
}

# ### Array join
Expand All @@ -61,6 +104,34 @@ array_join() {
IFS=$sep eval 'echo "$*"'
}

#
# $1: name of the array variable
# $2: index of element to remove
#
array_pop() {
Copy link

@rabbbit rabbbit Jul 30, 2019

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.

local arr_name=$1
local -i index=$2

eval "local -a _tmp_arr=(\"\${${arr_name}[@]}\")"

if [ "$index" -gt $((${#_tmp_arr[@]} - 1)) ]; then
echo "array_pop: index out of range" >&2
return 1
fi

local -a filtered_arr=()
local -i c=-1
for val in "${_tmp_arr[@]}"; do
c=$((c+1))
# Skip item at index
[ $c -eq $index ] && continue

filtered_arr+=("$val")
done

eval "${arr_name}=(\"\${filtered_arr[@]}\")"
}

################
# Benchmarking #
################
Expand Down Expand Up @@ -275,6 +346,22 @@ parallel() {
# Testing #
###########

#
# $1: string to match
# $2: value to check
#
match_string() {
[ "$2" == "$1" ]
}

#
# $1: regexp
# $2: value to check
#
match_regexp() {
[[ "$2" =~ $1 ]]
}

# ### List all test functions

#
Expand Down
89 changes: 87 additions & 2 deletions test-bash.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,85 @@ test_absolute_path_single_dot() {
)
}

test_array_contains() {
local -a test_input=("foo" "bar")
array_contains test_input foo
}

test_array_contains_multiline() {
local -a test_input=("foo" "bar"$'\n'"baz")
array_contains test_input "bar"$'\n'"baz"
! array_contains test_input baz
}

test_array_contains_false() {
local -a test_input=("foo" "bar")
! array_contains test_input baz
}

test_array_contains_does_not_do_partial_match() {
local -a test_input=("foobar" "baz")
! array_contains test_input foo
}

test_array_contains_handles_whitespace_and_empty() {
# We want to make sure our implementation doesn't collapse whitespaces, or
# consider them to be the same, or consider "" to be equivalent.
local -a test_input=(" " $'\n' " ")
! array_contains test_input ""
}

test_array_contains_regexp() {
local -a test_input=("foobar" "baz")
array_contains_regexp test_input foo
}

test_array_filter() {
local -a arr=("foo" "bar" "baz")

array_filter arr match_string bar

[ ${#arr[@]} -eq 2 ]
[ "${arr[0]}" == "foo" ]
[ "${arr[1]}" == "baz" ]
}

test_array_filter_multiline() {
local -a arr=("foo" "bar"$'\n'"baz" "quux")
[ ${#arr[@]} -eq 4 ]

array_filter arr match_string "foo"

[ ${#arr[@]} -eq 2 ]
[ "${arr[0]}" == "bar"$'\n'"baz" ]
[ "${arr[1]}" == "quux" ]
}

test_array_filter_regexp() {
local -a arr=("foo" "bar" "baz")

array_filter arr match_regexp "foo|bar"

[ ${#arr[@]} -eq 1 ]
[ "${arr[0]}" == "baz" ]
}

test_array_pop() {
local -a arr=("foo" "bar"$'\n'"baz" "qux")
[ ${#arr} -eq 3 ]

array_pop arr 1

[ ${#arr} -eq 2 ]
[ "${arr[0]}" == "foo" ]
[ "${arr[1]}" == "qux" ]
}

test_array_pop_out_of_range() {
local -a arr=("foo" "bar")
! array_pop arr 2
}

test_resolve_symlinks() {
touch /tmp/foo
mkdir -p /tmp/1/2
Expand Down Expand Up @@ -73,10 +152,16 @@ tests() {
}

main() {
Copy link

@rabbbit rabbbit Jul 30, 2019

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 -a tests_to_run=("$@")

if [ ${#tests_to_run[@]} -eq 0 ]; then
tests_to_run=($(compgen -A function | grep -E ^test_))
fi

echo "Testing bash version: ${BASH_VERSION}"
tests "$@"
tests "${tests_to_run[@]}"
}

if [ "$0" == "$BASH_SOURCE" ]; then
main $(compgen -A function | grep -E ^test_)
main "$@"
fi