[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#64914: none



On Mon, Apr 01, 2002 at 11:34:41AM -0500, Adam Di Carlo wrote:
> > 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.

Ah, yes, missed that.  So you should probably change the regexp in the
sed to \.cnf.* (although it's not really necessary).

> > double quotes (in case it's empty),
> 
> Why is that needed?  Seems to work fine w/o out...

Don't know whether it is, but it seems to do no harm.

> > 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)

True, but testing: if [ -a -f ]; then ... is just plain ugly (which is
what will happen if $OLD is empty).  I'd really encourage using
quotes; it does no harm, and might well save you one day!

I'm also not sure whether test is clever enough to short-circuit the
-f test when using -a, but it doesn't really matter.

> > 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?

$ ls -l /usr/bin/texhash
lrwxrwxrwx    1 root     root            8 Mar 19 18:28 \
  /usr/bin/texhash -> mktexlsr

If you run it with no args, it just rebuilds all of the databases.
That's probably the easiest thing to do.

> Again, thanks for the review.

Pleasure!

   Julian

-- 
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-

     Julian Gilbey, Dept of Maths,             Debian GNU/Linux Developer
      Queen Mary, Univ. of London         see http://people.debian.org/~jdg/
   http://www.maths.qmul.ac.uk/~jdg/           or http://www.debian.org/
        Visit http://www.thehungersite.com/ to help feed the hungry


-- 
To UNSUBSCRIBE, email to debian-tetex-maint-request@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org



Reply to: