quilt-dev
[Top][All Lists]
Advanced

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

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


From: Andreas Gruenbacher
Subject: Re: [Quilt-dev] Annotate any version of a file
Date: Mon, 12 Sep 2005 02:25:18 +0200
User-agent: KMail/1.7.1

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'':' "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?

I don't know.

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

Good.

-- Andreas.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]