Re: UString and locale conversion

[Jonathon Jongsma <jonathon quotidian org>]

Dodji Seketeli wrote:
> Hello,
>
> Sorry, I am forwarding the patch review to the list as there is no bug open
> opened for this issue yet. Maybe we should open one to track the progress of this ?
>
> From the first patch (90e7ac8d4ff7fddf9ccbae368f3425d35b0528eb):
>
> > diff --git a/src/common/nmv-ustring.h b/src/common/nmv-ustring.h
> > index bb98e87..eca1a6f 100644
> > --- a/src/common/nmv-ustring.h
> > +++ b/src/common/nmv-ustring.h
> > @@ -50,7 +50,7 @@ public:
> >      UString (const char *a_cstr, long a_len=-1) ;
> >      UString (const unsigned char *a_cstr, long a_len=-1) ;
> >      UString (const Glib::ustring &an_other_string) ;
> > -    UString (const string &an_other_string) ;
> > +    explicit UString (const string &an_other_string) ;
>
>
> Now that the UString is not doing any character set conversion anymore, is there
> a strong reason to keep this constructor explicit ?

Yeah, I was thinking that as well.  I suppose there's not a very compelling reason to keep 
it explicit any longer.

> > --- a/src/dbgengine/nmv-gdb-engine.cc
> > +++ b/src/dbgengine/nmv-gdb-engine.cc
> > @@ -529,11 +529,11 @@ public:
>
> [snip]
>
> > -    var.reset (new IDebugger::Variable (var_name));
> > +    var.reset (new IDebugger::Variable (UString (var_name)));
>
> See, not having the constructor I talked about above would save us from doing
> this change. This applies to many more changes in that patch.
>
> From the second patch:
>
> > diff --git a/src/dbgengine/nmv-gdb-engine.cc b/src/dbgengine/nmv-gdb-engine.cc
> > index e45b6c9..4de2b4f 100644
> > --- a/src/dbgengine/nmv-gdb-engine.cc
> > +++ b/src/dbgengine/nmv-gdb-engine.cc
> > @@ -524,7 +524,8 @@ public:
>
> [snip]
>
> > @@ -3487,6 +3488,7 @@ fetch_variable:
> >      }
> >      LOG_DD ("globals: got variable name: " << var_name );
> >
> > +    // TODO: What encoding is var_name?
> >      var.reset (new IDebugger::Variable (UString (var_name)));
>
>
> var_name is in ASCII for now :) So we shouldn't care too much I guess.
> In the future though, with the combination of the facts that:
>
> 1/some languages can have variables non ASCII variables, who knows.
> 2/GDB can be instructed to output its data in a given charset
>
> we can force GDB to ouput utf8 data.

Right, I guess I was just curious why you chose not to use UString in the function 
interface for your C++ parsing utility functions but you use UString in almost every other 
interface?

> > --- a/src/persp/dbgperspective/nmv-dbg-perspective.cc
> > +++ b/src/persp/dbgperspective/nmv-dbg-perspective.cc
> > @@ -980,7 +980,7 @@ struct DBGPerspective::Priv {
> >
> >          UString buf_content;
> >          if (is_buffer_valid_utf8 (a_input.c_str (), a_input.size ())) {
> > -            a_output = UString(a_input.c_str ());
> > +            a_output = UString(a_input);
>
> I guess it would be nice to take the opportunity to insert a space a space
> between UString and the '(', just to comply with the GNOME-ish coding style.

sure

--
jonner