Re: [PATCH v7 2/8] cherry-pick: treat CHERRY_PICK_HEAD and REVERT_HEAD as refs
- From
David Turner <dturner@twopensource.com>
- Date
- Jul 8, 2015, 23:41 UTC
- Message-ID
- <1436398884.4542.25.camel@twopensource.com>
- In-Reply-To
- <559D92CF.7000408@kdbg.org>
On Wed, 2015-07-08 at 23:14 +0200, Johannes Sixt wrote:
Show 50 quoted lines
> Am 08.07.2015 um 21:16 schrieb David Turner: > > On Wed, 2015-07-08 at 19:46 +0200, Johannes Sixt wrote: > >> Am 08.07.2015 um 02:55 schrieb David Turner: > >>> Instead of directly writing to and reading from files in > >>> $GIT_DIR, use ref API to interact with CHERRY_PICK_HEAD > >>> and REVERT_HEAD. > >>> > >>> Signed-off-by: David Turner <dturner@twopensource.com> > >>> --- > >> ... > >>> diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh > >>> index 366f0bc..e2c5583 100644 > >>> --- a/contrib/completion/git-prompt.sh > >>> +++ b/contrib/completion/git-prompt.sh > >>> @@ -415,9 +415,9 @@ __git_ps1 () > >>> fi > >>> elif [ -f "$g/MERGE_HEAD" ]; then > >>> r="|MERGING" > >>> - elif [ -f "$g/CHERRY_PICK_HEAD" ]; then > >>> + elif git rev-parse --quiet --verify "CHERRY_PICK_HEAD" >/dev/null; then > >>> r="|CHERRY-PICKING" > >>> - elif [ -f "$g/REVERT_HEAD" ]; then > >>> + elif git rev-parse --quiet --verify "REVERT_HEAD" >/dev/null; then > >>> r="|REVERTING" > >>> elif [ -f "$g/BISECT_LOG" ]; then > >>> r="|BISECTING" > >> > >> We are trying very hard not to spawn any new processes in __git_ps1(). > >> So, I raise a moderate veto against this hunk. > > > > Do you have an alternate suggestion about how to accomplish the same > > thing? Here are my ideas: > > > > We could special-case CHERRY_PICK_HEAD and REVERT_HEAD to be files > > independent of the ref backend, but that tends to complicate the > > backends. I think this is a mistake. > > > > We could reduce the number from two to one by providing a new > > git-am-status command which outputs one of "CHERRY-PICKING", > > "REVERTING", or "" (or maybe it would also handle rebase and am). We > > could also generalize it to "git-prompt-helper" or something by moving > > that entire bunch of if statements inside. This would replace calls to > > "git describe". > > > > But you probably have a better idea. > > Isn't it mere coincidence that the content of these two files looks like > a non-packed ref? Wouldn't it be better to consider the two akin to > MERGE_HEAD (which is not a ref because it records more than just a > commit name)?
There appears to be a bit of code that assumes they can be read as refs, and the tests test this functionality. Indeed, I wrote most of this code by replacing the backend and fixing failing tests.
At least some people seem to use the rev-ness of CHERRY_PICK_HEAD: https://mnaoumov.wordpress.com/2014/12/03/bulk-cherry-picking-process-and-magic-ref-cherry_pick_head/
So I think it's best to keep the feature.