threads / rfc / 16434

RFC patchbisect: teach "skip" to accept special arguments like "A..B"

Subject: [RFC/PATCH] bisect: teach "skip" to accept special arguments like "A..B"

## tl;dr

5 messages between Nov 23, 2008 and Nov 26, 2008. Diffs are folded; open one to read it.

replies: 4people: 4as markdown or json

Christian Couder· Nov 23, 2008, 21:02 UTC · lore

The current "git bisect skip" syntax is "git bisect skip [<rev>...]" so it's already possible to skip a range of revisions using something like:

$ git bisect skip $(git rev-list A..B)
where A and B are the bounds of the range we want to skip.
This patch teaches "git bisect skip" to accept:
$ git bisect skip A..B
as an abbreviation for the former command.

This is done by checking each argument to see if it contains two dots one after the other ('..'), and by expending it using "git rev-list" if that is the case.

Note that this patch will not make "git bisect skip" accept all that "git rev-list" accepts, as things like "^A B" for exemple will not work. But things like "A B..C D E F.. ..G H...I" should work as expected.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 git-bisect.sh |   19 ++++++++++++++++++-
 1 files changed, 18 insertions(+), 1 deletions(-)
	Dscho wrote:
	> Would it not be more intuitive to have support for
	>
        > git bisect skip A..B
	>
	> ?
	Here is a patch to do that. I am not sure it's worth it
	because this is a special case in many ways.
Show changes to git-bisect.sh +18 −1
diff --git a/git-bisect.sh b/git-bisect.sh
index 0d0e278..6706bc1 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -191,6 +191,21 @@ check_expected_revs() {
 	done
 }
 
+bisect_skip() {
+        all=''
+	for arg in "$@"
+	do
+	    case "$arg" in
+            *..*)
+                revs=$(git rev-list "$arg") || die "Bad rev input: $arg" ;;
+            *)
+                revs="'$arg'" ;;
+	    esac
+            all="$all $revs"
+        done
+        bisect_state 'skip' $all
+}
+
 bisect_state() {
 	bisect_autostart
 	state=$1
@@ -630,8 +645,10 @@ case "$#" in
         git bisect -h ;;
     start)
         bisect_start "$@" ;;
-    bad|good|skip)
+    bad|good)
         bisect_state "$cmd" "$@" ;;
+    skip)
+        bisect_skip "$@" ;;
     next)
         # Not sure we want "next" at the UI level anymore.
         bisect_next "$@" ;;
-- 
1.6.0.4.768.g22eb.dirty
Johannes Schindelin· Nov 24, 2008, 00:28 UTC · re: Christian Couder · lore

Re: [RFC/PATCH] bisect: teach "skip" to accept special arguments like "A..B"

Hi,
On Sun, 23 Nov 2008, Christian Couder wrote:
Show 17 quoted lines
> The current "git bisect skip" syntax is "git bisect skip [<rev>...]"
> so it's already possible to skip a range of revisions using
> something like:
> 
> $ git bisect skip $(git rev-list A..B)
> 
> where A and B are the bounds of the range we want to skip.
> 
> This patch teaches "git bisect skip" to accept:
> 
> $ git bisect skip A..B
> 
> as an abbreviation for the former command.
> 
> This is done by checking each argument to see if it contains two
> dots one after the other ('..'), and by expending it using
> "git rev-list" if that is the case.
s/expend/expand/
Show 19 quoted lines
> Note that this patch will not make "git bisect skip" accept all
> that "git rev-list" accepts, as things like "^A B" for exemple
> will not work. But things like "A B..C D E F.. ..G H...I" should
> work as expected.
> 
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
>  git-bisect.sh |   19 ++++++++++++++++++-
>  1 files changed, 18 insertions(+), 1 deletions(-)
> 
> 	Dscho wrote:
> 	> Would it not be more intuitive to have support for
> 	>
>         > git bisect skip A..B
> 	>
> 	> ?
> 
> 	Here is a patch to do that. I am not sure it's worth it
> 	because this is a special case in many ways.
Why not have something like
	skip)
		for arg in $(git rev-list "$@")
		do
			bisect_state skip $arg
		done
?

Ciao, Dscho

Johannes Sixt· Nov 24, 2008, 07:09 UTC · re: Johannes Schindelin · lore

Re: [RFC/PATCH] bisect: teach "skip" to accept special arguments like "A..B"

Johannes Schindelin schrieb:
Show 18 quoted lines
> On Sun, 23 Nov 2008, Christian Couder wrote:
>> 	Dscho wrote:
>> 	> Would it not be more intuitive to have support for
>> 	>
>>         > git bisect skip A..B
>> 	>
>> 	> ?
>>
>> 	Here is a patch to do that. I am not sure it's worth it
>> 	because this is a special case in many ways.
> 
> Why not have something like
> 
> 	skip)
> 		for arg in $(git rev-list "$@")
> 		do
> 			bisect_state skip $arg
> 		done
Because if you say
	$ git bisect skip
this would be incorrectly calling rev-list; but more importantly, if you say
	$ git bisect skip A
then this would skip A *and all its ancestors*. Not quite what you intended.
-- Hannes
Johannes Schindelin· Nov 24, 2008, 10:35 UTC · re: Johannes Sixt · lore

Re: [RFC/PATCH] bisect: teach "skip" to accept special arguments like "A..B"

Hi,
On Mon, 24 Nov 2008, Johannes Sixt wrote:
Show 29 quoted lines
> Johannes Schindelin schrieb:
> > On Sun, 23 Nov 2008, Christian Couder wrote:
> >> 	Dscho wrote:
> >> 	> Would it not be more intuitive to have support for
> >> 	>
> >>         > git bisect skip A..B
> >> 	>
> >> 	> ?
> >>
> >> 	Here is a patch to do that. I am not sure it's worth it
> >> 	because this is a special case in many ways.
> > 
> > Why not have something like
> > 
> > 	skip)
> > 		for arg in $(git rev-list "$@")
> > 		do
> > 			bisect_state skip $arg
> > 		done
> 
> Because if you say
> 
> 	$ git bisect skip
> 
> this would be incorrectly calling rev-list; but more importantly, if you say
> 
> 	$ git bisect skip A
> 
> then this would skip A *and all its ancestors*. Not quite what you intended.
Good point indeed.

Thanks, Dscho

Junio C Hamano· Nov 26, 2008, 05:42 UTC · re: Christian Couder · lore

Re: [RFC/PATCH] bisect: teach "skip" to accept special arguments like "A..B"

Christian Couder <chriscool@tuxfamily.org> writes:
Show 13 quoted lines
> The current "git bisect skip" syntax is "git bisect skip [<rev>...]"
> so it's already possible to skip a range of revisions using
> something like:
>
> $ git bisect skip $(git rev-list A..B)
>
> where A and B are the bounds of the range we want to skip.
>
> This patch teaches "git bisect skip" to accept:
>
> $ git bisect skip A..B
>
> as an abbreviation for the former command.

Although I fully realize that the established semantics of A..B in git is bottom-exclusive, top-inclusive, and this suggestion breaks the UI uniformity by deviating from that convention, I have to wonder if it would be more useful if you let the bottom commit (A in your example) also be skipped.

I would suspect that it would be more useful than the "replace" one, but that is a separate issue.

← back to recent threads