FILE_CORE_NAME="$(/usr/bin/env basename $(/usr/bin/env awk -F "." '{print $1}' <<< "${FILE_TO_PROCESS}"))"; local FILE_CORE_NAME;
Even when reformatted on several lines for legibility
FILE_CORE_NAME="$(
/usr/bin/env basename $(
/usr/bin/env awk -F "." '{print $1}' <<< "${FILE_TO_PROCESS}"
)
)"
local FILE_CORE_NAME
Is wrong on many accounts:
awk
runs that {print $1}
command for every line of its input, so if you have a $'/a.b/c\nd.e\nf/g.h.i'
file for instance, it will output /a<newline>d<newline>f/g'
. newline being as valid a character as any and filename not being guaranteed to be text, you shouldn't use line-based text utilities to process them.
- as hinted above, even for single-line file paths, if you have a
/foo.d/file.txt
or a /foo.bar.2020.jpg
file, you'll get /foo
instead of /foo.d/file
or /foo.bar.2020
. The basename should be done first, and it's the part up to the last .
that you want to retrieve.
- quotes were omitted around the second
$(...)
, so it's subject to split+glob with all the possible catastrophic consequences if can have.
- the
--
option delimiter is missing after basename
, so if the file path starts with a -
it will fail.
local
is to declare a variable local to the function. However here, you're defining it local
after having modified it, so you've modified the variable of the caller of the function. Then in bash (contrary to ash where the local
builtin comes from), unless the locvar_inherit
option has been enabled, local var
declares the variable local but without any value set.
- all uppercase variable names should be left for environment variable names or at least those with a global scope throughout the script and dependencies.
- I can't see the point of using
/usr/bin/env
here.
Here, the obvious thing to do would be to switch to zsh and do:
local core_name=$FILE_TO_PROCESS:t:r
Where, like in csh from the 70s or vim (and even bash history substitution, but not its parameter expansions), :t
gives you the tail and :r
the root name (with one extension removed).
If you have to use bash
, you can do:
local core_name
core_name=$(basename -- "$FILE_TO_PROCESS") || return
core_name=${core_name%.*}
(which would also work in all Ash like shells that have a local
builtin).
Bearing in mind that $(...)
removes trailing newline characters.
Beware that if $FILE_TO_PROCESS
is for instance /home/me/.zshrc
, $core_name
will become the empty string. You might want to guard against that and some other special values such as -
, .
or ..
depending on what you want to do with that $core_name
in the rest of the function.
env
for example, that makes no sense there). None of what you show seems to be a very good choice but it's hard to give you a better option without more information. I can tell you thatlocal var
will overwrite any value you originally had. – terdon Nov 09 '23 at 13:27local
, what you have on the right-hand side of the assignment isn't relevant. That would be the same issue with justvar=foobar; local var
. OTOH, for the issue of assigning the output of a command to a variable, the thing withlocal
is not relevant. I'm going to assume your issue is withlocal
, not the command substitution, and that's what the existing answer also seems to assume. You could post another question if there's another matter you wondering. – ilkkachu Nov 09 '23 at 13:36local
issue, but we don't know which part of all that is what the OP considers "best practice", so it seems a bit excessive to just completely remove it. They were doing things there such as usingenv
outside a script, that suggest they could use mor help than just focusing onlocal
. – terdon Nov 09 '23 at 13:50awk
andbasename
(standard utilities) are what they seem and you feel that you need to useenv
to get the utilities from portable paths, then you should also usebuiltin local
in place oflocal
, just in case a user has overloaded that name with a function. Personally, I would drop the use ofenv
and leave it to the user to shoot themselves in the feet. – Kusalananda Nov 09 '23 at 14:37local
in the text, too. If you disagree with that, you can just say that, without making me reiterate what I've just said. In any case, I was under the impression there was some sort of an idea that questions should only have one, and as it stands, this one could do with some streamlining. – ilkkachu Nov 09 '23 at 15:25env
within or without a script there, since the post doesn't appear to say where the snippet came from. It's probably not the whole context anyway, sincelocal
doesn't work outside a function (at least not in Bash, which is what the tag says, it might be different in other shells.) – ilkkachu Nov 09 '23 at 15:27