Re: [Quilt-dev] Annotate any version of a file

[Jean Delvare]

Hi Andreas,

> Here's a cleaned-up version.

Nice, although mixing the cleanups with the feature addition doesn't
help reviewing. Also, I have already committed the two minor fixed we
agreed upon, so this patch doesn't apply cleanly to current CVS.

A couple minor comments and questions:

> +-p   Stop checking for changes at the specified rather than the
> +     topmost patch.

That should be "-p patch" instead of just "-p".

> +     [ -s "$new_file" ] || new_file=/dev/null

Can this actually happen? This would mean that the annotated file would
be empty anyway, right?

> +     old_file="$(backup_file_name $patch "$opt_file")"
> (...)
> +     if [ "$opt_patch" = $patch ]

Shouldn't $patch be quoted? BTW, why do we need these quotes? In case
the variable contains odd characters such as spaces, or only in case the
variable is empty?

> -     sed -e 's:^:'$'\t'':' "$opt_file"
> +     sed -e 's:^:'$'\t'':' "address@hidden"

Yup, this reveals a bug in my original submission, that corner case
wasn't properly handled.

> -     echo "$((n+1))"$'\t'"$(print_patch ${patches[$n]})"
> +     echo "$((n+1))"$'\t'"$(print_patch ${patches[n]})"

I didn't know this simplified syntax was allowed. Are you sure it'll
work with older versions of bash though?

Oh, one last thing: this new code seems to be slightly faster than the
original, which is great :)

Thanks,
-- 
Jean Delvare