[prev in list] [next in list] [prev in thread] [next in thread] 

List:       git
Subject:    Re: [PATCH] filter-branch: Grok special characters in tag names
From:       Johannes Sixt <johannes.sixt () telecom ! at>
Date:       2008-08-21 19:07:33
Message-ID: 200808212107.33474.johannes.sixt () telecom ! at
[Download RAW message or body]

On Donnerstag, 21. August 2008, Brandon Casey wrote:
> Johannes Sixt wrote:
> > diff --git a/git-filter-branch.sh b/git-filter-branch.sh
> > index 2688254..81392ad 100755
> > --- a/git-filter-branch.sh
> > +++ b/git-filter-branch.sh
> > @@ -412,15 +412,17 @@ if [ "$filter_tag_name" ]; then
> >  		echo "$ref -> $new_ref ($sha1 -> $new_sha1)"
> >
> >  		if [ "$type" = "tag" ]; then
> > -			new_sha1=$(git cat-file tag "$ref" |
> > +			new_sha1=$( ( printf 'object %s\ntype commit\ntag %s\n' \
> > +						"$new_sha1" "$new_ref"
> > +				git cat-file tag "$ref" |
> >  				sed -n \
> >  				    -e "1,/^$/{
> > -					  s/^object .*/object $new_sha1/
> > -					  s/^type .*/type commit/
> > -					  s/^tag .*/tag $new_ref/
> > +					  /^object /d
> > +					  /^type /d
> > +					  /^tag /d
>
> Junio complained that my initial version of this was fragile which has
> similarities with the above. Initially, I was blindly changing the first
> line to contain "object...", second line to "type...", etc.

I don't see what's wrong with that. "object" must be the first line anyway, 
otherwise git mktag chokes.

> Would something like the following be equivalent _and_ clearer? Emphasis on
> "and" because both are necessary, not because I strongly feel it to be so.
>
> diff --git a/git-filter-branch.sh b/git-filter-branch.sh
> index a324cf0..11c5c04 100755
> --- a/git-filter-branch.sh
> +++ b/git-filter-branch.sh
> @@ -419,9 +419,12 @@ if [ "$filter_tag_name" ]; then
>  			new_sha1=$(git cat-file tag "$ref" |
>  				sed -n \
>  				    -e "1,/^$/{
> -					  s/^object .*/object $new_sha1/
> -					  s/^type .*/type commit/
> -					  s/^tag .*/tag $new_ref/
> +					  /^object .*/c\\
> +object $new_sha1
> +					  /^type .*/c\\
> +type commit
> +					  /^tag .*/c\\
> +tag $new_ref
>  					}" \
>  				    -e '/^-----BEGIN PGP SIGNATURE-----/q' \
>  				    -e 'p' |

This looks fine, too. (The '.*' in /^foo .*/c\\ is unnecessary.)

-- Hannes
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
[prev in list] [next in list] [prev in thread] [next in thread]