Skip to content

Commit 6af2900

Browse files
committed
commands: Don't raise error for duplicate commands
While trying to create a test case for #131 and #132 to demonstrate that setting the `_GO_SCRIPTS_DIR` for a plugin incorrectly may cause it to find the wrong command scripts, the test exited with an error at some point because of the constraint that no duplicate scripts are allowed by `_@go.find_commands` (via `_@go.merge_scripts_into_list`). Since plugins may contain command names that match those in other plugins or the top-level `_GO_SCRIPTS_DIR`, and since `_GO_SEARCH_PATHS` has the correct semantics with regard to which command should take precedence for any given script, I realized raising an error was no longer appropriate. Now the first command found via searching `_GO_SEARCH_PATHS` takes precedence over duplicates in later paths.
1 parent 2b8f09f commit 6af2900

File tree

2 files changed

+12
-17
lines changed

2 files changed

+12
-17
lines changed

lib/internal/commands

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,9 @@ _@go.merge_scripts_into_list() {
2323
rhs_name="${rhs_script##*/}"
2424

2525
if [[ "$lhs_name" == "$rhs_name" ]]; then
26-
printf "ERROR: duplicate command $lhs_name:\n\n %s\n %s\n" \
27-
"$lhs_script" "$rhs_script" >&2
28-
return 1
26+
result+=("$lhs_script")
27+
((++i))
28+
((++j))
2929
elif [[ "$lhs_name" < "$rhs_name" ]]; then
3030
result+=("$lhs_script")
3131
((++i))
@@ -56,10 +56,7 @@ _@go.find_commands() {
5656
scripts+=("$script")
5757
fi
5858
done
59-
60-
if ! _@go.merge_scripts_into_list "${scripts[@]#$_GO_ROOTDIR/}"; then
61-
return 1
62-
fi
59+
_@go.merge_scripts_into_list "${scripts[@]#$_GO_ROOTDIR/}"
6360
done
6461

6562
if [[ "${#__go_command_scripts[@]}" -eq '0' ]]; then

tests/commands/find.bats

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -94,21 +94,19 @@ assert_command_scripts_equal() {
9494
assert_command_scripts_equal "${__all_scripts[@]}"
9595
}
9696

97-
@test "$SUITE: return error if duplicates exists" {
97+
@test "$SUITE: commands from earlier paths precede duplicates in later paths" {
98+
# In this case, we're trying to duplicate a builtin. Since
99+
# `_GO_CORE_DIR/libexec` comes first in `_GO_SEARCH_PATHS`, it takes
100+
# precedence over the duplicate we add.
98101
local duplicate_cmd="${BUILTIN_SCRIPTS[0]##*/}"
99102
local __all_scripts=("${BUILTIN_SCRIPTS[@]}")
100103

101104
add_scripts "$duplicate_cmd"
102105
run "$TEST_GO_SCRIPT"
103-
assert_failure
104-
105-
assert_line_equals 0 "ERROR: duplicate command $duplicate_cmd:"
106-
107-
# Because the go-core.bash file is in the test's $_GO_ROOTDIR, and the test
108-
# script has a different $_GO_ROOTDIR, the builtin scripts will retain their
109-
# absolute path, whereas user scripts will be relative.
110-
assert_line_equals 1 " $_GO_ROOTDIR/${BUILTIN_SCRIPTS[0]}"
111-
assert_line_equals 2 " scripts/$duplicate_cmd"
106+
assert_success
107+
assert_line_equals 0 "LONGEST NAME LEN: ${#LONGEST_BUILTIN_NAME}"
108+
assert_line_equals 1 "COMMAND_NAMES: ${__all_scripts[*]##*/}"
109+
assert_command_scripts_equal "${__all_scripts[@]}"
112110
}
113111

114112
@test "$SUITE: return subcommands only" {

0 commit comments

Comments
 (0)