[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: For consistency mark some dates and strings for localization

From: Karl Fogel <kfogel_at_red-bean.com>
Date: 2007-09-10 21:18:55 CEST

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

This is an archived mail posted to the Subversion Dev mailing list.