Anyone like to critique this script for grammatical efficiency and correctness? In particular the line indicated. I'm guessing I can do command substitution better than this. title="test" ls -1 *.mp3 > mp3.txt while read filename do echo $filename song=$title${filename:5:4} echo $song id3ed -isq "`echo $filename`" id3ed -q -s "`echo $song`" "`echo $filename`" <---- id3ed -isq "`echo $filename`" done < mp3.txt
On Thu, 10 Nov 2011 12:43:14 +1100, David Zuccaro <david.zuccaro@optusnet.com.au> wrote:
I'm guessing I can do command substitution better than this. id3ed -isq "`echo $filename`"
In bash, parameter substitution happens within double quotes (but not single quotes), so there is no need to use echo at all. id3ed -isq "$filename" Glenn -- pgp.mit.edu 0x228AC090
On Thu, Nov 10, 2011 at 12:43 PM, David Zuccaro < david.zuccaro@optusnet.com.au> wrote:
Anyone like to critique this script for grammatical efficiency and correctness? In particular the line indicated. I'm guessing I can do command substitution better than this.
title="test" ls -1 *.mp3 > mp3.txt while read filename do
Relying on ls in bash scripts is bad, from memory. It could be aliased to "ls -laF --color" for instance, breaking your script. Far better to just use: for filename in *.mp3 do cheers, / Brett
On 10 November 2011 13:13, Brett Pemberton <brett.pemberton@gmail.com> wrote:
Relying on ls in bash scripts is bad, from memory. It could be aliased to "ls -laF --color" for instance, breaking your script. Far better to just use: for filename in *.mp3 do
There are plenty of ways that parsing 'ls' output will result in breakage, new lines characters in file names being my personal favourite, it's covered pretty well here: http://mywiki.wooledge.org/ParsingLs I recommend anyone who's interested in ensuring they're writing robust and correct bash code read through as much of the guide as they can manage though: http://mywiki.wooledge.org/BashGuide Cheers, Dave
David Zuccaro wrote:
Anyone like to critique this script for grammatical efficiency and correctness? In particular the line indicated. I'm guessing I can do command substitution better than this.
title="test" ls -1 *.mp3 > mp3.txt while read filename do echo $filename song=$title${filename:5:4} echo $song id3ed -isq "`echo $filename`" id3ed -q -s "`echo $song`" "`echo $filename`" <---- id3ed -isq "`echo $filename`" done < mp3.txt
export TITLE=${TITLE:-unknown} find -iname \*.mp3 -maxdepth 1 \ -exec bash -xc 'id3ed -isq "$0"' {} \; \ -exec bash -xc 'id3ed -q -s "$TITLE${0:5:4}" "$0"' {} \; \ -exec bash -xc 'id3ed -isq "$0"' {} \; \ -ls Using :5:4 seems a little optimistic, I would recommend instead e.g. $ x=foo-bar-baz.mp3 $ y=${x%.mp3}; a=${y%%-*} c=${y##*-} b=${y%-*}; b=${b#*-} $ echo $a, $b, $c foo, bar, baz
On Thu, 2011-11-10 at 13:16 +1100, Trent W. Buck wrote:
export TITLE=${TITLE:-unknown}
Why not: export TITLE=unknown
find -iname \*.mp3 -maxdepth 1 \ -exec bash -xc 'id3ed -isq "$0"' {} \; \ -exec bash -xc 'id3ed -q -s "$TITLE${0:5:4}" "$0"' {} \; \ -exec bash -xc 'id3ed -isq "$0"' {} \; \ -ls
Why the need for the -ls option? Thanks a lot.
On 2011-11-11 00:22, David Zuccaro wrote:
On Thu, 2011-11-10 at 13:16 +1100, Trent W. Buck wrote:
export TITLE=${TITLE:-unknown}
Why not: export TITLE=unknown
Because Trent's version lets you explicily set TITLE earlier (e.g. before invoking the script), if you want to override the default of 'unknown.'
find -iname \*.mp3 -maxdepth 1 \ -exec bash -xc 'id3ed -isq "$0"' {} \; \ -exec bash -xc 'id3ed -q -s "$TITLE${0:5:4}" "$0"' {} \; \ -exec bash -xc 'id3ed -isq "$0"' {} \; \ -ls
Why the need for the -ls option?
Only for use as a type of progress indicator to see which file has just been processed. -- Regards, Matthew Cengia
On Fri, Nov 11, 2011 at 09:47:10AM +1100, Matthew Cengia wrote:
On 2011-11-11 00:22, David Zuccaro wrote:
On Thu, 2011-11-10 at 13:16 +1100, Trent W. Buck wrote:
export TITLE=${TITLE:-unknown}
Why not: export TITLE=unknown
Because Trent's version lets you explicily set TITLE earlier (e.g. before invoking the script), if you want to override the default of 'unknown.'
yep. another minor variation is to write it something like the following, so that the script can take the default TITLE as the first ARG on the command line, defaulting to 'unknown' if not specified. export TITLE=${1:-unknown}
find -iname \*.mp3 -maxdepth 1 \ -exec bash -xc 'id3ed -isq "$0"' {} \; \ -exec bash -xc 'id3ed -q -s "$TITLE${0:5:4}" "$0"' {} \; \ -exec bash -xc 'id3ed -isq "$0"' {} \; \ -ls
Why the need for the -ls option?
Only for use as a type of progress indicator to see which file has just been processed.
it's hard to see any benefit in using find here, though. with -maxdepth 1, you may as well just use 'for filename in *.mp3; do...', and have a much easier to read script for filename in *.mp3 ; do id3ed -isq "$filename" id3ed -q -s "$TITLE${0:5:4}" "$filename" id3ed -isq "$filename" echo "$filename" done but don't mind me. i'm biased against using find's -exec and go out of my way to use xargs or for or anything else instead, because i think -exec further uglifies the already grotesque horror of shell quoting into something of almost Lovecraftian proportions - the mere sight likely to drive anyone insane. craig ps: i still think there are better tools then id3ed for this job. lltag, for instance. -- craig sanders <cas@taz.net.au> BOFH excuse #13: we're waiting for [the phone company] to fix that line
Craig Sanders wrote:
yep. another minor variation is to write it something like the following, so that the script can take the default TITLE as the first ARG on the command line, defaulting to 'unknown' if not specified.
export TITLE=${1:-unknown}
Nod, I tend to adopt the style I used because the usage then becomes logical (i.e. by name) rather than positional, so that when you have umpteen variables, the user can specify exactly/only the ones they care about. getopts (bashism) is the other way I often use, but it's inherently limited to single-char variable names, which is a bit of a downer. See start of http://cyber.com.au/~twb/.bin/ru for example.
it's hard to see any benefit in using find here, though. with -maxdepth 1, you may as well just use 'for filename in *.mp3; do...', and have a much easier to read script
for filename in *.mp3 ; do id3ed -isq "$filename" id3ed -q -s "$TITLE${0:5:4}" "$filename" id3ed -isq "$filename" echo "$filename" done
Agreed; I'd have done it that way if I'd initially noticed he didn't want to recurse; when I did, I just lazily added maxdepth.
On Fri, Nov 11, 2011 at 01:36:05PM +1100, Trent W. Buck wrote:
getopts (bashism) is the other way I often use, but it's inherently limited to single-char variable names, which is a bit of a downer.
yep, downer is right. IMO the built-in getopts is inadequate. i've written lots of stuff using /usr/bin/getopt from the util-linux package. supports both long and short options, and pretty easy to use. similar enough to getopts usage that it should be no problem to pick up. e.g. from my myth-list-queue.sh(*) script: TEMP=$(getopt -o 'drqcehfts:AD' --long 'debug,running,queued,completed,errors,flag,transcode,sort:,ASC,DESC' -n "$0" -- "$@") in fact, i use it for anything needing more than just a list of filename(s) on the command line. (*) one of these days i should publish my myth utility scripts. craig -- craig sanders <cas@taz.net.au>
On Thu, Nov 10, 2011 at 12:43:14PM +1100, David Zuccaro wrote:
id3ed -isq "`echo $filename`"
FYI there are numerous id3 tag editors which have built-in filename parsing, using either built-in or user-defined filename templates. a few years back when i was still using mp3, i used to use one called lltag[1]. it could be run in batch mode to automatically set the tags based on the filename, or interactively to prompt you with its best guess for each file and allow you to approve or edit its suggestion. it's actually a front-end to mp3info[2] (and also supports ogg and flac via vorbiscomment and metaflac) [1] http://home.gna.org/lltag/ [2] http://www.ibiblio.org/mp3info/ either of these will almost certainly do a better job than trying to write your own filename parser in bash. and for a GUI tag editor, i always thought that the original amarok 1.x user interface was the best available. Then amarok 2 was released and it sucked. Clementine[3] (a clone of the old amarok 1.x) is almost as good as the old amarok. ex-falso / quod-libet[4] also had excellent tag-editing abilities. EasyTAG[5] looked pretty good. [3] http://www.clementine-player.org/ [4] http://code.google.com/p/quodlibet/ [5] http://easytag.sourceforge.net/ there are many other CLI & GUI tag editors. all of these are packaged for debian, and presumably for debian-derivatives like ubuntu. probably for other distros too. craig -- craig sanders <cas@taz.net.au> BOFH excuse #87: Password is too complex to decrypt
participants (7)
-
Brett Pemberton -
Craig Sanders -
David Schoen -
David Zuccaro -
Glenn McIntosh -
Matthew Cengia -
Trent W. Buck