Skip to content
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

Call bootstrap scripts with a tty. #449

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

JeffFaer
Copy link

@JeffFaer JeffFaer commented Mar 8, 2023

What does this PR do?

Changes the contributed bootstrap-in-dir script so that it passes the tty through to the individual bootstrap scripts.

What issues does this PR fix or reference?

#344

Previous Behavior

stdin for the individual scripts was the pipe from the find command.

New Behavior

stdin for the individual scripts is the same as it is for the bootstrap-in-dir script (typically a tty)

Have tests been written for this change?

no

Have these commits been signed with GnuPG?

no


Please review yadm's Contributing Guide for best practices.

The exising bootstrip-in-dir script is changing stdin to be the result
of the find command.

fix TheLocehiliosan#344
@TheLocehiliosan TheLocehiliosan self-assigned this Mar 16, 2023
@mweberjc
Copy link

mweberjc commented Jun 7, 2023

This triggers a shellcheck complaint:

In .config/yadm/bootstrap line 18:
bootstraps=( $(find -L "$BOOTSTRAP_D" -type f | sort) )
             ^-- SC2207 (warning): Prefer mapfile or read -a to split command output (or quote to avoid splitting).

@JeffFaer
Copy link
Author

That shellcheck warning seems like a bit of a false positive to me. IFS is being explicitly set, so there shouldn't be any surprises about how the command is being split into the array.


https://www.shellcheck.net/wiki/SC2207

If you have already taken care (through setting IFS and set -f) to have word splitting work the way you intend, you can ignore this warning.

TIL that file name globbing would still happen in this situation:

$ print_elements() {
>   echo "foo"
>   echo "bar"
>   echo "D*"
> }
$ IFS=$'\n' arr=( $(print_elements) )
$ echo "${arr[@]}"
foo bar Desktop Documents Downloads

Wild. I'll switch to use mapfile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants