Bug#64914: none
Julian Gilbey <J.D.Gilbey@qmul.ac.uk> writes:
> On Sat, Mar 30, 2002 at 04:17:01PM -0500, Adam Di Carlo wrote:
> > It's untested, but I'm about to test it and play around with it,
> > should be uploaded shortly.
>
> Well done!
Thanks. I like shell scripting! It's a guilty pleasure almost.
> Some small comments follow....
>
> > + if [ -f ${FMTDIR}/40jadetex.cnf ] ||
> > + ls ${FMTDIR}/*jadetex.cnf 2>/dev/null; then
> > + :
> > + elif [ -f ${FMTDIR}/40jadetex.cnf.disable ]; then
> > + mv ${FMTDIR}/40jadetex.cnf.disable ${FMTDIR}/40jadetex.cnf
> > + else
> > + OLD=`ls -1 ${FMTDIR}/*jadetex.cnf* | tail -1`
>
> How do you know this will output anything? Better would be:
> OLD="`ls -1 ${FMTDIR}/*jadetex.cnf.* 2>/dev/null | tail -1`"
>
> Note three changes: cnf* -> cnf.* (as this is what you test for
> later),
Um, don't confuse the regexp in the sed with the glob here.
> double quotes (in case it's empty),
Why is that needed? Seems to work fine w/o out...
> and the 2>/dev/null.
Yes, I caught that one already in my testing.
> > + if [ -f ${OLD} ]; then
>
> You would probably be better to test:
> if [ -n "${OLD}" ] && [ -f "${OLD}" ]; then
Thanks, very good review. I also caught that in my testing, opting for
if [ ${OLD} -a -f ${OLD} ]; then
Again, double quotes not needed (note this is a /bin/bash script, not
/bin/sh)
> Only other comment is that using mktexlsr would be better than texhash
> in the postinst and prerm, being the new name for the old script.
Ah, I didn't know that. I'll change it. I just run it with no args?
Again, thanks for the review.
--
...Adam Di Carlo..<adam@onshore-devel.com>...<URL:http://www.onshored.com/>
--
To UNSUBSCRIBE, email to debian-tetex-maint-request@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Reply to: