Jeff King <peff@xxxxxxxx> writes:
> A minor nit, but wouldn't:
>
> int approxidate_careful(const char *str, unsigned long *out);
>
> returning an error code be the more usual pattern for a function with
> error-plus-output (your approxidate wrapper would have to be a function then,
> not a macro)?
I don't have strong preference either way; the one in the patch was
modelled after setup_git_directory_gently(&nongit_ok), and slightly easier
to work with for existing callers that don't care enough.
>> @@ -413,8 +413,11 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
>> } else if (0 <= nth)
>> at_time = 0;
>> else {
>> + int errors = 0;
>> char *tmp = xstrndup(str + at + 2, reflog_len);
>> - at_time = approxidate(tmp);
>> + at_time = approxidate_careful(tmp, &errors);
>> + if (errors)
>> + die("Bogus timestamp '%s'", tmp);
>> free(tmp);
>
> I was just going to "return -1" here, which yields:
>
> $ git show @{bogosity}
> fatal: ambiguous argument '@{bogosity}': unknown revision or path not in the working tree.
> Use '--' to separate paths from revisions
>
> instead of
>
> $ git show @{bogosity}
> fatal: Bogus timestamp 'bogosity'
Good point. Let's change it to silently return -1 and let the caller take
care of it. Perhaps there are some callers that say "does this name an
object? If not, let's try pathname".
>> +test_expect_success '@{30.years.ago} shows old' '
>> + check_at @{30.years.ago} one
>
> Side note: I chose this because we needed to go back from the current
> time beyond where test_tick would place the commit. Which means this
> test has a 2035 bug. :)
Can't we use an absolute date, given that test_tick gives fixed timestamp
sequence to pretend as if we were still in 2005 when we are running these
tests?
sha1_name.c | 4 ++--
t/t0101-at-syntax.sh | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/sha1_name.c b/sha1_name.c
index f4a74fe..04fb3b8 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -398,9 +398,9 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
int errors = 0;
char *tmp = xstrndup(str + at + 2, reflog_len);
at_time = approxidate_careful(tmp, &errors);
- if (errors)
- die("Bogus timestamp '%s'", tmp);
free(tmp);
+ if (errors)
+ return -1;
}
if (read_ref_at(real_ref, at_time, nth, sha1, NULL,
&co_time, &co_tz, &co_cnt)) {
diff --git a/t/t0101-at-syntax.sh b/t/t0101-at-syntax.sh
index ccabc37..5e298c5 100755
--- a/t/t0101-at-syntax.sh
+++ b/t/t0101-at-syntax.sh
@@ -26,8 +26,8 @@ test_expect_success '@{now} shows current' '
check_at @{now} two
'
-test_expect_success '@{30.years.ago} shows old' '
- check_at @{30.years.ago} one
+test_expect_success '@{2001-09-17} (before the first commit) shows old' '
+ check_at @{2001-09-17} one
'
test_expect_success 'silly approxidates work' '
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html