You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Fabien COELHO <fa...@coelho.net> on 2008/01/23 16:06:02 UTC

Broken translation strings since r28979

Since r28979 several strings are broken in 
subversion/libsvn_fs_base/dag.c, e.g.

     _("Invalid value (%" APR_INT64_T_FMT ") for "
       "node revision mergeinfo count")

a #define such as the format cannot be used in in a _() because the 
derivation script does not know how to interpret it, and moreover it is 
not a constant and may differ from one implementation to another.

Maybe it should allocate a temporary string with the integer and use a 
constant "%s" format.

-- 
Fabien.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Broken translation strings since r28979

Posted by Fabien COELHO <fa...@coelho.net>.
>> Since r28979 several strings are broken in
>>
>> Maybe it should allocate a temporary string with the integer and use a
>> constant "%s" format.
>
> Yes. See r28692.

Ok, thanks... Sorry for the noise.
I cannot understand why my "svn update" did not get it.

-- 
Fabien.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Broken translation strings since r28979

Posted by Arfrever Frehtes Taifersar Arahesis <ar...@gmail.com>.
2008-01-25 21:17:37 Karl Fogel napisał(a):
> Arfrever Frehtes Taifersar Arahesis <ar...@gmail.com> writes:
> >> ...in both of these cases, shouldn't the first "%s" be "%%s"?
> >
> > You're right.
> > I'm attaching the updated patch.
> 
> +1 to commit.

Committed in r29053.

> I'm assuming you've built & tested it now;

Yes.

-- 
Arfrever Frehtes Taifersar Arahesis

Re: [PATCH] Broken translation strings since r28979

Posted by Karl Fogel <kf...@red-bean.com>.
Arfrever Frehtes Taifersar Arahesis <ar...@gmail.com> writes:
>> ...in both of these cases, shouldn't the first "%s" be "%%s"?
>
> You're right.
> I'm attaching the updated patch.

+1 to commit.  I'm assuming you've built & tested it now; the change
looks good to me, but I only reviewed in email.

Thanks!

-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Broken translation strings since r28979

Posted by Arfrever Frehtes Taifersar Arahesis <ar...@gmail.com>.
2008-01-24 22:25:07 Karl Fogel napisał(a):
> Arfrever Frehtes Taifersar Arahesis <ar...@gmail.com> writes:
> > I'm attaching a patch to fix this bug.
> 
> Thanks!  Did you build/test it?  I haven't yet, but I just noticed
> something reading the patch...
> 
> > --- subversion/libsvn_fs_fs/dag.c	(wersja 29007)
> > +++ subversion/libsvn_fs_fs/dag.c	(kopia robocza)
> > @@ -527,8 +527,10 @@
> >        svn_string_t *idstr = svn_fs_fs__id_unparse(node->id, pool);
> >        return svn_error_createf
> >          (SVN_ERR_FS_CORRUPT, NULL,
> > -         _("Can't increment mergeinfo count on node-revision %s to negative "
> > -           "value %" APR_INT64_T_FMT),
> > +         apr_psprintf(pool,
> > +                      _("Can't increment mergeinfo count on node-revision %s "
> > +                        "to negative value %%%s"),
> > +                      APR_INT64_T_FMT),
> >           idstr->data, noderev->mergeinfo_count);
> >      }
> >    if (noderev->mergeinfo_count > 1 && noderev->kind == svn_node_file)
> > @@ -536,8 +538,10 @@
> >        svn_string_t *idstr = svn_fs_fs__id_unparse(node->id, pool);
> >        return svn_error_createf
> >          (SVN_ERR_FS_CORRUPT, NULL,
> > -         _("Can't increment mergeinfo count on *file* node-revision %s to "
> > -           "%" APR_INT64_T_FMT " (> 1)"),
> > +         apr_psprintf(pool,
> > +                      _("Can't increment mergeinfo count on *file* "
> > +                        "node-revision %s to %%%s (> 1)"),
> > +                      APR_INT64_T_FMT),
> >           idstr->data, noderev->mergeinfo_count);
> >      }
> 
> ...in both of these cases, shouldn't the first "%s" be "%%s"?

