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'':' "[EMAIL PROTECTED]"
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
_______________________________________________
Quilt-dev mailing list
[email protected]
http://lists.nongnu.org/mailman/listinfo/quilt-dev
