Jeff King venit, vidit, dixit 29.10.2012 10:04: > On Mon, Oct 29, 2012 at 10:02:47AM +0100, Michael J Gruber wrote: > >> Jeff King venit, vidit, dixit 29.10.2012 07:58: >>> On Fri, Oct 26, 2012 at 03:33:27PM +0200, Michael J Gruber wrote: >>> >>>> for (p = argv; *p; p++) { >>>> - if (snprintf(ref, sizeof(ref), "refs/replace/%s", *p) >>>> + q = *p; >>>> + if (get_sha1(q, sha1)) >>>> + warning("Failed to resolve '%s' as a valid ref; taking >>>> it literally.", q); >>>> + else >>>> + q = sha1_to_hex(sha1); >>> >>> Doesn't get_sha1 already handle this for 40-byte sha1s (and for anything >>> else, it would not work anyway)? >> >> What is "this"??? >> >> So far, "git replace -d <rev>" only accepts a full sha1, because it uses >> it literally as a ref name "resf/replace/<rev>" without resolving anything. >> >> The patch makes it so that <rev> gets resolved to a sha1 even if it is >> abbreviated, and then it gets used. >> >> Or do you mean the warning? > > Sorry, yeah, I meant the warning and fallback. > > If I understand correctly, the fallback will never work unless we are > fed a 40-byte sha1. But get_sha1 should always return a 40-byte sha1 > without doing any further processing.
You do understand correctly, and I misunderstood get_sha1(). I does not check for the presence of that sha1 in the object db, so that it succeeds no matter what, that is: if it's valid hex. So I should probably rewrite the error handling: no more need to check for lengths. 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