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 ?
> --- 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.
> --- 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.
Thanks,
Dodji.
_______________________________________________
nemiver-list mailing list
[email protected]
http://mail.gnome.org/mailman/listinfo/nemiver-list
