Conversation
Skips empty arguments. Handles defaults. Has tests.
| local IFS | ||
|
|
||
| default="$1"; shift | ||
|
|
| # | ||
| # $1: String separator | ||
| # $@: Array elements | ||
| # $1: String default, used in case array to join is empty |
There was a problem hiding this comment.
Based on the principle of do one thing and do it well, can you remove the "default"?
I feel this is conflating two responsibilities into one function (join should just join, nothing more, nothing less).
| # $1: String separator | ||
| # $@: Array elements | ||
| # $1: String default, used in case array to join is empty | ||
| # $2: String separator, has to be a single element |
There was a problem hiding this comment.
Since we're making array_join better, why don't we improve it to allow any join string, rather than just a single character?
I can imagine this is useful, e.g. say you want to create a human-readable CSV, so you want to join by ", " (comma-space).
| join() { | ||
| local -a args | ||
| local default | ||
| local ifs |
There was a problem hiding this comment.
A better name for the ifs variable here would be join_string.
|
|
||
| [ "$result" == "def" ] | ||
| } | ||
|
|
There was a problem hiding this comment.
Can you add the following test for joining an element containing a newline?
test_join_multiline_string_element() {
local result
local -a test_input=(
"foo"
"bar"
"omg"$'\n'"baz"
)
result=$(join "def" "," "${test_input[@]}")
[ "$result" == "foo,bar,omg"$'\n'"baz" ]
}The current code fails this test.
Skips empty arguments.
Handles defaults.
Has tests.