You're right.
I'm attaching the updated patch.

[[[
* subversion/libsvn_fs_base/dag.c
  (svn_fs_base__dag_set_mergeinfo_count,
   svn_fs_base__dag_adjust_mergeinfo_count):
* subversion/libsvn_fs_fs/dag.c
  (svn_fs_fs__dag_increment_mergeinfo_count):
   Pass translatable strings to apr_psprintf() function to create intermediate
   strings with appropriate format specifier.

Found by: fabien

Patch by: arfrever

Review by: kfogel
]]]

-- 
Arfrever Frehtes Taifersar Arahesis

Re: [PATCH] Broken translation strings since r28979

Posted by Karl Fogel <kf...@red-bean.com>.
Arfrever Frehtes Taifersar Arahesis <ar...@gmail.com> writes:
> I know.  I only mentioned r28692 as an example of how to fix this bug.
>
> I'm attaching a patch to fix this bug.
>
> [[[
> * subversion/libsvn_fs_base/dag.c
>   (svn_fs_base__dag_set_mergeinfo_count,
>    svn_fs_base__dag_adjust_mergeinfo_count):
> * subversion/libsvn_fs_fs/dag.c
>   (svn_fs_fs__dag_increment_mergeinfo_count):
>    Pass translatable strings to apr_psprintf() function to create intermediate
>    strings with appropriate format specifier.
>
> Found by: fabien
>
> Patch by: arfrever
> ]]]

Thanks!  Did you build/test it?  I haven't yet, but I just noticed
something reading the patch...

> --- subversion/libsvn_fs_fs/dag.c	(wersja 29007)
> +++ subversion/libsvn_fs_fs/dag.c	(kopia robocza)
> @@ -527,8 +527,10 @@
>        svn_string_t *idstr = svn_fs_fs__id_unparse(node->id, pool);
>        return svn_error_createf
>          (SVN_ERR_FS_CORRUPT, NULL,
> -         _("Can't increment mergeinfo count on node-revision %s to negative "
> -           "value %" APR_INT64_T_FMT),
> +         apr_psprintf(pool,
> +                      _("Can't increment mergeinfo count on node-revision %s "
> +                        "to negative value %%%s"),
> +                      APR_INT64_T_FMT),
>           idstr->data, noderev->mergeinfo_count);
>      }
>    if (noderev->mergeinfo_count > 1 && noderev->kind == svn_node_file)
> @@ -536,8 +538,10 @@
>        svn_string_t *idstr = svn_fs_fs__id_unparse(node->id, pool);
>        return svn_error_createf
>          (SVN_ERR_FS_CORRUPT, NULL,
> -         _("Can't increment mergeinfo count on *file* node-revision %s to "
> -           "%" APR_INT64_T_FMT " (> 1)"),
> +         apr_psprintf(pool,
> +                      _("Can't increment mergeinfo count on *file* "
> +                        "node-revision %s to %%%s (> 1)"),
> +                      APR_INT64_T_FMT),
>           idstr->data, noderev->mergeinfo_count);
>      }

...in both of these cases, shouldn't the first "%s" be "%%s"?  Or
another way: you could do the idstr->data substitution in the
apr_psprintf() instead of the svn_error_createf().  

Either way works; but I think the current way does not.

-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

[PATCH] Broken translation strings since r28979

Posted by Arfrever Frehtes Taifersar Arahesis <ar...@gmail.com>.
2008-01-24 12:49:48 Fabien COELHO napisał(a):
> 
> >> Since r28979 several strings are broken in
> >> subversion/libsvn_fs_base/dag.c, e.g.
> >>
> >>      _("Invalid value (%" APR_INT64_T_FMT ") for "
> >>        "node revision mergeinfo count")
> >>
> >> a #define such as the format cannot be used in in a _() because the
> >> derivation script does not know how to interpret it, and moreover it is
> >> not a constant and may differ from one implementation to another.
> >>
> >> Maybe it should allocate a temporary string with the integer and use a
> >> constant "%s" format.
> >
> > Yes. See r28692.
> 
> Sorry for my previous 'ok', I'm not okay:-)
> The problem is still in HEAD

