Date: Sat, 8 Feb 2025 19:33:10 -0500
From: Greg Wooledge <g...@wooledge.org>
Message-ID: <20250209003310.gm29...@wooledge.org>
| You really want bash's ${var//search/replace} feature for the
| renaming step,
In some cases I might agree, not necessarily about the
mechanism, but about doing the change entirely using shell
primitives, to avoid the costs of forking.
But if we're going to stop and ask the user every time
a rename might need to be done, clearly that need is
gone, and we might as well just do
newname=$( printf '%sX\n' "${oldname}" | tr ' ' _)
newname=${newmame%X}
where the nonsense with X is just in case $oldname happens
to end in a \n. But most likely, if we're replacing spaces
we'd want to be replacing \n as well, and we can just do
newname=$( printf '%s' "${oldname}" | tr $' \n' __)
(whether visible easily or not, there are two underscores).
| so as long as you're already using that,
So, I wouldn't be, so I can continue writing a portable
script.
| For the problem as stated (no recursion), we can discard the "ls"
| command,
Sure, that one I agree with, and makes it much easier, I was just
maintaining the original design as much as possible.
| The only bash extensions we really want, then, are the parameter
| expansion for the renaming step,
which, as above, isn't really needed at all.
| and "read -p" for the prompt.
which could be just a printf without a trailing \n before the
read -- but doesn't need to be as read -p is in the standard.
| We could use case instead of [[ for checking the user's
| input, but why?
Because it works everywhere, and is more flexible.
| We're already using two bash extensions,
You're using 1, and that one isn't needed for the task
(if we needed something more than tr, I'd be using sed).
| and [[ is nicer to use.
seems so perhaps in this simple case, but is less so
when there are more possibilities - like if we wanted
a 'q' reply to read to mean "exit script here" (which
I did with an EOF reply to read - your versions ignore
the exit status from read .. naughty naughty), and we
might want an 'l' reply to mean "make the chars in this
file name visible, so I can see anything odd", and a
'r' command to allow the user to type the new file name
rather than just replacing spaces with underscores.
And ...
If we start out using case, adding more variants is
child's play - using [[ ]] (or anything else using "if")
and things get very messy and ugly very quickly (or
we just discard that code, and switch to using case, in
which case (no pun intended) using the if variant at all
was just a waste of time).
| If I were going to write this in POSIX sh, the read -p could be
| replaced with printf + read,
except we don't need to do that,
| and the [[ with case,
doing that is better anyway, even in bash
| but the name alteration becomes *ugly*.
a little yes.
| new=$(
| printf %s "$f" | tr '[:space:]' _
| printf x
| )
Hmm ... I typed my reply above before I read down this far.
Great minds.... The difference between X and x is irrelevant,
but there's no reason not to pass the 'x' (or X) through tr, it
won't alter it, but yes, I agree using [:space:] might be better
than the ' ' (literal space) I used above, though in the majority
of cases, probably doesn't make a difference (and might even be
doing the wrong thing).
| Forking multiple processes just to replace some characters
| in a string is sad.
Not really. Sometimes inefficient. As I (believe I) have said
before, I *never* write bash scripts (I use bash interactively)
except when testing bash (and usally a bunch of other shells too)
and have never failed to write useful scripts to do whatever I
need (the shell I use has a few POSIX extensions, but I rarely
use them - the most common would be $RANDOM I think, but most
shells support that now).
Fork/exec isn't sad, it is very neat, and extremely versatile,
but it can sometimes be slow. What I do in cases where that
matters is to just use a shell loop, here:
while case $f in
(*' '*) f=${f%% *}_$f{#* };;
(*) break;;
esac
do :; done
or something similar, depending upon the actual needs. In the shell
I use that's quite fast enough (and the shell itself being much smaller
and faster than bash, means unless there are a lot of spaces in a lot
of filenames, the script probably runs faster as well). But, in this
case, as noted above, none of that is needed, script execution speed cannot
be relevant, so the simplest, clearest, most portable, method is best.
[aside: all the literal spaces in the matching above could become [[:space:]]
if desired, that would change the meaning slightly, but still work.]
| P.S. Yeah, I could have skipped the "printf x" and ${new%x} steps here,
| because it's not possible for tr's output to end with a newline (even
| if the original filename does), because the newline is replaced with
| a _ character.
Hmm, yes, so it is, I keep confusing [:space:] and [:blank:] (the latter
doesn't include \n, or a few others). But if the original requirement
was just to replace spaces, then we shouldn't be using the char classes
at all (kind of depends whether unbreakable spaces, etc, are to be included)
And as I included the X trick as well, in general, I agree, it is the
right thing to do in cases like this.
kre
