Skip to content

Conversation

@segudev
Copy link

@segudev segudev commented Jul 30, 2025

Summary

Fixed a bash syntax error that was causing cmdk to fail with "bad substitution: no closing ')'" error.

Root Cause

The original code used process substitution < <(...) with conditional logic (if/return) inside it. This creates a subshell where return statements don't work properly and can cause syntax errors in different bash versions.

Solution

  • Refactored to use command substitution $() to capture fzf output
  • Moved error checking outside the substitution
  • Use here-string <<< to process the captured output
  • Changed return to exit 1 for proper subprocess handling

Testing

  • ✅ Tested on macOS with zsh calling bash script
  • ✅ Should be compatible with Linux bash and fish (via wrapper)
  • ✅ Uses standard bash features for better portability

Changes

  • cmdk-core.sh: Restructured fzf execution and output processing

Fixes the "bad substitution" error that prevented cmdk from running.

Refactored the fzf command execution to use command substitution
instead of process substitution with conditional logic. The original
code had a `return` statement inside a process substitution which
created a bash syntax error.

Changes:
- Capture fzf output in a variable first
- Check exit status separately
- Use here-string to process the output
- Replace `return` with `exit 1` for proper subprocess handling

This improves compatibility across different bash versions and
makes the code more readable.

Fixes the "bad substitution: no closing ')'" error.
@mieubrisse
Copy link
Owner

Thank you @segudev! I just pushed a cleanup to main that's going to cause a conflict with your branch; I'll put the fix you need in a comment. Excited to get this merged!


output_paths=()

# EXPLANATION:
Copy link
Owner

Choose a reason for hiding this comment

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

Replace all these lines with the following to fix the merge conflict:

Suggested change
# EXPLANATION:
# EXPLANATION:
# -m allows multiple selections
# --ansi tells fzf to parse the ANSI color codes that we're generating with fd
# --scheme=path optimizes for path-based input
# --with-nth allows us to use the custom sorting mechanism
fzf_output="$(FZF_DEFAULT_COMMAND="bash ${script_dirpath}/list-files.sh ${*}" fzf \
-m \
--ansi \
--bind='change:top' \
--scheme=path \
--preview="bash ${script_dirpath}/preview.sh {}")"
if [ "${?}" -ne 0 ]; then
exit 1
fi
while IFS="" read -r line; do # IFS="" -> no splitting (we may have paths with spaces)
output_paths+=("${line}")
done <<< "${fzf_output}"

@mieubrisse
Copy link
Owner

Hey @segudev , just wanted to check if you'd still like to merge this?

- Remove braces from $* and $@ parameter expansions to handle empty arguments
- Change list-files.sh shebang from sh to bash (script uses bash arrays)
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