Mixing
bash how to improve this script
Clash Royale CLAN TAG#URR8PPP
.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;
up vote
4
down vote
favorite
I'm new to shell scripting and I'd like to know if there is maybe a better solution then the one I figured out:
I want to check if a user is in a list and if yes than the script should terminate with the exit_program function:
$USER is defined as somebody who logs into the system
My solution (it works) is:
$IGNORE_USER="USER1 USER2 USER3"
if [ ! -z "$IGNORE_USER" ]; then
for usr in $IGNORE_USER
do
if $USER = $usr; then
exit_program "bye bye"
fi
done
fi
bash shell-script
asked 13 hours ago
bernhard
212
add a comment |Â
up vote
4
down vote
favorite
I'm new to shell scripting and I'd like to know if there is maybe a better solution then the one I figured out:
I want to check if a user is in a list and if yes than the script should terminate with the exit_program function:
$USER is defined as somebody who logs into the system
My solution (it works) is:
$IGNORE_USER="USER1 USER2 USER3"
if [ ! -z "$IGNORE_USER" ]; then
for usr in $IGNORE_USER
do
if $USER = $usr; then
exit_program "bye bye"
fi
done
fi
bash shell-script
asked 13 hours ago
bernhard
212
You don't need to loop over your array of users. See here
– xenoid
10 hours ago
If you have a working script and want it reviewed, consider codereview.stackexchange.com
– kojiro
7 hours ago
add a comment |Â
up vote
4
down vote
favorite
up vote
4
down vote
favorite
I'm new to shell scripting and I'd like to know if there is maybe a better solution then the one I figured out:
I want to check if a user is in a list and if yes than the script should terminate with the exit_program function:
$USER is defined as somebody who logs into the system
My solution (it works) is:
$IGNORE_USER="USER1 USER2 USER3"
if [ ! -z "$IGNORE_USER" ]; then
for usr in $IGNORE_USER
do
if $USER = $usr; then
exit_program "bye bye"
fi
done
fi
bash shell-script
asked 13 hours ago
bernhard
212
I'm new to shell scripting and I'd like to know if there is maybe a better solution then the one I figured out:
I want to check if a user is in a list and if yes than the script should terminate with the exit_program function:
$USER is defined as somebody who logs into the system
My solution (it works) is:
$IGNORE_USER="USER1 USER2 USER3"
if [ ! -z "$IGNORE_USER" ]; then
for usr in $IGNORE_USER
do
if $USER = $usr; then
exit_program "bye bye"
fi
done
fi
bash shell-script
asked 13 hours ago
bernhard
212
asked 13 hours ago
bernhard
212
asked 13 hours ago
bernhard
212
asked 13 hours ago
bernhard
212
212
You don't need to loop over your array of users. See here
– xenoid
10 hours ago
If you have a working script and want it reviewed, consider codereview.stackexchange.com
– kojiro
7 hours ago
add a comment |Â
You don't need to loop over your array of users. See here
– xenoid
10 hours ago
If you have a working script and want it reviewed, consider codereview.stackexchange.com
– kojiro
7 hours ago
You don't need to loop over your array of users. See here
– xenoid
10 hours ago
You don't need to loop over your array of users. See here
– xenoid
10 hours ago
If you have a working script and want it reviewed, consider codereview.stackexchange.com
– kojiro
7 hours ago
If you have a working script and want it reviewed, consider codereview.stackexchange.com
– kojiro
7 hours ago
add a comment |Â
2 Answers
2
active
oldest
votes
up vote
7
down vote
That script does not work.
You have a syntax error on the first line in that an assignment to IGNORE_USER should not dereference the variable with $.
There is another syntax error in your if statement. Use [ "string1" = "string2" ] to compare strings.
Your code relies on using $IGNORE_USER unquoted. This splits the string on whitespaces, which is what you want to do. In the most general case, this is not what you want to do, as the items in the list may well contain whitespace characters that should be preserved. The shell would also perform filename generation (globbing) on the values in the string if it's used unquoted.
It would be better to use an array for this as you're dealing with separate items (usernames). In general, whenever you want to treat separate items as separate items, don't put them inside a single string. Doing so would potentially make it hard to distinguish one item from another.
Suggestion:
ignore=( 'user1' 'user2' 'user3' )
for u in "$ignore[@]"; do
if [ "$USER" = "$u" ]; then
exit_program 'bye bye'
fi
done
This assumes that exit_program takes care of exiting the program. If not, add exit afterwards. There is no need to test whether the ignore array is empty as the loop would not run a single iteration if it was.
In the code above, "$ignore[@]" (note the double quotes) would expand to the list of usernames, each username individually quoted and protected from further word splitting and filename generation.
Related:
- The ShellCheck website
For a version that is not specific to bash, but that would run in any POSIX-like shell:
set -- 'user1' 'user2' 'user3'
for u do
if [ "$USER" = "$u" ]; then
exit_program 'bye bye'
fi
done
This uses the list of positional parameters as the list of usernames to ignore instead of an array.
answered 12 hours ago
Kusalananda
100k13199310
add a comment |Â
up vote
3
down vote
grep solution
Use the "-w" feature of grep, to match a word.
Daresay it won't work on older grep implementations, such as Solaris, AIX etc.
echo $IGNORE_USER | grep -qw $USER && exit_program 'bye bye'
Try it online!
bash internal solution
Go entirely bash, and don't rely on grep.
bash doesn't allow a construct of =~ regex unless you run shopt -s compat31 first. So by using the =~ $(echo regex) we can overcome this. We use double quotes in this example, so that $USER gets expanded, and in so doing we need to escape out the b to be \b.
[[ $IGNORE_USER =~ $(echo "\b$USER\b") ]] && exit_program 'bye bye'
Try it online!
answered 8 hours ago
steve
11.8k22046
add a comment |Â
2 Answers
2
active
oldest
votes
2 Answers
2
active
oldest
votes
active
oldest
votes
active
oldest
votes
up vote
7
down vote
That script does not work.
You have a syntax error on the first line in that an assignment to IGNORE_USER should not dereference the variable with $.
There is another syntax error in your if statement. Use [ "string1" = "string2" ] to compare strings.
Your code relies on using $IGNORE_USER unquoted. This splits the string on whitespaces, which is what you want to do. In the most general case, this is not what you want to do, as the items in the list may well contain whitespace characters that should be preserved. The shell would also perform filename generation (globbing) on the values in the string if it's used unquoted.
It would be better to use an array for this as you're dealing with separate items (usernames). In general, whenever you want to treat separate items as separate items, don't put them inside a single string. Doing so would potentially make it hard to distinguish one item from another.
Suggestion:
ignore=( 'user1' 'user2' 'user3' )
for u in "$ignore[@]"; do
if [ "$USER" = "$u" ]; then
exit_program 'bye bye'
fi
done
This assumes that exit_program takes care of exiting the program. If not, add exit afterwards. There is no need to test whether the ignore array is empty as the loop would not run a single iteration if it was.
In the code above, "$ignore[@]" (note the double quotes) would expand to the list of usernames, each username individually quoted and protected from further word splitting and filename generation.
Related:
- The ShellCheck website
For a version that is not specific to bash, but that would run in any POSIX-like shell:
set -- 'user1' 'user2' 'user3'
for u do
if [ "$USER" = "$u" ]; then
exit_program 'bye bye'
fi
done
This uses the list of positional parameters as the list of usernames to ignore instead of an array.
answered 12 hours ago
Kusalananda
100k13199310
add a comment |Â
up vote
7
down vote
That script does not work.
You have a syntax error on the first line in that an assignment to IGNORE_USER should not dereference the variable with $.
There is another syntax error in your if statement. Use [ "string1" = "string2" ] to compare strings.
Your code relies on using $IGNORE_USER unquoted. This splits the string on whitespaces, which is what you want to do. In the most general case, this is not what you want to do, as the items in the list may well contain whitespace characters that should be preserved. The shell would also perform filename generation (globbing) on the values in the string if it's used unquoted.
It would be better to use an array for this as you're dealing with separate items (usernames). In general, whenever you want to treat separate items as separate items, don't put them inside a single string. Doing so would potentially make it hard to distinguish one item from another.
Suggestion:
ignore=( 'user1' 'user2' 'user3' )
for u in "$ignore[@]"; do
if [ "$USER" = "$u" ]; then
exit_program 'bye bye'
fi
done
This assumes that exit_program takes care of exiting the program. If not, add exit afterwards. There is no need to test whether the ignore array is empty as the loop would not run a single iteration if it was.
In the code above, "$ignore[@]" (note the double quotes) would expand to the list of usernames, each username individually quoted and protected from further word splitting and filename generation.
Related:
- The ShellCheck website
For a version that is not specific to bash, but that would run in any POSIX-like shell:
set -- 'user1' 'user2' 'user3'
for u do
if [ "$USER" = "$u" ]; then
exit_program 'bye bye'
fi
done
This uses the list of positional parameters as the list of usernames to ignore instead of an array.
answered 12 hours ago
Kusalananda
100k13199310
add a comment |Â
up vote
7
down vote
up vote
7
down vote
That script does not work.
You have a syntax error on the first line in that an assignment to IGNORE_USER should not dereference the variable with $.
There is another syntax error in your if statement. Use [ "string1" = "string2" ] to compare strings.
Your code relies on using $IGNORE_USER unquoted. This splits the string on whitespaces, which is what you want to do. In the most general case, this is not what you want to do, as the items in the list may well contain whitespace characters that should be preserved. The shell would also perform filename generation (globbing) on the values in the string if it's used unquoted.
It would be better to use an array for this as you're dealing with separate items (usernames). In general, whenever you want to treat separate items as separate items, don't put them inside a single string. Doing so would potentially make it hard to distinguish one item from another.
Suggestion:
ignore=( 'user1' 'user2' 'user3' )
for u in "$ignore[@]"; do
if [ "$USER" = "$u" ]; then
exit_program 'bye bye'
fi
done
This assumes that exit_program takes care of exiting the program. If not, add exit afterwards. There is no need to test whether the ignore array is empty as the loop would not run a single iteration if it was.
In the code above, "$ignore[@]" (note the double quotes) would expand to the list of usernames, each username individually quoted and protected from further word splitting and filename generation.
Related:
- The ShellCheck website
For a version that is not specific to bash, but that would run in any POSIX-like shell:
set -- 'user1' 'user2' 'user3'
for u do
if [ "$USER" = "$u" ]; then
exit_program 'bye bye'
fi
done
This uses the list of positional parameters as the list of usernames to ignore instead of an array.
answered 12 hours ago
Kusalananda
100k13199310
That script does not work.
You have a syntax error on the first line in that an assignment to IGNORE_USER should not dereference the variable with $.
There is another syntax error in your if statement. Use [ "string1" = "string2" ] to compare strings.
Your code relies on using $IGNORE_USER unquoted. This splits the string on whitespaces, which is what you want to do. In the most general case, this is not what you want to do, as the items in the list may well contain whitespace characters that should be preserved. The shell would also perform filename generation (globbing) on the values in the string if it's used unquoted.
It would be better to use an array for this as you're dealing with separate items (usernames). In general, whenever you want to treat separate items as separate items, don't put them inside a single string. Doing so would potentially make it hard to distinguish one item from another.
Suggestion:
ignore=( 'user1' 'user2' 'user3' )
for u in "$ignore[@]"; do
if [ "$USER" = "$u" ]; then
exit_program 'bye bye'
fi
done
This assumes that exit_program takes care of exiting the program. If not, add exit afterwards. There is no need to test whether the ignore array is empty as the loop would not run a single iteration if it was.
In the code above, "$ignore[@]" (note the double quotes) would expand to the list of usernames, each username individually quoted and protected from further word splitting and filename generation.
Related:
- The ShellCheck website
For a version that is not specific to bash, but that would run in any POSIX-like shell:
set -- 'user1' 'user2' 'user3'
for u do
if [ "$USER" = "$u" ]; then
exit_program 'bye bye'
fi
done
This uses the list of positional parameters as the list of usernames to ignore instead of an array.
answered 12 hours ago
Kusalananda
100k13199310
edited 8 hours ago
answered 12 hours ago
Kusalananda
100k13199310
answered 12 hours ago
Kusalananda
100k13199310
answered 12 hours ago
Kusalananda
100k13199310
100k13199310
add a comment |Â
add a comment |Â
up vote
3
down vote
grep solution
Use the "-w" feature of grep, to match a word.
Daresay it won't work on older grep implementations, such as Solaris, AIX etc.
echo $IGNORE_USER | grep -qw $USER && exit_program 'bye bye'
Try it online!
bash internal solution
Go entirely bash, and don't rely on grep.
bash doesn't allow a construct of =~ regex unless you run shopt -s compat31 first. So by using the =~ $(echo regex) we can overcome this. We use double quotes in this example, so that $USER gets expanded, and in so doing we need to escape out the b to be \b.
[[ $IGNORE_USER =~ $(echo "\b$USER\b") ]] && exit_program 'bye bye'
Try it online!
answered 8 hours ago
steve
11.8k22046
add a comment |Â
up vote
3
down vote
grep solution
Use the "-w" feature of grep, to match a word.
Daresay it won't work on older grep implementations, such as Solaris, AIX etc.
echo $IGNORE_USER | grep -qw $USER && exit_program 'bye bye'
Try it online!
bash internal solution
Go entirely bash, and don't rely on grep.
bash doesn't allow a construct of =~ regex unless you run shopt -s compat31 first. So by using the =~ $(echo regex) we can overcome this. We use double quotes in this example, so that $USER gets expanded, and in so doing we need to escape out the b to be \b.
[[ $IGNORE_USER =~ $(echo "\b$USER\b") ]] && exit_program 'bye bye'
Try it online!
answered 8 hours ago
steve
11.8k22046
add a comment |Â
up vote
3
down vote
up vote
3
down vote
grep solution
Use the "-w" feature of grep, to match a word.
Daresay it won't work on older grep implementations, such as Solaris, AIX etc.
echo $IGNORE_USER | grep -qw $USER && exit_program 'bye bye'
Try it online!
bash internal solution
Go entirely bash, and don't rely on grep.
bash doesn't allow a construct of =~ regex unless you run shopt -s compat31 first. So by using the =~ $(echo regex) we can overcome this. We use double quotes in this example, so that $USER gets expanded, and in so doing we need to escape out the b to be \b.
[[ $IGNORE_USER =~ $(echo "\b$USER\b") ]] && exit_program 'bye bye'
Try it online!
answered 8 hours ago
steve
11.8k22046
grep solution
Use the "-w" feature of grep, to match a word.
Daresay it won't work on older grep implementations, such as Solaris, AIX etc.
echo $IGNORE_USER | grep -qw $USER && exit_program 'bye bye'
Try it online!
bash internal solution
Go entirely bash, and don't rely on grep.
bash doesn't allow a construct of =~ regex unless you run shopt -s compat31 first. So by using the =~ $(echo regex) we can overcome this. We use double quotes in this example, so that $USER gets expanded, and in so doing we need to escape out the b to be \b.
[[ $IGNORE_USER =~ $(echo "\b$USER\b") ]] && exit_program 'bye bye'
Try it online!
answered 8 hours ago
steve
11.8k22046
edited 6 hours ago
answered 8 hours ago
steve
11.8k22046
answered 8 hours ago
steve
11.8k22046
answered 8 hours ago
steve
11.8k22046
11.8k22046
add a comment |Â
add a comment |Â
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2funix.stackexchange.com%2fquestions%2f460658%2fbash-how-to-improve-this-script%23new-answer', 'question_page');
);
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
You don't need to loop over your array of users. See here
– xenoid
10 hours ago
If you have a working script and want it reviewed, consider codereview.stackexchange.com
– kojiro
7 hours ago