Re: [PATCHv3] replace: parse revision argument for -d
Jeff King venit, vidit, dixit 12.11.2012 21:42:
On Mon, Nov 12, 2012 at 03:18:02PM +0100, Michael J Gruber wrote:
'git replace' parses the revision arguments when it creates replacements
(so that a sha1 can be abbreviated, e.g.) but not when deleting
replacements.
Make it parse the argument to 'replace -d' in the same way.
Signed-off-by: Michael J Gruber g...@drmicha.warpmail.net
---
Notes:
v3 safeguards the hex buffer against reuse
Thanks, I don't see any other functional problems.
diff --git a/builtin/replace.c b/builtin/replace.c
index e3aaf70..33e6ec3 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -46,24 +46,28 @@ typedef int (*each_replace_name_fn)(const char *name,
const char *ref,
static int for_each_replace_name(const char **argv, each_replace_name_fn fn)
{
-const char **p;
+const char **p, *q;
I find this readable today, but I wonder if in six months we will wonder
what in the world q means. Maybe short_refname or something would be
appropriate?
That would be sooo inappropriate! ;)
Maybe full_hex?
I should also do away with the first replacement which really made sense
in v1 only.
v4 to follow.
Michael
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Notes in format-patch (was: Re: [PATCHv3] replace: parse revision argument for -d)
Michael J Gruber venit, vidit, dixit 12.11.2012 15:18: 'git replace' parses the revision arguments when it creates replacements (so that a sha1 can be abbreviated, e.g.) but not when deleting replacements. Make it parse the argument to 'replace -d' in the same way. Signed-off-by: Michael J Gruber g...@drmicha.warpmail.net --- Notes: v3 safeguards the hex buffer against reuse builtin/replace.c | 16 ++-- t/t6050-replace.sh | 11 +++ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/builtin/replace.c b/builtin/replace.c By the way - Junio, is that the intented outcome of format-patch --notes? I would rather put the newline between the note and the diffstat (and omit the one after the ---) but may have goofed up a rebase: ... Signed-off-by: Michael J Gruber g...@drmicha.warpmail.net --- Notes: v3 safeguards the hex buffer against reuse builtin/replace.c | 16 ++-- ... -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Notes in format-patch (was: Re: [PATCHv3] replace: parse revision argument for -d)
On Tue, Nov 13, 2012 at 11:30:19AM +0100, Michael J Gruber wrote: Michael J Gruber venit, vidit, dixit 12.11.2012 15:18: 'git replace' parses the revision arguments when it creates replacements (so that a sha1 can be abbreviated, e.g.) but not when deleting replacements. Make it parse the argument to 'replace -d' in the same way. Signed-off-by: Michael J Gruber g...@drmicha.warpmail.net --- Notes: v3 safeguards the hex buffer against reuse builtin/replace.c | 16 ++-- t/t6050-replace.sh | 11 +++ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/builtin/replace.c b/builtin/replace.c By the way - Junio, is that the intented outcome of format-patch --notes? I would rather put the newline between the note and the diffstat (and omit the one after the ---) but may have goofed up a rebase: I do not know was intended, but the above quoted output is hard to read, and your suggested change looks way better. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv3] replace: parse revision argument for -d
'git replace' parses the revision arguments when it creates replacements
(so that a sha1 can be abbreviated, e.g.) but not when deleting
replacements.
Make it parse the argument to 'replace -d' in the same way.
Signed-off-by: Michael J Gruber g...@drmicha.warpmail.net
---
Notes:
v3 safeguards the hex buffer against reuse
builtin/replace.c | 16 ++--
t/t6050-replace.sh | 11 +++
2 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/builtin/replace.c b/builtin/replace.c
index e3aaf70..33e6ec3 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -46,24 +46,28 @@ typedef int (*each_replace_name_fn)(const char *name, const
char *ref,
static int for_each_replace_name(const char **argv, each_replace_name_fn fn)
{
- const char **p;
+ const char **p, *q;
char ref[PATH_MAX];
int had_error = 0;
unsigned char sha1[20];
for (p = argv; *p; p++) {
- if (snprintf(ref, sizeof(ref), refs/replace/%s, *p)
- = sizeof(ref)) {
- error(replace ref name too long: %.*s..., 50, *p);
+ q = *p;
+ if (get_sha1(q, sha1)) {
+ error(Failed to resolve '%s' as a valid ref., q);
had_error = 1;
continue;
}
+ q = sha1_to_hex(sha1);
+ snprintf(ref, sizeof(ref), refs/replace/%s, q);
+ /* read_ref() may reuse the buffer */
+ q = ref + strlen(refs/replace/);
if (read_ref(ref, sha1)) {
- error(replace ref '%s' not found., *p);
+ error(replace ref '%s' not found., q);
had_error = 1;
continue;
}
- if (fn(*p, ref, sha1))
+ if (fn(q, ref, sha1))
had_error = 1;
}
return had_error;
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index 5c87f28..decdc33 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -140,6 +140,17 @@ test_expect_success 'git replace replacing' '
test $HASH2 = $(git replace)
'
+test_expect_success 'git replace resolves sha1' '
+ SHORTHASH2=$(git rev-parse --short=8 $HASH2)
+ git replace -d $SHORTHASH2
+ git replace $SHORTHASH2 $R
+ git show $HASH2 | grep O Thor
+ test_must_fail git replace $HASH2 $R
+ git replace -f $HASH2 $R
+ test_must_fail git replace -f
+ test $HASH2 = $(git replace)
+'
+
# This creates a side branch where the bug in H2
# does not appear because P2 is created by applying
# H2 and squashing H5 into it.
--
1.8.0.311.gdd08018
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