I know.
I only mentioned r28692 as an example of how to fix this bug.

I'm attaching a patch to fix this bug.

[[[
* subversion/libsvn_fs_base/dag.c
  (svn_fs_base__dag_set_mergeinfo_count,
   svn_fs_base__dag_adjust_mergeinfo_count):
* subversion/libsvn_fs_fs/dag.c
  (svn_fs_fs__dag_increment_mergeinfo_count):
   Pass translatable strings to apr_psprintf() function to create intermediate
   strings with appropriate format specifier.

Found by: fabien

Patch by: arfrever
]]]

-- 
Arfrever Frehtes Taifersar Arahesis

Re: Broken translation strings since r28979

Posted by Fabien COELHO <fa...@ensmp.fr>.
>> Since r28979 several strings are broken in
>> subversion/libsvn_fs_base/dag.c, e.g.
>>
>>      _("Invalid value (%" APR_INT64_T_FMT ") for "
>>        "node revision mergeinfo count")
>>
>> a #define such as the format cannot be used in in a _() because the
>> derivation script does not know how to interpret it, and moreover it is
>> not a constant and may differ from one implementation to another.
>>
>> Maybe it should allocate a temporary string with the integer and use a
>> constant "%s" format.
>
> Yes. See r28692.

Sorry for my previous 'ok', I'm not okay:-)
The problem is still in HEAD :

fabien> svn info subversion/libsvn_fs_base/dag.c
Path: subversion/libsvn_fs_base/dag.c
Name: dag.c
URL: 
https://svn.collab.net/repos/svn/trunk/subversion/libsvn_fs_base/dag.c
Repository Root: https://svn.collab.net/repos/svn
Repository UUID: 65390229-12b7-0310-b90b-f21a5aa7ec8e
Revision: 29007
Node Kind: file
Schedule: normal
Last Changed Author: cmpilato
Last Changed Rev: 29005
Last Changed Date: 2008-01-23 19:28:07 +0100 (Wed, 23 Jan 2008)
Text Last Updated: 2008-01-23 19:28:07 +0100 (Wed, 23 Jan 2008)
Checksum: b1a1be768a2f14afea2f27bbb3e83896


fabien> grep FMT subversion/libsvn_fs_base/dag.c
    _("Invalid value (%" APR_INT64_T_FMT ") for "
    _("Invalid value (%" APR_INT64_T_FMT ") for "


fabien> make po-update
../libsvn_fs_base/dag.c:1602: warning: Although being used in a format 
string position, the msgid is not a valid C format string. Reason: The 
string ends in the middle of a directive.
../libsvn_fs_fs/dag.c:530: warning: Although being used in a format string 
position, the msgid is not a valid C format string. Reason: The string 
ends in the middle of a directive.
../libsvn_fs_fs/dag.c:539: warning: Although being used in a format string 
position, the msgid is not a valid C format string. Reason: The string 
ends in the middle of a directive.


-- 
Fabien.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Broken translation strings since r28979

Posted by Arfrever Frehtes Taifersar Arahesis <ar...@gmail.com>.
2008-01-23, Fabien COELHO <fa...@coelho.net> napisał(a):
>
> Since r28979 several strings are broken in
> subversion/libsvn_fs_base/dag.c, e.g.
>
>      _("Invalid value (%" APR_INT64_T_FMT ") for "
>        "node revision mergeinfo count")
>
> a #define such as the format cannot be used in in a _() because the
> derivation script does not know how to interpret it, and moreover it is
> not a constant and may differ from one implementation to another.
>
> Maybe it should allocate a temporary string with the integer and use a
> constant "%s" format.

Yes. See r28692.

--
Arfrever Frehtes Taifersar Arahesis