On Sunday 11 September 2005 14:36, Jean Delvare wrote:
> 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".
Yes.
> > + [ -s "$new_file" ] || new_file=/dev/null
>
> Can this actually happen? This would mean that the annotated file would
> be empty anyway, right?
The file could be deleted then added again.
> > + old_file="$(backup_file_name $patch "$opt_file")"
> > (...)
> > + if [ "$opt_patch" = $patch ]
>
> Shouldn't $patch be quoted?
Would be better to protect special characters, yes.
> 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?
Both. (In this case $patch can't be 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?
I don't know.
> Oh, one last thing: this new code seems to be slightly faster than the
> original, which is great :)
Good.
-- Andreas.
_______________________________________________
Quilt-dev mailing list
[email protected]
http://lists.nongnu.org/mailman/listinfo/quilt-dev
