Re: [PATCH] filter-branch: add git_commit_non_empty_tree and --prune-empty.

[Pierre Habouzit <madcoder@debian.org>]

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

On Mon, Nov 03, 2008 at 04:58:44AM +0000, Junio C Hamano wrote:
> Pierre Habouzit <madcoder@debian.org> writes:
> 
> > +case "$prune_empty,$filter_commit" in
> > +',')
> > +	filter_commit='git commit-tree "$@"';;
> > +'t,')
> > +	filter_commit="$functions;"' git_commit_non_empty_tree "$@"';;
> > +','*)
> > +	;;
> > +*)
> > +	die "Cannot set --prune-empty and --filter-commit at the same time"
> > +esac
> 
> This is only style issue, but I find the above extremely difficult to
> read.  If it were either:
> 
> 	case ... in
>         ,) do "neither set case" ;;
>         t,) do "prune but not filter case" ;;
>         *) do "both set case" ;;
>         esac
> 
> or (rather amateurish but conveys what it wants to do more clearly):
>         
> 	case ... in
>         '','') do "neither set case" ;;
>         t,'') do "prune but not filter case" ;;
>         t,t) do "both set case" ;;
>         esac
> 
> I wouldn't have to wonder which sq pairs with which one.

agreed.

> > diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
> > index b0a9d7d..352b56b 100755
> > --- a/t/t7003-filter-branch.sh
> > +++ b/t/t7003-filter-branch.sh
> > @@ -262,4 +262,12 @@ test_expect_success 'Tag name filtering allows slashes in tag names' '
> >  	test_cmp expect actual
> >  '
> >  
> > +test_expect_success 'Prune empty commits' '
> > +	make_commit to_remove &&
> > +	(git rev-list HEAD | grep -v $(git rev-parse HEAD)) > expect &&
> 
> I am not sure what this one is doing.
> 
>  - Isn't this the same as "git rev-list HEAD^"?
>  - Do you need a subshell?

The filter-branch is supposed to prune the last commit done (current
HEAD) from the revision list. So I build the rev-list we're supposed to
have in the end, and remove the matching ref from it. I don't see how to
avoid the subshell though, but if someone knows better please do :)

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

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