[m-rev.] diff: rename get predicates

[Peter Moulder]

On Fri, Sep 30, 2005 at 06:07:14PM +1000, Zoltan Somogyi wrote:
> tools/subst:
> 	A simple tool for performing substitutions on the source files of the
> 	compiler.
...
> +++ tools/subst	27 Sep 2005 06:39:20 -0000
> @@ -0,0 +1,18 @@
> +#!/bin/sh
> +
> +mkdir NEW
> +
> +for file in *.pp
> +do
> +	echo $file
> +	sed -f SUBST $file > NEW/$file
> +	rm `basename $file`.m

This basename invocation is a noop (assuming that $file has no spaces or
other shell special characters): file is guaranteed to be in the current
directory, so basename foo.pp gives foo.pp.

I'd guess that the intent is

  built_file=`basename "$file" .pp`
  rm -f "$built_file"

It's worthwhile for saved shell scripts to do proper quoting ("$file"
throughout), even if only to influence other people's scripts that might
be used in cases where it's important to quote.

Exiting non-zero on error might be useful too (set -e).

It might be useful to exit with an error if [ $# != 0 ].

I think the behaviour when NEW already exists should be documented in a
comment if we retain the existing behaviour.  The most common case is
that it contains exactly the set of files that are to be written in this
run, but the script won't detect the case that it contains other files,
which might be a problem.  Current behaviour favours caller convenience
for common case; alternatives are simply exiting non-zero (happens
automatically if the `set -e' precedes mkdir) or asking the user what to
do if [ -e NEW ] (perhaps explicitly doing rm -rf if user wants to go
ahead, to avoid having unwanted files there or to handle the case that
NEW exists and is not a directory).

It might also be useful for the documentation to draw attention to the
.pp/.m behaviour (e.g. removing the .m file).  It can be brief, like
`# Note the behaviour when foo.pp and foo.m both exist.'.  One benefit
is that it reminds the user of a trap to avoid if the user wants to do
some manipulation without using the subst script.

pjrm.
--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------