Skip to content

spell/typos+quotes to avoid globbing/wordsplitting#5

Merged
geekcomputers merged 2 commits into
masterfrom
unknown repository
Jan 3, 2023
Merged

spell/typos+quotes to avoid globbing/wordsplitting#5
geekcomputers merged 2 commits into
masterfrom
unknown repository

Conversation

@ghost

@ghost ghost commented Jan 2, 2023

Copy link
Copy Markdown

fixing some spelling errors and adding double quotes to avoid globbing or word splitting

@ghost ghost left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hello. These are just some thoughts I had which might be of use. I hope they at least gives you a different perspective.

Comment thread batch_file_rename.sh Outdated
funct_check_params() # Start of the procedure
{
if [ ${NARG} -lt 3 ]; then # Check there are 3 arguments passed to the script, if not display the help
if [ "${NARG}" -lt 3 ]; then # Check there are 3 arguments passed to the script, if not display the help

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This change shouldn't be needed, because the variable should be an integer. Rather than forcing this within the test, I would make the script more robust by first ensuring it's an integer and catching when it's not.

Comment thread batch_file_rename.sh Outdated
# This will carry out the rename

for file in $(ls $work_dir/*$old_ext); do mv $file `echo $file | sed 's/\(.*\.\)'$old_ext_cut'/\1'$new_ext_cut/` ; done
for file in $(ls "$work_dir"/*"$old_ext"); do mv "$file" $(echo "$file" | sed 's/\(.*\.\)'"$old_ext_cut"'/\1'"$new_ext_cut"/) ; done

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It's almost never recommended or necessary to use ls(1) in a shell script, largely because glob filename pattern matching more than suffices, is more reliable, and is more efficient. In this case, you should be able to:

  for file in "$work_dir"/*"$old_ext"; do ...

Confirm first, in-case there's something unusual I'm overlooking.

@ghost ghost left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

legacy backticks for $(...) notation

@ghost ghost self-requested a review January 3, 2023 02:51
@ghost

ghost commented Jan 3, 2023

Copy link
Copy Markdown

I think it's important to note that the use of backticks or graves for command substitution is not going anywhere any time soon, as far as I know. It may be syntax from days of old, but I believe it still holds relevance.

Variable=`some_command`

In the above example, there is no need for the $(...) syntax. No nesting of command substitution is required, and there are no quotes to confuse between the graves. The added character of the newer notation is therefore redundant.

However,

Variable=$(some_command "string $(other_command)")

In the above example, using the newer syntax makes sense, because it avoids the confusion between the final double-quote and the right-most grave, and nesting is required.

This is just my stance on command substitution syntax in shell programming. I use both, where appropriate.

Actually, there is another important argument for the newer method: many claim that the newer syntax is simply easier to see, regardless of what's next to it. However, this largely seems dependent on the user's font. I believe each user should always have a font suitable for such text, so that each character is sufficiently distinguishable from the rest.

@geekcomputers geekcomputers merged commit 34ec479 into geekcomputers:master Jan 3, 2023
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.

1 participant