Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Don't clobber NOGLOB in functions #7

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Don't clobber NOGLOB in functions #7

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 24 additions & 15 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,12 @@ without leading/trailing white-space and with truncated spaces.
trim_all() {
# Usage: trim_all " example string "

# Disable globbing to make the word-splitting below safe.
set -f
# Save and disable the globbing state to make the word-splitting
# below safe.
case $- in
*f*) DIDGLOB=false; ;;
*) DIDGLOB=true; set -f; ;;
esac

# Set the argument list to the word-splitted string.
# This removes all leading/trailing white-space and reduces
Expand All @@ -189,8 +193,8 @@ trim_all() {
# Print the argument list as a string.
printf '%s\n' "$*"

# Re-enable globbing.
set +f
# Re-set globbing to it's previous condition
$DIDGLOB && set +f
Copy link

Choose a reason for hiding this comment

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

I wouldn't use && here as it confuses set -e environments in case $DIDGLOB is false. Another issue is this pollutes environment with variable DIDGLOB which might cause issues (either by overwriting an existing variable or by initializing it too early or whatsoever) - I know this is a general issue in sh apps, but I'd avoid it if possible or used trim_all()( ... ) instead of trim_all(){} or at least checked (set lists all currently set vars and function_names) whether the variable is already set or not.

Just my 2 cents 😉.

Copy link

Choose a reason for hiding this comment

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

It is simpler to understand, if you provide the example code. $DIDGLOB is also semi-terrible, because $DIDGLOB2 may also exist in nested contexts. Better use ${DIDGLOB}.

}
```

Expand Down Expand Up @@ -273,9 +277,12 @@ This is an alternative to `cut`, `awk` and other tools.

```sh
split() {
# Disable globbing.
# This ensures that the word-splitting is safe.
set -f
# Save and disable the globbing state to make the word-splitting
# below safe.
case $- in
*f*) DIDGLOB=false; ;;
*) DIDGLOB=true; set -f; ;;
esac

# Store the current value of 'IFS' so we
# can restore it later.
Expand All @@ -299,9 +306,8 @@ split() {
# Restore the value of 'IFS'.
IFS=$old_ifs

# Re-enable globbing.
set +f
}
# Re-set globbing to it's previous condition
$DIDGLOB && set +f
```

**Example Usage:**
Expand Down Expand Up @@ -329,9 +335,12 @@ $ split "1, 2, 3, 4, 5" ", "
trim_quotes() {
# Usage: trim_quotes "string"

# Disable globbing.
# This makes the word-splitting below safe.
set -f
# Save and disable the globbing state to make the word-splitting
# below safe.
case $- in
*f*) DIDGLOB=false; ;;
*) DIDGLOB=true; set -f; ;;
esac

# Store the current value of 'IFS' so we
# can restore it later.
Expand All @@ -358,8 +367,8 @@ trim_quotes() {
# Restore the value of 'IFS'.
IFS=$old_ifs

# Re-enable globbing.
set +f
# Re-set globbing to it's previous condition
$DIDGLOB && set +f
}
```

Expand Down