Arfrever Frehtes Taifersar Arahesis <arfrever.fta@gmail.com> writes:
> I'm attaching a new patch. I hope that it won't be forgotten.
>
> [[[
> Mark some dates and strings for localization.
>
> * subversion/libsvn_diff/diff_file.c
> (output_unified_default_hdr): Mark a date for localization.
> * subversion/svn/list-cmd.c
> (print_dirent): Mark dates for localization.
> * subversion/svnlook/main.c
> (generate_label): Mark strings for localization.
> "txn" and "rev" are translatable here.
>
> Patch by: Arfrever Frehtes Taifersar Arahesis <arfrever.fta@gmail.com>
> ]]]
First, thanks for including a log message with the patch :-). And I
see you already got Hyrum's mail about "[PATCH]" in the subject
header, so no problem there.
I was just discussing your patch with Erik Hülsmann in IRC. In the
end, we weren't convinced that it should be applied. Here's our
conversation:
<kfogel> Is it our norm to localize date formats? Someone just
posted a patch that does, e.g., this:
<kfogel> - "%a %b %e :%M:%S %Y", &exploded_time);
+ _("%a %b %e :%M:%S %Y"), &exploded_time);
<ehu> kfogel: I meant to answer that mail. He already did so for
another date. But I think we shouldn't do that.
<ehu> kfogel: Because that is already part of the locale data
<kfogel> ehu: that's what I was worried about.
<kfogel> Want me to just follow up w/ an edited transcript of our IRC
conversation right now? (save you some time)
<ehu> please, yes.
<ehu> I've looked at strftime and it provides %c (courtisy of
Yorick?)
<ehu> which is documented to 'return the date format in the
specific locale'
<kfogel> aha
<ehu> to be exact:
<ehu> %c The preferred date and time representation for
the current locale
<kfogel> What about the part of his patch that applies to diff
labels?
<kfogel> e.g.:
<kfogel> if (name)
- *label = apr_psprintf(pool, "%s\t%s (txn %s)",
+ *label = apr_psprintf(pool, _("%s\t%s (txn %s)"),
path, datestr, name);
else
- *label = apr_psprintf(pool, "%s\t%s (rev %ld)",
+ *label = apr_psprintf(pool, _("%s\t%s (rev %ld)"),
path, datestr, rev);
<ehu> no way. that's just diff specific. Why would that need
translation? There's not even a real word there!?
<kfogel> yeah, "txn" and "rev" don't really need to get translated
anyway, right?
<kfogel> or do they?
<ehu> I think (rev %ld) should have been (r%ld) though
<kfogel> good point!
<kfogel> that's a separate commit; I'll just do that and see if
anyone complains.
<ehu> And personally, I never saw txn appear anywhere.
<kfogel> I highly doubt anyone is parsing that part of the format,
and if they are, they can easily adjust
<kfogel> maybe the txn part is new, I don't know
<ehu> never mind that. I've never seen a complaint someone didn't
understand the rev part.
<ehu> which is about to disappear anyway.
<ehu> txn isn't common enough (in output) to bother translators
with.
<ehu> (imo)
<kfogel> "rev" is about to disappear?
<kfogel> (oh, you mean to "r"?)
<ehu> yup
<kfogel> gotcha
<kfogel> okay, responding to original poster now
<ehu> thanks
So right now, I think it wouldn't be applied. If we're missing
something important, please let us know.
Below is your patch, just for reference.
Best,
-Karl
> Index: subversion/libsvn_diff/diff_file.c
> ===================================================================
> --- subversion/libsvn_diff/diff_file.c (revision 26497)
> +++ subversion/libsvn_diff/diff_file.c (working copy)
> @@ -1058,7 +1058,7 @@
> apr_time_exp_lt(&exploded_time, file_info.mtime);
>
> apr_strftime(time_buffer, &time_len, sizeof(time_buffer) - 1,
> - "%a %b %e %H:%M:%S %Y", &exploded_time);
> + _("%a %b %e %H:%M:%S %Y"), &exploded_time);
>
> *header = apr_psprintf(pool, "%s\t%s", path, time_buffer);
>
> Index: subversion/svn/list-cmd.c
> ===================================================================
> --- subversion/svn/list-cmd.c (revision 26497)
> +++ subversion/svn/list-cmd.c (working copy)
> @@ -87,12 +87,12 @@
> && apr_time_sec(dirent->time - now) < (365 * 86400 / 2))
> {
> apr_err = apr_strftime(timestr, &size, sizeof(timestr),
> - "%b %d %H:%M", &exp_time);
> + _("%b %d %H:%M"), &exp_time);
> }
> else
> {
> apr_err = apr_strftime(timestr, &size, sizeof(timestr),
> - "%b %d %Y", &exp_time);
> + _("%b %d %Y"), &exp_time);
> }
>
> /* if that failed, just zero out the string and print nothing */
> Index: subversion/svnlook/main.c
> ===================================================================
> --- subversion/svnlook/main.c (revision 26497)
> +++ subversion/svnlook/main.c (working copy)
> @@ -700,10 +700,10 @@
> datestr = " ";
>
> if (name)
> - *label = apr_psprintf(pool, "%s\t%s (txn %s)",
> + *label = apr_psprintf(pool, _("%s\t%s (txn %s)"),
> path, datestr, name);
> else
> - *label = apr_psprintf(pool, "%s\t%s (rev %ld)",
> + *label = apr_psprintf(pool, _("%s\t%s (rev %ld)"),
> path, datestr, rev);
> return SVN_NO_ERROR;
> }
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Sep 10 21:15:33 2007