You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Stefan Sperling <st...@elego.de> on 2011/05/17 00:45:50 UTC
[PATCH] -r { DATE } with words
Back before 1.0 we had a date parser written in Yacc that could
parse all sorts of fancy strings such as "yesterday", "last month",
or "two fortnights ago". This was dropped in r848401/r848402 because
of maintenance concerns. The parser was missing some desired features
and also did a lot more than needed. E.g. it could even parse future
dates such as "tomorrow" or "next week" which makes no sense at all
since future commits don't exist yet :)
See http://svn.haxx.se/dev/archive-2003-12/0737.shtml (linked from
issue #408) for details.
It has been pointed out to me that Subversion does not support such date
specifications at all. I think they're rather useful because they allow
users to easily get at commits that happened e.g. during the last N days
or weeks relative to the local time of their machine without having to
calculate appropriate date strings. The patch below adds support for simple
word strings for specifying dates (see log message for details).
There are a couple of restrictions:
To keep things simple years always have 365 days and months
always have 30 days. If more precision is desired normal dates
can be used. Stephen Butler convinced me to take this route.
I do not intend to grow the set of strings any further unless new
strings open up very exciting possibilities.
Localising the strings to other languages would also complicate
things too much since the order of words might change (also pointed
out by Steve). All command names and long options are always in
English anyway.
If the system clock on the client side is wrong revisions might not
be resolved correctly.
Combined with the new --diff option of svn log this makes it very
easy to review commits which happened during e.g. the last 3 days:
svn log --diff -r {"3 days ago"}:{"now"} ^/trunk | less
I am posting this prior to commit because I have received mixed reactions
to this idea. Like myself, Greg was surprised that we don't already support
this. C-Mike said that this might open the door for potential maintenance
problems to reappear in particular if we keep adding more strings.
I don't think we will have problems if we don't go too far with adding
complexity to this feature. But let's continue discussion based on this patch.
Any comments or objections?
[[[
Add support for the following revision { DATE } specifications:
"now" resolves to the most recent revision as of the current time.
"yesterday" resolves to the most recent revision prior to 00:00h of today.
"N years|months|weeks|days|hours|minutes ago" resolve to the most recent
revision prior to the specified time. For years, months, weeks, and days,
round up to 00:00h of the day following the specified time.
A year always has 365 days. A month always has 30 days.
N may be a word from "zero" up to "twelve", or a non-negative digit.
To help scripts, N=0 is allowed and produces the same result as "now",
and if N=1 the final 's' of the unit name is allowed, but not required.
* subversion/libsvn_subr/date.c
(unit_words_table, number_words_table, words_match): New.
(svn_parse_date): Parse new date specifications.
]]]
Index: subversion/libsvn_subr/date.c
===================================================================
--- subversion/libsvn_subr/date.c (revision 1103298)
+++ subversion/libsvn_subr/date.c (working copy)
@@ -22,6 +22,7 @@
#include "svn_time.h"
#include "svn_error.h"
+#include "svn_string.h"
#include "svn_private_config.h"
@@ -187,6 +188,146 @@ template_match(apr_time_exp_t *expt, svn_boolean_t
return TRUE;
}
+static struct unit_words_table {
+ const char *word;
+ apr_time_t value;
+} unit_words_table[] = {
+ /* Word matching does not concern itself with exact days of the month
+ * or leap years so these amounts are always fixed. */
+ { "years", apr_time_from_sec(60 * 60 * 24 * 365) },
+ { "months", apr_time_from_sec(60 * 60 * 24 * 30) },
+ { "weeks", apr_time_from_sec(60 * 60 * 24 * 7) },
+ { "days", apr_time_from_sec(60 * 60 * 24) },
+ { "hours", apr_time_from_sec(60 * 60) },
+ { "minutes", apr_time_from_sec(60) },
+ { "mins", apr_time_from_sec(60) },
+ { NULL , 0 }
+};
+
+static struct number_words_table {
+ const char *word;
+ int number;
+} number_words_table[] = {
+ { "zero", 0 }, { "one", 1 }, { "two", 2 }, { "three", 3 }, { "four", 4 },
+ { "five", 5 }, { "six", 6 }, { "seven", 7 }, { "eight", 8 }, { "nine", 9 },
+ { "ten", 10 }, { "eleven", 11 }, { "twelve", 12 }, { NULL, 0 }
+};
+
+/* Attempt to match the date-string in TEXT according to the following rules:
+ *
+ * "now" resolves to the most recent revision as of the current time NOW.
+ * "yesterday" resolves to the most recent revision prior to 00:00h of today.
+ * "N years|months|weeks|days|hours|minutes ago" resolve to the most recent
+ * revision prior to the specified time. For years, months, weeks, and days,
+ * round up to 00:00h of the day following the specified time.
+ * N may either be a word from NUMBER_WORDS_TABLE defined above, or a
+ * non-negative digit.
+ *
+ * Return TRUE on successful match, FALSE otherwise. On successful match,
+ * fill in *EXP with the matched value and set *LOCALTZ to TRUE (this
+ * function always uses local time). Use POOL for temporary allocations. */
+static svn_boolean_t
+words_match(apr_time_exp_t *expt, svn_boolean_t *localtz,
+ apr_time_t now, const char *text, apr_pool_t *pool)
+{
+ apr_time_t t = -1;
+ const char *word;
+ svn_boolean_t round_to_next_day = TRUE;
+ apr_array_header_t *words;
+
+ words = svn_cstring_split(text, " ", TRUE /* chop_whitespace */, pool);
+
+ if (words->nelts == 0)
+ return FALSE;
+
+ word = APR_ARRAY_IDX(words, 0, const char *);
+
+ if (words->nelts == 1)
+ {
+ if (!strcmp(word, "now"))
+ {
+ t = now;
+ round_to_next_day = FALSE;
+ }
+ else if (!strcmp(word, "yesterday"))
+ t = now - apr_time_from_sec(60 * 60 * 24);
+ }
+ else if (words->nelts == 3)
+ {
+ int i;
+ int n = -1;
+ const char *number_str;
+ const char *unit_str;
+
+ /* Try to parse a number word. */
+ for (i = 0, number_str = number_words_table[i].word;
+ number_str = number_words_table[i].word, number_str != NULL; i++)
+ {
+ if (!strcmp(word, number_str))
+ {
+ n = number_words_table[i].number;
+ break;
+ }
+ }
+
+ if (n < 0)
+ {
+ svn_error_t *err;
+
+ /* Try to parse a digit. */
+ err = svn_cstring_atoi(&n, word);
+ if (err)
+ {
+ svn_error_clear(err);
+ return FALSE;
+ }
+ if (n < 0)
+ return FALSE;
+ }
+
+ /* Try to parse a unit. */
+ word = APR_ARRAY_IDX(words, 1, const char *);
+ for (i = 0, unit_str = unit_words_table[i].word;
+ unit_str = unit_words_table[i].word, unit_str != NULL; i++)
+ {
+ /* Tolerate missing trailing 's' from unit for n=1. */
+ if (!strcmp(word, unit_str) ||
+ (n == 1 && !strncmp(word, unit_str, strlen(unit_str) - 1)))
+ {
+ t = now - (n * unit_words_table[i].value);
+ round_to_next_day = !(!strcmp(unit_str, "hours") ||
+ !strcmp(unit_str, "minutes") ||
+ !strcmp(unit_str, "mins"));
+ break;
+ }
+ }
+ if (t < 0)
+ return FALSE;
+
+ /* Require trailing "ago". */
+ word = APR_ARRAY_IDX(words, 2, const char *);
+ if (strcmp(word, "ago"))
+ return FALSE;
+ }
+ else
+ return FALSE;
+
+ if (t >= 0)
+ {
+ if (apr_time_exp_lt(expt, t) != APR_SUCCESS)
+ return FALSE;
+ if (round_to_next_day)
+ {
+ expt->tm_usec = expt->tm_sec = expt->tm_min = expt->tm_hour = 0;
+ expt->tm_mday++;
+ }
+ *localtz = TRUE;
+ return TRUE;
+ }
+
+ return FALSE;
+}
+
static int
valid_days_by_month[] = {
31, 29, 31, 30,
@@ -244,7 +385,7 @@ svn_parse_date(svn_boolean_t *matched, apr_time_t
expt.tm_mon = expnow.tm_mon;
expt.tm_mday = expnow.tm_mday;
}
- else
+ else if (!words_match(&expt, &localtz, now, text, pool))
return SVN_NO_ERROR;
/* Range validation, allowing for leap seconds */
Re: [PATCH] -r { DATE } with words
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Branko Čibej wrote on Thu, May 19, 2011 at 14:35:01 +0200:
> On 19.05.2011 11:53, Stefan Sperling wrote:
> > On Wed, May 18, 2011 at 09:38:55PM +0200, Branko Čibej wrote:
> >> Why? That doesn't make sense. Second of all, all these wordy aliases are
> >> just shorthands for real timestamps anyway -- by your reasoning, you
> >> could eliminate all of them.
> > There is otherwise no way to express dates relative to the current time.
>
> So instead of introducing a subset of the silliness that was in CVS, why
> then don't you invent an unambiguous format that /can/ express dates
> relative to the current time?
>
> For example, you might support: svn -r {-1.12:13:56}, meaning one day,
> twelve hours, 13 minutes and 56 seconds ago.
>
Which is language-independent, much shorter to type, doesn't require
quoting spaces, avoids the "months are 30 days" edge case...
Anyway, to keep things simple, how about: a '{' followed by either
a plus or a minus means "N days in the future/past" where N is the
number following the plus-or-minus; and if the plus-or-minus is missing
is missing, then it's parsed as a date.
> -- Brane
>
Re: [PATCH] -r { DATE } with words
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Greg Stein wrote on Thu, May 19, 2011 at 10:41:11 -0400:
> 2011/5/19 Branko Čibej <br...@e-reka.si>:
> > On 19.05.2011 15:38, Greg Stein wrote:
> >> 2011/5/19 Branko Čibej <br...@e-reka.si>:
> >>> On 19.05.2011 11:53, Stefan Sperling wrote:
> >>>> On Wed, May 18, 2011 at 09:38:55PM +0200, Branko Čibej wrote:
> >>>>> Why? That doesn't make sense. Second of all, all these wordy aliases are
> >>>>> just shorthands for real timestamps anyway -- by your reasoning, you
> >>>>> could eliminate all of them.
> >>>> There is otherwise no way to express dates relative to the current time.
> >>> So instead of introducing a subset of the silliness that was in CVS, why
> >>> then don't you invent an unambiguous format that /can/ express dates
> >>> relative to the current time?
> >>>
> >>> For example, you might support: svn -r {-1.12:13:56}, meaning one day,
> >>> twelve hours, 13 minutes and 56 seconds ago.
> >> "one day ago" is certainly easier than "-1"
> >>
> >> I don't see this as "silliness" but an easy way to express certain
> >> times. So what if it doesn't do everything? It doesn't the easy stuff
> >> just fine. It hasn't made the medium or hard stuff any more difficult.
> >
> > So someone who's not a native English speaker (or a fair imitation like
> > myself) will have to go looking at the docs ... it is silliness. We
> > don't parse anything but ISO dates, and now suddenly we'll parse whole
> > essays just to get the equivalent of that "-1 day". Sigh.
>
> "whole essays" ... LOL :-)
wc -l `stsp's docstring in 'svn merge' et al`
Re: [PATCH] -r { DATE } with words
Posted by Greg Stein <gs...@gmail.com>.
2011/5/19 Branko Čibej <br...@e-reka.si>:
> On 19.05.2011 15:38, Greg Stein wrote:
>> 2011/5/19 Branko Čibej <br...@e-reka.si>:
>>> On 19.05.2011 11:53, Stefan Sperling wrote:
>>>> On Wed, May 18, 2011 at 09:38:55PM +0200, Branko Čibej wrote:
>>>>> Why? That doesn't make sense. Second of all, all these wordy aliases are
>>>>> just shorthands for real timestamps anyway -- by your reasoning, you
>>>>> could eliminate all of them.
>>>> There is otherwise no way to express dates relative to the current time.
>>> So instead of introducing a subset of the silliness that was in CVS, why
>>> then don't you invent an unambiguous format that /can/ express dates
>>> relative to the current time?
>>>
>>> For example, you might support: svn -r {-1.12:13:56}, meaning one day,
>>> twelve hours, 13 minutes and 56 seconds ago.
>> "one day ago" is certainly easier than "-1"
>>
>> I don't see this as "silliness" but an easy way to express certain
>> times. So what if it doesn't do everything? It doesn't the easy stuff
>> just fine. It hasn't made the medium or hard stuff any more difficult.
>
> So someone who's not a native English speaker (or a fair imitation like
> myself) will have to go looking at the docs ... it is silliness. We
> don't parse anything but ISO dates, and now suddenly we'll parse whole
> essays just to get the equivalent of that "-1 day". Sigh.
"whole essays" ... LOL :-)
Re: [PATCH] -r { DATE } with words
Posted by Branko Čibej <br...@e-reka.si>.
On 19.05.2011 18:29, Stefan Sperling wrote:
>>> Or just not use the feature?
>>>
>>> (It is, after all, completely undocumented for a reason.)
>>>
>> Lack of time on stsp's side?
> We decided to treat is as an undocumented easter egg.
I can actually agree with that part. Just imagine finding an easter egg
sometime in July and cracking the shell by mistake ... phew!
-- Brane
Re: [PATCH] -r { DATE } with words
Posted by Stefan Sperling <st...@elego.de>.
On Thu, May 19, 2011 at 05:05:15PM +0200, Daniel Shahaf wrote:
> Hyrum K Wright wrote on Thu, May 19, 2011 at 16:51:27 +0200:
> > 2011/5/19 Branko Čibej <br...@e-reka.si>:
> > > So someone who's not a native English speaker (or a fair imitation like
> > > myself)
I am not a native English speaker either.
And you might as well argue that option names such as --reintegrate
should be localised.
> > Or just not use the feature?
> >
> > (It is, after all, completely undocumented for a reason.)
> >
>
> Lack of time on stsp's side?
We decided to treat is as an undocumented easter egg.
Re: [PATCH] -r { DATE } with words
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Hyrum K Wright wrote on Thu, May 19, 2011 at 16:51:27 +0200:
> 2011/5/19 Branko Čibej <br...@e-reka.si>:
> > On 19.05.2011 15:38, Greg Stein wrote:
> >> 2011/5/19 Branko Čibej <br...@e-reka.si>:
> >>> On 19.05.2011 11:53, Stefan Sperling wrote:
> >>>> On Wed, May 18, 2011 at 09:38:55PM +0200, Branko Čibej wrote:
> >>>>> Why? That doesn't make sense. Second of all, all these wordy aliases are
> >>>>> just shorthands for real timestamps anyway -- by your reasoning, you
> >>>>> could eliminate all of them.
> >>>> There is otherwise no way to express dates relative to the current time.
> >>> So instead of introducing a subset of the silliness that was in CVS, why
> >>> then don't you invent an unambiguous format that /can/ express dates
> >>> relative to the current time?
> >>>
> >>> For example, you might support: svn -r {-1.12:13:56}, meaning one day,
> >>> twelve hours, 13 minutes and 56 seconds ago.
> >> "one day ago" is certainly easier than "-1"
> >>
> >> I don't see this as "silliness" but an easy way to express certain
> >> times. So what if it doesn't do everything? It doesn't the easy stuff
> >> just fine. It hasn't made the medium or hard stuff any more difficult.
> >
> > So someone who's not a native English speaker (or a fair imitation like
> > myself) will have to go looking at the docs ... it is silliness. We
> > don't parse anything but ISO dates, and now suddenly we'll parse whole
> > essays just to get the equivalent of that "-1 day". Sigh.
>
> Or just not use the feature?
>
> (It is, after all, completely undocumented for a reason.)
>
Lack of time on stsp's side?
Re: [PATCH] -r { DATE } with words
Posted by Hyrum K Wright <hy...@hyrumwright.org>.
2011/5/19 Branko Čibej <br...@e-reka.si>:
> On 19.05.2011 15:38, Greg Stein wrote:
>> 2011/5/19 Branko Čibej <br...@e-reka.si>:
>>> On 19.05.2011 11:53, Stefan Sperling wrote:
>>>> On Wed, May 18, 2011 at 09:38:55PM +0200, Branko Čibej wrote:
>>>>> Why? That doesn't make sense. Second of all, all these wordy aliases are
>>>>> just shorthands for real timestamps anyway -- by your reasoning, you
>>>>> could eliminate all of them.
>>>> There is otherwise no way to express dates relative to the current time.
>>> So instead of introducing a subset of the silliness that was in CVS, why
>>> then don't you invent an unambiguous format that /can/ express dates
>>> relative to the current time?
>>>
>>> For example, you might support: svn -r {-1.12:13:56}, meaning one day,
>>> twelve hours, 13 minutes and 56 seconds ago.
>> "one day ago" is certainly easier than "-1"
>>
>> I don't see this as "silliness" but an easy way to express certain
>> times. So what if it doesn't do everything? It doesn't the easy stuff
>> just fine. It hasn't made the medium or hard stuff any more difficult.
>
> So someone who's not a native English speaker (or a fair imitation like
> myself) will have to go looking at the docs ... it is silliness. We
> don't parse anything but ISO dates, and now suddenly we'll parse whole
> essays just to get the equivalent of that "-1 day". Sigh.
Or just not use the feature?
(It is, after all, completely undocumented for a reason.)
-Hyrum
Re: [PATCH] -r { DATE } with words
Posted by Branko Čibej <br...@e-reka.si>.
On 19.05.2011 15:38, Greg Stein wrote:
> 2011/5/19 Branko Čibej <br...@e-reka.si>:
>> On 19.05.2011 11:53, Stefan Sperling wrote:
>>> On Wed, May 18, 2011 at 09:38:55PM +0200, Branko Čibej wrote:
>>>> Why? That doesn't make sense. Second of all, all these wordy aliases are
>>>> just shorthands for real timestamps anyway -- by your reasoning, you
>>>> could eliminate all of them.
>>> There is otherwise no way to express dates relative to the current time.
>> So instead of introducing a subset of the silliness that was in CVS, why
>> then don't you invent an unambiguous format that /can/ express dates
>> relative to the current time?
>>
>> For example, you might support: svn -r {-1.12:13:56}, meaning one day,
>> twelve hours, 13 minutes and 56 seconds ago.
> "one day ago" is certainly easier than "-1"
>
> I don't see this as "silliness" but an easy way to express certain
> times. So what if it doesn't do everything? It doesn't the easy stuff
> just fine. It hasn't made the medium or hard stuff any more difficult.
So someone who's not a native English speaker (or a fair imitation like
myself) will have to go looking at the docs ... it is silliness. We
don't parse anything but ISO dates, and now suddenly we'll parse whole
essays just to get the equivalent of that "-1 day". Sigh.
-- Brane
Re: [PATCH] -r { DATE } with words
Posted by Greg Stein <gs...@gmail.com>.
2011/5/19 Branko Čibej <br...@e-reka.si>:
> On 19.05.2011 11:53, Stefan Sperling wrote:
>> On Wed, May 18, 2011 at 09:38:55PM +0200, Branko Čibej wrote:
>>> Why? That doesn't make sense. Second of all, all these wordy aliases are
>>> just shorthands for real timestamps anyway -- by your reasoning, you
>>> could eliminate all of them.
>> There is otherwise no way to express dates relative to the current time.
>
> So instead of introducing a subset of the silliness that was in CVS, why
> then don't you invent an unambiguous format that /can/ express dates
> relative to the current time?
>
> For example, you might support: svn -r {-1.12:13:56}, meaning one day,
> twelve hours, 13 minutes and 56 seconds ago.
"one day ago" is certainly easier than "-1"
I don't see this as "silliness" but an easy way to express certain
times. So what if it doesn't do everything? It doesn't the easy stuff
just fine. It hasn't made the medium or hard stuff any more difficult.
Cheers,
-g
Re: [PATCH] -r { DATE } with words
Posted by Branko Čibej <br...@e-reka.si>.
On 19.05.2011 11:53, Stefan Sperling wrote:
> On Wed, May 18, 2011 at 09:38:55PM +0200, Branko Čibej wrote:
>> Why? That doesn't make sense. Second of all, all these wordy aliases are
>> just shorthands for real timestamps anyway -- by your reasoning, you
>> could eliminate all of them.
> There is otherwise no way to express dates relative to the current time.
So instead of introducing a subset of the silliness that was in CVS, why
then don't you invent an unambiguous format that /can/ express dates
relative to the current time?
For example, you might support: svn -r {-1.12:13:56}, meaning one day,
twelve hours, 13 minutes and 56 seconds ago.
-- Brane
Re: [PATCH] -r { DATE } with words
Posted by Stefan Sperling <st...@elego.de>.
On Wed, May 18, 2011 at 09:38:55PM +0200, Branko Čibej wrote:
> Why? That doesn't make sense. Second of all, all these wordy aliases are
> just shorthands for real timestamps anyway -- by your reasoning, you
> could eliminate all of them.
There is otherwise no way to express dates relative to the current time.
> But first of all, if you do have wordy aliases, they should be
> "compatible" with what people find in other tools (to wit, CVS) and what
> a speaker would find natural -- do you prefer "one day ago" to
> "yesterday" when speaking, even in German? :)
CVS had concepts in its date parser that don't even make sense in
the context of CVS (e.g. future dates) and had a ridiculous amount
of redundancy. That is why the old parser was dropped before 1.0.
I don't want to reintroduce that problem.
The initial commit was meant to include only the absolute minimum
of functionality needed to provide relative date stamps.
We can discuss adding more stuff, or removing this simple implementation
if you don't like it. However, it really is a useful feature for some
people. The person who pointed out this missing feature to me is now
planning to switch from CVS to SVN 1.7 because of this feature!
> By the way, one of the reasons why this was dropped before svn-1.0 is
> that it's almost impossible to translate.
All command line svn commands and their options are in English.
The date stamps are part of this interface so it is OK not to translate
them (and yes, doing so would be very, very hard).
> And now I see you parse "one
> days ago" which, I might add, is silly, let alone grammatically incorrect.
My original patch did not allow invalid grammer but Greg suggested
to accept it because it's easy to do and doesn't really hurt.
I'm indifferent.
> My take is -- either do this right, i.e., make it translatable and
> correct, or drop the feature altogether.
I think it's done right. There is no redundancy, it's very few lines
of code, and it solves the problem it is designed to solve (relative
dates). What isn't right about that?
If you want to be able to specify the time stamp "The Ides of March as
of the most recent year of the Chinese year of the dragon" (I stole
this quote) typed on the command line in ancient Egypt glyphs font,
then by all means feel free to make it work. I don't think that's the
right approach to this kind of feature. The language used should be
straightforward and simple but powerful enough to go back from the
current time by an arbitrary amount. That's what it is.
Re: [PATCH] -r { DATE } with words
Posted by Branko Čibej <br...@e-reka.si>.
On 22.05.2011 10:49, Daniel Shahaf wrote:
> Neels J Hofmeyr wrote on Sun, May 22, 2011 at 04:51:10 +0200:
>> We should probably have a separate commandline tool instead that
>> creates absolute timestamps:
>>
>> svn log -r "{`reldate yesterday`}"
> +1
What, are you guys serious about that? I though Neels was joking ...
this goes way past bikeshed colour to the make of the actual bike inside
the shed (I want mine fusion-powered 4-wheel-drive).
-- Brane
Re: [PATCH] -r { DATE } with words
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Neels J Hofmeyr wrote on Sun, May 22, 2011 at 04:51:10 +0200:
> We should probably have a separate commandline tool instead that
> creates absolute timestamps:
>
> svn log -r "{`reldate yesterday`}"
+1
Re: [PATCH] -r { DATE } with words
Posted by Stefan Sperling <st...@elego.de>.
On Sun, May 22, 2011 at 04:51:10AM +0200, Neels J Hofmeyr wrote:
> On 05/21/2011 02:02 PM, Stefan Sperling wrote:
> >But I'd rather use "in 3 months" or something like that than +3m.
>
> wait, now you're also reaching to the future :)
>
> No I was thinking, if I want to have a range, then I could pinpoint
> the start with a revision, date, whatever, and the end of the range
> could be given by -r {+3d} (three days from that point on, and
> similar).
To do that you'll have to reorganise the code that calls svn_parse_date()
and bump the API.
> >It's simply less cryptic.
> ...to the English speaking, yes. Next, people will want to have
> German, French, Zulu words as well...
>
> What I like about my format idea is that it is considerably less
> language dependent (can be documented in any language without
> imposing English spelling on the user) and has considerably less
> characters to type. It also avoids the whole discussion around "day"
> vs. "days" or whether "yestrday" should also be understood, etc.
I agree with the above points. I just don't think they are a good
replacement for the existing english expressions.
> Remember our hackathon discussions -- even to a native English
> speaker, -r {yesterday} isn't self-documenting at all. "Yesterday"
> is a fuzzy concept... IMHO {-1d} is more clearly == -24h. ......
That's why we removed "yesterday" before commit.
> >But since we support a lot of "absolute" time formats there's no
> >reason why we couldn't have more than one "relative" format.
> >This is probably the best answer to the problem of different people
> >having different taste :)
>
> sure, let's have libsvn_dateparsing.
> Oh well, I thought you might have liked my proposal :)
libsvn_bikeshed
Re: [PATCH] -r { DATE } with words
Posted by Neels J Hofmeyr <ne...@elego.de>.
On 05/21/2011 02:02 PM, Stefan Sperling wrote:
> But I'd rather use "in 3 months" or something like that than +3m.
wait, now you're also reaching to the future :)
No I was thinking, if I want to have a range, then I could pinpoint the
start with a revision, date, whatever, and the end of the range could be
given by -r {+3d} (three days from that point on, and similar).
> It's simply less cryptic.
...to the English speaking, yes. Next, people will want to have German,
French, Zulu words as well...
What I like about my format idea is that it is considerably less language
dependent (can be documented in any language without imposing English
spelling on the user) and has considerably less characters to type. It also
avoids the whole discussion around "day" vs. "days" or whether "yestrday"
should also be understood, etc.
Remember our hackathon discussions -- even to a native English speaker, -r
{yesterday} isn't self-documenting at all. "Yesterday" is a fuzzy concept...
IMHO {-1d} is more clearly == -24h. ......
> But since we support a lot of "absolute" time formats there's no
> reason why we couldn't have more than one "relative" format.
> This is probably the best answer to the problem of different people
> having different taste :)
sure, let's have libsvn_dateparsing.
Oh well, I thought you might have liked my proposal :)
We should probably have a separate commandline tool instead that creates
absolute timestamps:
svn log -r "{`reldate yesterday`}"
then I can have my own one. Not that I really use dates. But I *might* get
used to {-3d} ;) Anyway, you go ahead, I'm outta here...
~Neels
Re: [PATCH] -r { DATE } with words
Posted by Stefan Sperling <st...@elego.de>.
On Sat, May 21, 2011 at 02:14:03PM +0300, Daniel Shahaf wrote:
> Stefan Sperling wrote on Sat, May 21, 2011 at 13:07:19 +0200:
> > On Fri, May 20, 2011 at 10:20:41PM +0300, Daniel Shahaf wrote:
> > > Stefan Sperling wrote on Fri, May 20, 2011 at 17:26:39 +0200:
> > > > On Fri, May 20, 2011 at 04:28:55PM +0200, Neels J Hofmeyr wrote:
> > > > > BUT, why don't we just use standardized unit letters? e.g. {-1d}
> > > > > means one day ago. Then we'd have something like
> > > > >
> > > > > [-+]<float-nr>[YyMDdHhmSs]
> > > >
> > > > What would we need the + for?
> > >
> > > (see below)
> > >
> > > > We cannot resolve future revisions.
> > > >
> > >
> > > That's a false statement.
> >
> > Can you explain why?
> > Do you mean if people set their clock to the past?
>
> Either that, or set their svn:date properties to the future.
>
> I'm basically saying that 'svn log -r {yyyy-mm-dd} file:///path/to/somewhere'
> does not depend on what the machine thinks the current time is.
Fair enough.
But I'd rather use "in 3 months" or something like that than +3m.
It's simply less cryptic.
But since we support a lot of "absolute" time formats there's no
reason why we couldn't have more than one "relative" format.
This is probably the best answer to the problem of different people
having different taste :)
Re: [PATCH] -r { DATE } with words
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Stefan Sperling wrote on Sat, May 21, 2011 at 13:07:19 +0200:
> On Fri, May 20, 2011 at 10:20:41PM +0300, Daniel Shahaf wrote:
> > Stefan Sperling wrote on Fri, May 20, 2011 at 17:26:39 +0200:
> > > On Fri, May 20, 2011 at 04:28:55PM +0200, Neels J Hofmeyr wrote:
> > > > BUT, why don't we just use standardized unit letters? e.g. {-1d}
> > > > means one day ago. Then we'd have something like
> > > >
> > > > [-+]<float-nr>[YyMDdHhmSs]
> > >
> > > What would we need the + for?
> >
> > (see below)
> >
> > > We cannot resolve future revisions.
> > >
> >
> > That's a false statement.
>
> Can you explain why?
> Do you mean if people set their clock to the past?
Either that, or set their svn:date properties to the future.
I'm basically saying that 'svn log -r {yyyy-mm-dd} file:///path/to/somewhere'
does not depend on what the machine thinks the current time is.
Re: [PATCH] -r { DATE } with words
Posted by Stefan Sperling <st...@elego.de>.
On Fri, May 20, 2011 at 10:20:41PM +0300, Daniel Shahaf wrote:
> Stefan Sperling wrote on Fri, May 20, 2011 at 17:26:39 +0200:
> > On Fri, May 20, 2011 at 04:28:55PM +0200, Neels J Hofmeyr wrote:
> > > BUT, why don't we just use standardized unit letters? e.g. {-1d}
> > > means one day ago. Then we'd have something like
> > >
> > > [-+]<float-nr>[YyMDdHhmSs]
> >
> > What would we need the + for?
>
> (see below)
>
> > We cannot resolve future revisions.
> >
>
> That's a false statement.
Can you explain why?
Do you mean if people set their clock to the past?
Re: [PATCH] -r { DATE } with words
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Stefan Sperling wrote on Fri, May 20, 2011 at 17:26:39 +0200:
> On Fri, May 20, 2011 at 04:28:55PM +0200, Neels J Hofmeyr wrote:
> > On 05/18/2011 09:38 PM, Branko Čibej wrote:
> > >On 17.05.2011 11:36, Stefan Sperling wrote:
> > >>On Tue, May 17, 2011 at 12:45:50AM +0200, Stefan Sperling wrote:
> > >>>Any comments or objections?
> > >>Neels didn't like the arbitrary "round to 00:00 of next day" rules
> > >>and everyone in the hackathon room seems to agree. So "one day ago"
> > >>is now the same as "24 hours ago".
> > >>
> > >>I also dropped the "yesterday" keyword because it overlaps with "one day ago".
> >
> > I liked 'yesterday' as 'yesterday' == 'one day ago', even add
> > 'fortnight' and so on. But I agree with Brane's reservations. Having
> > an untranslatable, grammar dependent non-feature that isn't even
> > documented... weeeell...
> >
> > BUT, why don't we just use standardized unit letters? e.g. {-1d}
> > means one day ago. Then we'd have something like
> >
> > [-+]<float-nr>[YyMDdHhmSs]
>
> What would we need the + for?
(see below)
> We cannot resolve future revisions.
>
That's a false statement.
Re: [PATCH] -r { DATE } with words
Posted by Stefan Sperling <st...@elego.de>.
On Fri, May 20, 2011 at 04:28:55PM +0200, Neels J Hofmeyr wrote:
> On 05/18/2011 09:38 PM, Branko Čibej wrote:
> >On 17.05.2011 11:36, Stefan Sperling wrote:
> >>On Tue, May 17, 2011 at 12:45:50AM +0200, Stefan Sperling wrote:
> >>>Any comments or objections?
> >>Neels didn't like the arbitrary "round to 00:00 of next day" rules
> >>and everyone in the hackathon room seems to agree. So "one day ago"
> >>is now the same as "24 hours ago".
> >>
> >>I also dropped the "yesterday" keyword because it overlaps with "one day ago".
>
> I liked 'yesterday' as 'yesterday' == 'one day ago', even add
> 'fortnight' and so on. But I agree with Brane's reservations. Having
> an untranslatable, grammar dependent non-feature that isn't even
> documented... weeeell...
>
> BUT, why don't we just use standardized unit letters? e.g. {-1d}
> means one day ago. Then we'd have something like
>
> [-+]<float-nr>[YyMDdHhmSs]
What would we need the + for? We cannot resolve future revisions.
> (M = months, m = minutes, the others are case insensitive)
> No need to translate, no grammar involved, less characters to write.
>
> svn log -r {-1.5d}
> (one-and-a-half days ago)
>
> Heh and we'd drop the '+' too, I guess, making the '-' optional.
> Brainstorming more, the [+-] could define the range:
>
> svn log -r {1y} -r {+1M}
>
> (get the log from one year ago to a month after that.)
> and the reverse case, though some may probably not want to support this:
>
> svn log -r {-1d} -r {5M}
> svn log -r {5M} -r {-1d}
>
> (get the log from five months and a day ago until five months ago.)
>
> If I were to write this feature, this would be my choice. stsp?
I think the current simple grammer is easier to understand at first sight.
It doesn't even need documentation. -r{-1d} isn't self-documenting at all.
Re: [PATCH] -r { DATE } with words
Posted by Neels J Hofmeyr <ne...@elego.de>.
On 05/18/2011 09:38 PM, Branko Čibej wrote:
> On 17.05.2011 11:36, Stefan Sperling wrote:
>> On Tue, May 17, 2011 at 12:45:50AM +0200, Stefan Sperling wrote:
>>> Any comments or objections?
>> Neels didn't like the arbitrary "round to 00:00 of next day" rules
>> and everyone in the hackathon room seems to agree. So "one day ago"
>> is now the same as "24 hours ago".
>>
>> I also dropped the "yesterday" keyword because it overlaps with "one day ago".
I liked 'yesterday' as 'yesterday' == 'one day ago', even add 'fortnight'
and so on. But I agree with Brane's reservations. Having an untranslatable,
grammar dependent non-feature that isn't even documented... weeeell...
BUT, why don't we just use standardized unit letters? e.g. {-1d} means one
day ago. Then we'd have something like
[-+]<float-nr>[YyMDdHhmSs]
(M = months, m = minutes, the others are case insensitive)
No need to translate, no grammar involved, less characters to write.
svn log -r {-1.5d}
(one-and-a-half days ago)
Heh and we'd drop the '+' too, I guess, making the '-' optional.
Brainstorming more, the [+-] could define the range:
svn log -r {1y} -r {+1M}
(get the log from one year ago to a month after that.)
and the reverse case, though some may probably not want to support this:
svn log -r {-1d} -r {5M}
svn log -r {5M} -r {-1d}
(get the log from five months and a day ago until five months ago.)
If I were to write this feature, this would be my choice. stsp?
~Neels
Re: [PATCH] -r { DATE } with words
Posted by Branko Čibej <br...@e-reka.si>.
On 17.05.2011 11:36, Stefan Sperling wrote:
> On Tue, May 17, 2011 at 12:45:50AM +0200, Stefan Sperling wrote:
>> Any comments or objections?
> Neels didn't like the arbitrary "round to 00:00 of next day" rules
> and everyone in the hackathon room seems to agree. So "one day ago"
> is now the same as "24 hours ago".
>
> I also dropped the "yesterday" keyword because it overlaps with "one day ago".
Why? That doesn't make sense. Second of all, all these wordy aliases are
just shorthands for real timestamps anyway -- by your reasoning, you
could eliminate all of them.
But first of all, if you do have wordy aliases, they should be
"compatible" with what people find in other tools (to wit, CVS) and what
a speaker would find natural -- do you prefer "one day ago" to
"yesterday" when speaking, even in German? :)
By the way, one of the reasons why this was dropped before svn-1.0 is
that it's almost impossible to translate. And now I see you parse "one
days ago" which, I might add, is silly, let alone grammatically incorrect.
My take is -- either do this right, i.e., make it translatable and
correct, or drop the feature altogether.
-- Brane
> + { "zero", 0 },
P.S.: ain't none such word in English, unless you talk American. :)
Re: [PATCH] -r { DATE } with words
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Stefan Sperling wrote on Tue, May 17, 2011 at 13:21:20 +0200:
> On Tue, May 17, 2011 at 01:13:31PM +0200, Daniel Shahaf wrote:
> > Offline I advocated for 'N days ago' to be rounded up/down to the full day.
>
> I'll commit it now without rounding and without the "now" keyword.
> We can then bikeshed about extending it.
Okay. (In reply to myself, I could just use 'N+1 days ago' and be done
with bikeshedding)
Re: [PATCH] -r { DATE } with words
Posted by Stefan Sperling <st...@elego.de>.
On Tue, May 17, 2011 at 01:13:31PM +0200, Daniel Shahaf wrote:
> Offline I advocated for 'N days ago' to be rounded up/down to the full day.
I'll commit it now without rounding and without the "now" keyword.
We can then bikeshed about extending it.
Re: [PATCH] -r { DATE } with words
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Offline I advocated for 'N days ago' to be rounded up/down to the full day.
Stefan Sperling wrote on Tue, May 17, 2011 at 11:36:55 +0200:
> On Tue, May 17, 2011 at 12:45:50AM +0200, Stefan Sperling wrote:
> > Any comments or objections?
>
> Neels didn't like the arbitrary "round to 00:00 of next day" rules
> and everyone in the hackathon room seems to agree. So "one day ago"
> is now the same as "24 hours ago".
>
> I also dropped the "yesterday" keyword because it overlaps with "one day ago".
>
> [[[
> Add support for the following revision { DATE } specifications:
>
> "now" resolves to the most recent revision as of the current time.
>
> "N years|months|weeks|days|hours|minutes ago" resolve to the most recent
> revision prior to the specified time. A year always has 365 days.
> A month always has 30 days. N may be a word from "zero" up to "twelve",
> or a non-negative digit. To help scripts, N=0 is allowed and produces
> the same result as "now", and if N=1 the final 's' of the unit name
> is allowed, but not required.
>
> * subversion/libsvn_subr/date.c
> (unit_words_table, number_words_table, words_match): New.
> (svn_parse_date): Parse new date specifications.
> ]]]
Re: [PATCH] -r { DATE } with words
Posted by Stefan Sperling <st...@elego.de>.
On Tue, May 17, 2011 at 12:45:50AM +0200, Stefan Sperling wrote:
> Any comments or objections?
Neels didn't like the arbitrary "round to 00:00 of next day" rules
and everyone in the hackathon room seems to agree. So "one day ago"
is now the same as "24 hours ago".
I also dropped the "yesterday" keyword because it overlaps with "one day ago".
[[[
Add support for the following revision { DATE } specifications:
"now" resolves to the most recent revision as of the current time.
"N years|months|weeks|days|hours|minutes ago" resolve to the most recent
revision prior to the specified time. A year always has 365 days.
A month always has 30 days. N may be a word from "zero" up to "twelve",
or a non-negative digit. To help scripts, N=0 is allowed and produces
the same result as "now", and if N=1 the final 's' of the unit name
is allowed, but not required.
* subversion/libsvn_subr/date.c
(unit_words_table, number_words_table, words_match): New.
(svn_parse_date): Parse new date specifications.
]]]
Index: subversion/libsvn_subr/date.c
===================================================================
--- subversion/libsvn_subr/date.c (revision 1104073)
+++ subversion/libsvn_subr/date.c (working copy)
@@ -22,6 +22,7 @@
#include "svn_time.h"
#include "svn_error.h"
+#include "svn_string.h"
#include "svn_private_config.h"
@@ -187,6 +188,126 @@ template_match(apr_time_exp_t *expt, svn_boolean_t
return TRUE;
}
+static struct unit_words_table {
+ const char *word;
+ apr_time_t value;
+} unit_words_table[] = {
+ /* Word matching does not concern itself with exact days of the month
+ * or leap years so these amounts are always fixed. */
+ { "years", apr_time_from_sec(60 * 60 * 24 * 365) },
+ { "months", apr_time_from_sec(60 * 60 * 24 * 30) },
+ { "weeks", apr_time_from_sec(60 * 60 * 24 * 7) },
+ { "days", apr_time_from_sec(60 * 60 * 24) },
+ { "hours", apr_time_from_sec(60 * 60) },
+ { "minutes", apr_time_from_sec(60) },
+ { "mins", apr_time_from_sec(60) },
+ { NULL , 0 }
+};
+
+static struct number_words_table {
+ const char *word;
+ int number;
+} number_words_table[] = {
+ { "zero", 0 }, { "one", 1 }, { "two", 2 }, { "three", 3 }, { "four", 4 },
+ { "five", 5 }, { "six", 6 }, { "seven", 7 }, { "eight", 8 }, { "nine", 9 },
+ { "ten", 10 }, { "eleven", 11 }, { "twelve", 12 }, { NULL, 0 }
+};
+
+/* Attempt to match the date-string in TEXT according to the following rules:
+ *
+ * "now" resolves to the most recent revision as of the current time NOW.
+ * "N years|months|weeks|days|hours|minutes ago" resolve to the most recent
+ * revision prior to the specified time. N may either be a word from
+ * NUMBER_WORDS_TABLE defined above, or a non-negative digit.
+ *
+ * Return TRUE on successful match, FALSE otherwise. On successful match,
+ * fill in *EXP with the matched value and set *LOCALTZ to TRUE (this
+ * function always uses local time). Use POOL for temporary allocations. */
+static svn_boolean_t
+words_match(apr_time_exp_t *expt, svn_boolean_t *localtz,
+ apr_time_t now, const char *text, apr_pool_t *pool)
+{
+ apr_time_t t = -1;
+ const char *word;
+ apr_array_header_t *words;
+
+ words = svn_cstring_split(text, " ", TRUE /* chop_whitespace */, pool);
+
+ if (words->nelts == 0)
+ return FALSE;
+
+ word = APR_ARRAY_IDX(words, 0, const char *);
+
+ if ((words->nelts == 1) && (!strcmp(word, "now")))
+ t = now;
+ else if (words->nelts == 3)
+ {
+ int i;
+ int n = -1;
+ const char *number_str;
+ const char *unit_str;
+
+ /* Try to parse a number word. */
+ for (i = 0, number_str = number_words_table[i].word;
+ number_str = number_words_table[i].word, number_str != NULL; i++)
+ {
+ if (!strcmp(word, number_str))
+ {
+ n = number_words_table[i].number;
+ break;
+ }
+ }
+
+ if (n < 0)
+ {
+ svn_error_t *err;
+
+ /* Try to parse a digit. */
+ err = svn_cstring_atoi(&n, word);
+ if (err)
+ {
+ svn_error_clear(err);
+ return FALSE;
+ }
+ if (n < 0)
+ return FALSE;
+ }
+
+ /* Try to parse a unit. */
+ word = APR_ARRAY_IDX(words, 1, const char *);
+ for (i = 0, unit_str = unit_words_table[i].word;
+ unit_str = unit_words_table[i].word, unit_str != NULL; i++)
+ {
+ /* Tolerate missing trailing 's' from unit for n=1. */
+ if (!strcmp(word, unit_str) ||
+ (n == 1 && !strncmp(word, unit_str, strlen(unit_str) - 1)))
+ {
+ t = now - (n * unit_words_table[i].value);
+ break;
+ }
+ }
+ if (t < 0)
+ return FALSE;
+
+ /* Require trailing "ago". */
+ word = APR_ARRAY_IDX(words, 2, const char *);
+ if (strcmp(word, "ago"))
+ return FALSE;
+ }
+ else
+ return FALSE;
+
+ if (t >= 0)
+ {
+ if (apr_time_exp_lt(expt, t) != APR_SUCCESS)
+ return FALSE;
+ *localtz = TRUE;
+ return TRUE;
+ }
+
+ return FALSE;
+}
+
static int
valid_days_by_month[] = {
31, 29, 31, 30,
@@ -244,7 +365,7 @@ svn_parse_date(svn_boolean_t *matched, apr_time_t
expt.tm_mon = expnow.tm_mon;
expt.tm_mday = expnow.tm_mday;
}
- else
+ else if (!words_match(&expt, &localtz, now, text, pool))
return SVN_NO_ERROR;
/* Range validation, allowing for leap seconds */