Re: [PATCH] guilt(1): simplifications...

[Pierre Habouzit <madcoder@debian.org>]

[-- Attachment #1: Type: text/plain, Size: 3712 bytes --]

On Thu, Jun 14, 2007 at 10:58:45AM -0400, Josef Sipek wrote:
> On Thu, Jun 14, 2007 at 02:50:58PM +0200, Pierre Habouzit wrote:

> >  # usage: do_make_header <hash>
> >  do_make_header()
> >  {
> > -	# which revision do we want to work with?
> > -	local rev="$1"
> > -
> >  	# we should try to work with commit objects only
> > -	if [ `git-cat-file -t "$rev"` != "commit" ]; then
> > -		echo "Hash $rev is not a commit object" >&2
> > +	if [ `git-cat-file -t "$1"` != "commit" ]; then
> > +		echo "Hash $1 is not a commit object" >&2
> >  		echo "Aborting..." >&2
> >  		exit 2
> >  	fi
> >  
> > -	# get the author line from the commit object
> > -	local author=`git-cat-file -p "$rev" | grep -e '^author ' | head -1`
> > -
> > -	# strip the timestamp & '^author ' string
> > -	author=`echo "$author" | sed -e 's/^author //' -e 's/ [0-9]* [+-]*[0-9][0-9]*$//'`
> > -
> > -	git-cat-file -p "$rev" | awk "
> > -BEGIN{ok=0}
> > -(ok==1){print \$0; print \"\nFrom: $author\"; ok=2; next}
> > -(ok==2){print \$0}
> > -/^\$/ && (ok==0){ok=1}
> > -"
> > +	git-cat-file -p "$1" | sed -e \
> > +		'1,/^$/ {
> > +			/^author/ {
> > +				s/^author /From: /
> > +				s/ [0-9]* [+-]*[0-9][0-9]*$//
> > +				p
> > +			}
> > +			/^$/p
> > +			d
> > +		}'
> 
> You changed the output slightly. The original awk script outputed:
> 
> >>>>>
> the first line of the commit message
> 
> From: foo@....
> 
> remainder of the commit message
> <<<<<
> 
> Yours outputs:
> 
> >>>>>
> From: foo@....
> 
> the entire commit message
> <<<<<
> 
> I'd like to keep the previous format as it makes it easier to grab the first
> line of each patch file to get the "short summary" (assuming you follow the
> kernel/git/guilt commit message conventions). This doesn't make anything in
> guilt easier, it just makes the patch files more friendly to other tools one
> might use. (Especially if they grab the first line and use it as the subject
> in emails.)

  damn, I thought I had checked that thouroughly, I'll see that in
details then. and will give you a new patch.

> > @@ -283,10 +264,11 @@ series_remove_patch()
> >  # usage: series_rename_patch <oldname> <newname>
> >  series_rename_patch()
> >  {
> > -	local old=`echo "$1" | sed -e 's,/,\\\\/,g'`
> > -	local new=`echo "$2" | sed -e 's,/,\\\\/,g'` 
> > +	awk -v old="$1" -v new="$2" \
> > +		'{ if ($0 == old) print new; else print $0 }' \
> > +		"$series.tmp" > "$series"
>  
> Shouldn't that be '"$series" > "$series.tmp"' ?
> 
> > -	sed -i -e "s/^$old\$/$new/" "$series"
> > +	mv "$series.tmp" "$series"
> >  }
> >  
> >  # Beware! This is one of the few (only?) places where we modify the applied
> > @@ -295,10 +277,15 @@ series_rename_patch()
> >  # usage: applied_rename_patch <oldname> <newname>
> >  applied_rename_patch()
> >  {
> > -	local old=`echo "$1" | sed -e 's,/,\\\\/,g'`
> > -	local new=`echo "$2" | sed -e 's,/,\\\\/,g'` 
> > +	awk -v old="$1" -v new="$2" \
> > +			'BEGIN{FS=":"}
> > +			{ if ($1 ~ /^[0-9a-f]*$/ && length($1) == 40 && substr($0, 42) == old)
> > +				print substr($0, 0, 41) new;
> > +			else
> > +				print;
> > +			}' "$applied" > "$applied.new"
>                                                  ^^^^
> 
> > -	sed -i -e "s/^\\([0-9a-f]\\{40\\}\\):$old\$/\\1:$new/" "$applied"
> > +	mv "$applied.tmp" "$applied"
>                     ^^^^
> 
> ..new or .tmp?

  Those are obvious blatant mistakes. THe fixes will be available in the
coming patch as well.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]