You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Philip Martin <ph...@codematters.co.uk> on 2004/09/29 20:22:29 UTC

Re: svn commit: r11172 - trunk/subversion/svnlook

lundblad@tigris.org writes:

> Author: lundblad
> Date: Wed Sep 29 14:54:26 2004
> New Revision: 11172

>  do_log (svnlook_ctxt_t *c, svn_boolean_t print_size, apr_pool_t *pool)
>  {
>    svn_string_t *prop_value;
> +  svn_string_t *prop_value_native;
>  
> -  SVN_ERR (get_property (&prop_value, TRUE, c, SVN_PROP_REVISION_LOG, pool));
> +  SVN_ERR (get_property (&prop_value, c, SVN_PROP_REVISION_LOG, pool));
>    if (! (prop_value && prop_value->data))
>      {
>        SVN_ERR (svn_cmdline_printf (pool, "%s\n", print_size ? "0" : ""));
> @@ -1083,8 +1083,14 @@
>      }
>    
>    if (print_size)
> -    SVN_ERR (svn_cmdline_printf (pool, "%" APR_SIZE_T_FMT "\n",
> -                                 prop_value->len));
> +    {
> +      /* svn_cmdline_printf will convert to the native locale and eol-style
> +         for us, but we need the size of the converted message. */
> +      SVN_ERR (svn_subst_detranslate_string (&prop_value_native, prop_value,
> +                                             TRUE, pool));
> +      SVN_ERR (svn_cmdline_printf (pool, "%" APR_SIZE_T_FMT "\n",
> +                                   prop_value_native->len));
> +    }
>  
>    SVN_ERR (svn_cmdline_printf (pool, "%s\n", prop_value->data));

Nearly, but not quite!  This now fails to do EOL conversion on the
printed log message so multi-line log messages on non-LF systems won't
be correct, also since svn_subst_detranslate_string does do the EOL
conversion the lengths won't match.

How about calling svn_subst_detranslate_string unconditionally and
then using svn_stream_printf/write on prop_value_native?

-- 
Philip Martin

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

Re: svn commit: r11172 - trunk/subversion/svnlook

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Wed, 29 Sep 2004, Philip Martin wrote:

> "Peter N. Lundblad" <pe...@famlundblad.se> writes:
>
> >> Nearly, but not quite!  This now fails to do EOL conversion on the
> >> printed log message so multi-line log messages on non-LF systems won't
> >> be correct, also since svn_subst_detranslate_string does do the EOL
> >> conversion the lengths won't match.
> >>
> > Did you test it? svn_cmdline_printf does LF -> CRLF on platforms that need
> > it through the C stdio streams.
>
> I didn't test it, I just noticed that you removed an EOL conversion.
> My apologies if it does work.
>
I haven't tested it on Windows either... I think it was discussed before
and the conclusion was that it is translated.

> > How do you think all our \ns in the code work?
>
> I've always assumed they are translated at compile time.
No, they are not. '\n' must be a single character if I understand
correctly.

Regards,
//Peter

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

Re: svn commit: r11172 - trunk/subversion/svnlook

Posted by Philip Martin <ph...@codematters.co.uk>.
Philip Martin <ph...@codematters.co.uk> writes:

>> How do you think all our \ns in the code work?
>
> I've always assumed they are translated at compile time.

Hmm, obviously I've never had to think about it :-/

\n must be a single character on all platforms, so non-LF platforms
must convert somewhere else at runtime.  So your fix probably does
work.  Sorry for the confusion.

-- 
Philip Martin

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

Re: svn commit: r11172 - trunk/subversion/svnlook

Posted by Philip Martin <ph...@codematters.co.uk>.
"Peter N. Lundblad" <pe...@famlundblad.se> writes:

>> Nearly, but not quite!  This now fails to do EOL conversion on the
>> printed log message so multi-line log messages on non-LF systems won't
>> be correct, also since svn_subst_detranslate_string does do the EOL
>> conversion the lengths won't match.
>>
> Did you test it? svn_cmdline_printf does LF -> CRLF on platforms that need
> it through the C stdio streams.

I didn't test it, I just noticed that you removed an EOL conversion.
My apologies if it does work.

> How do you think all our \ns in the code work?

I've always assumed they are translated at compile time.

-- 
Philip Martin

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

Re: svn commit: r11172 - trunk/subversion/svnlook

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Wed, 29 Sep 2004, Philip Martin wrote:

> lundblad@tigris.org writes:
>
> > Author: lundblad
> > Date: Wed Sep 29 14:54:26 2004
> > New Revision: 11172
>
> >  do_log (svnlook_ctxt_t *c, svn_boolean_t print_size, apr_pool_t *pool)
> >  {
> >    svn_string_t *prop_value;
> > +  svn_string_t *prop_value_native;
> >
> > -  SVN_ERR (get_property (&prop_value, TRUE, c, SVN_PROP_REVISION_LOG, pool));
> > +  SVN_ERR (get_property (&prop_value, c, SVN_PROP_REVISION_LOG, pool));
> >    if (! (prop_value && prop_value->data))
> >      {
> >        SVN_ERR (svn_cmdline_printf (pool, "%s\n", print_size ? "0" : ""));
> > @@ -1083,8 +1083,14 @@
> >      }
> >
> >    if (print_size)
> > -    SVN_ERR (svn_cmdline_printf (pool, "%" APR_SIZE_T_FMT "\n",
> > -                                 prop_value->len));
> > +    {
> > +      /* svn_cmdline_printf will convert to the native locale and eol-style
> > +         for us, but we need the size of the converted message. */
> > +      SVN_ERR (svn_subst_detranslate_string (&prop_value_native, prop_value,
> > +                                             TRUE, pool));
> > +      SVN_ERR (svn_cmdline_printf (pool, "%" APR_SIZE_T_FMT "\n",
> > +                                   prop_value_native->len));
> > +    }
> >
> >    SVN_ERR (svn_cmdline_printf (pool, "%s\n", prop_value->data));
>
> Nearly, but not quite!  This now fails to do EOL conversion on the
> printed log message so multi-line log messages on non-LF systems won't
> be correct, also since svn_subst_detranslate_string does do the EOL
> conversion the lengths won't match.
>
Did you test it? svn_cmdline_printf does LF -> CRLF on platforms that need
it through the C stdio streams. How do you think all our \ns in the code
work?

Regards,
//Peter

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