Re: [PATCH v2 2/3] stash: Allow git stash branch to process commits that look like stashes but are not stash references.

3 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH v2 2/3] stash: Allow git stash branch to process commits that look like stashes but are not stash references.

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:49:15

Jon Seymour [off-list ref] writes:
This patch allows git stash branch to work with stash-like commits created by git stash create.

Two changes were required:

* relax the pre-condition so that a stash stack is required if and only if a stash argument is not specified
* don't attempt to drop a stash argument that doesn't look like a stash reference.


Signed-off-by: Jon Seymour <redacted>
Please wrap very long lines.
quoted hunk
diff --git a/git-stash.sh b/git-stash.sh
index 1d95447..432ddae 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -225,6 +225,12 @@ show_stash () {
 	git diff $flags $b_commit $w_commit
 }
 
+if_stash_ref() {
+	ref="$1"
+	shift
+	test "${ref#stash}" = "${ref}" -a "${ref#$ref_stash}" = "${ref}" || "$@"
+}
The interface to this function looks a rather bad taste to me; wouldn't it
look more natural if the callers can say:

	if stash_ref $it
        then
        	do this
	fi

Your criteria used here is that the given parameter does not begin with
"stash" nor "refs/stash".  If it begins with either of these two strings,
the "test" fails and "$@" is run.  Wouldn't this produce a false hit if
you kept a handcrafted stash-looking commit with a tag "stash-42" or
something?

It may make more sense to give "stash drop" an option to be silent if
the given parameter is not on the list to begin with, perhaps?

Re: [PATCH v2 2/3] stash: Allow git stash branch to process commits that look like stashes but are not stash references.

From: Jon Seymour <hidden>
Date: 2016-06-15 22:49:15

Junio,

Thanks for the feedback. I'll rework along the lines you suggest. If
it makes sense to make the other stash commands tolerant of non-stash
entry references I'll add tests, support and documentation for that.

jon.

On Thu, August 2010 at 9:51 AM, Junio C Hamano [off-list ref] wrote:
Jon  Seymour [off-list ref] writes:
quoted
This patch allows git stash branch to work with stash-like commits created by git stash create.

Two changes were required:

* relax the pre-condition so that a stash stack is required if and only if a stash argument is not specified
* don't attempt to drop a stash argument that doesn't look like a stash reference.


Signed-off-by: Jon Seymour <redacted>
Please wrap very long lines.
quoted
diff --git a/git-stash.sh b/git-stash.sh
index 1d95447..432ddae 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -225,6 +225,12 @@ show_stash () {
      git diff $flags $b_commit $w_commit
 }

+if_stash_ref() {
+     ref="$1"
+     shift
+     test "${ref#stash}" = "${ref}" -a "${ref#$ref_stash}" = "${ref}" || "$@"
+}
The interface to this function looks a rather bad taste to me; wouldn't it
look more natural if the callers can say:

       if stash_ref $it
       then
               do this
       fi

Your criteria used here is that the given parameter does not begin with
"stash" nor "refs/stash".  If it begins with either of these two strings,
the "test" fails and "$@" is run.  Wouldn't this produce a false hit if
you kept a handcrafted stash-looking commit with a tag "stash-42" or
something?

It may make more sense to give "stash drop" an option to be silent if
the given parameter is not on the list to begin with, perhaps?

Re: [PATCH v2 2/3] stash: Allow git stash branch to process commits that look like stashes but are not stash references.

From: Jon Seymour <hidden>
Date: 2016-06-15 22:49:15

One question about test patches. Are you ok with test_expect_failure
tests that document the expected failure of a feature yet to be
developed, followed by the feature, followed by the patch that makes
the tests into test_expect_success tests, or would you prefer to see
the pre- and post- test patches rolled into a single test that is
delivered after the feature patch?

On Thu, Aug 5, 2010 at 3:23 PM, Jon Seymour [off-list ref] wrote:
Junio,

Thanks for the feedback. I'll rework along the lines you suggest. If
it makes sense to make the other stash commands tolerant of non-stash
entry references I'll add tests, support and documentation for that.

jon.

On Thu, August 2010 at 9:51 AM, Junio C Hamano [off-list ref] wrote:
quoted
Jon  Seymour [off-list ref] writes:
quoted
This patch allows git stash branch to work with stash-like commits created by git stash create.

Two changes were required:

* relax the pre-condition so that a stash stack is required if and only if a stash argument is not specified
* don't attempt to drop a stash argument that doesn't look like a stash reference.


Signed-off-by: Jon Seymour <redacted>
Please wrap very long lines.
quoted
diff --git a/git-stash.sh b/git-stash.sh
index 1d95447..432ddae 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -225,6 +225,12 @@ show_stash () {
      git diff $flags $b_commit $w_commit
 }

+if_stash_ref() {
+     ref="$1"
+     shift
+     test "${ref#stash}" = "${ref}" -a "${ref#$ref_stash}" = "${ref}" || "$@"
+}
The interface to this function looks a rather bad taste to me; wouldn't it
look more natural if the callers can say:

       if stash_ref $it
       then
               do this
       fi

Your criteria used here is that the given parameter does not begin with
"stash" nor "refs/stash".  If it begins with either of these two strings,
the "test" fails and "$@" is run.  Wouldn't this produce a false hit if
you kept a handcrafted stash-looking commit with a tag "stash-42" or
something?

It may make more sense to give "stash drop" an option to be silent if
the given parameter is not on the list to begin with, perhaps?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help