From: Pierre Habouzit <madcoder@debian•org>
To: Junio C Hamano <gitster@pobox•com>
Cc: git@vger•kernel.org, pasky@suse•cz, srabbelier@gmail•com
Subject: Re: [PATCH] filter-branch: add git_commit_non_empty_tree and --prune-empty.
Date: Mon, 03 Nov 2008 10:27:29 +0100 [thread overview]
Message-ID: <20081103092729.GE13930@artemis.corp> (raw)
In-Reply-To: <7viqr5wgl7.fsf@gitster.siamese.dyndns.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 --]
next prev parent reply other threads:[~2008-11-03 9:28 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-10-30 0:33 [PATCH] git-filter-branch: Add an example on how to remove empty commits Petr Baudis
2008-10-30 0:39 ` Sam Vilain
2008-10-30 0:56 ` Johannes Schindelin
2008-10-30 13:26 ` Pierre Habouzit
2008-10-30 15:06 ` Deskin Miller
2008-10-30 15:10 ` Pierre Habouzit
2008-10-30 16:18 ` filter-branch enhancements Pierre Habouzit
2008-10-30 16:18 ` [PATCH] make git-filter-branch use parse-options Pierre Habouzit
2008-10-30 16:18 ` [Proof of concept PATCH] implement --prune-empty switch for filter-branch Pierre Habouzit
2008-10-31 8:22 ` [PATCH] make git-filter-branch use parse-options Pierre Habouzit
2008-10-31 9:26 ` [PATCH] filter-branch: add git_commit_non_empty_tree and --prune-empty Pierre Habouzit
2008-10-31 22:36 ` Johannes Schindelin
2008-10-31 22:42 ` Pierre Habouzit
2008-11-03 4:58 ` Junio C Hamano
2008-11-03 9:27 ` Pierre Habouzit [this message]
2008-11-03 15:18 ` Pierre Habouzit
2009-01-09 19:29 ` Jay Soffian
2009-01-11 11:18 ` Pierre Habouzit
2009-01-11 13:35 ` Johannes Schindelin
2009-01-11 14:27 ` Pierre Habouzit
2009-01-11 14:40 ` Sverre Rabbelier
2009-01-11 14:55 ` Pierre Habouzit
2009-01-11 15:08 ` Sverre Rabbelier
2009-01-11 20:52 ` Junio C Hamano
2009-01-11 20:55 ` Johannes Schindelin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20081103092729.GE13930@artemis.corp \
--to=madcoder@debian$(echo .)org \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=pasky@suse$(echo .)cz \
--cc=srabbelier@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox