Re: [cdt-dev] Fix for bug 307311

[Mathias Kunter <mathiaskunter@xxxxxxxxx>]

Marc,

thanks for your response. Reviewing is hopefully not a big deal since 
the code contains many comments.

> It is up to you if you want to add this feature to both integration 
of GDB
> or only one of them.

Okay, but isn't it ugly to have duplicate code within DSF and CDI?

Thanks,
Mathias


Am 20.10.2011 19:51, schrieb Marc Khouzam:
> > -----Original Message-----
> > From: cdt-dev-bounces@xxxxxxxxxxx
> > [mailto:cdt-dev-bounces@xxxxxxxxxxx] On Behalf Of Mathias Kunter
> > Sent: Friday, October 14, 2011 1:37 PM
> > To: cdt-dev@xxxxxxxxxxx
> > Subject: [cdt-dev] Fix for bug 307311
>
> Hi Mathias,
>
> apologies for not being able to answer earlier.
> It is nice to see your effort for this contribution
> and I hope a committer will have time to review it soon.
>
> I personally have no time right now, but I'll keep this
> in the back of my mind if time does become available and no
> one else has jumped in.
>
> > I've already posted to this mailing list some weeks ago, but didn't
> > receive any reply so far - so I'm trying again now: I've fixed bug
> > 307311 and attached a corresponding patch. Please see
> > https://bugs.eclipse.org/bugs/show_bug.cgi?id=307311
> >
> > The fix brings UTF-8 Unicode support to the Eclipse CDT
> > debugging views.
> >
> > The patch works well for me, but a short review might cover the
> > following things:
> >
> > 1) Changes have been made to the MIConst.java file contained within
> > the package org.eclipse.cdt.dsf.mi.service.command.output, but what
> > about the package org.eclipse.cdt.debug.mi.core.output? Does it have
> > to be changed, too?
>
> org.eclipse.cdt.dsf.mi.service.command.output is used for the DSF integration of GDB while
> org.eclipse.cdt.debug.mi.core.output is used for the CDI integration of GDB.
> CDT uses the DSF integration by default, but the CDI one is still available if the
> user selects it explicitly.
>
> It is up to you if you want to add this feature to both integration of GDB
> or only one of them.  Doing it in the DSF one (as you have done) will make
> it available by default, which is nice.
>
> Marc
>
> > 2) MIVarEvaluateExpressionInfo and MIDataEvaluateExpressionInfo should
> > probably call MIConst.getString instead of MIConst.getCString in order
> > to support UTF-8 for evaluated expressions too, if this has no other
> > side effects (I didn't notice any, at least).
> >
> > 3) This patch is related to the following gdb options:
> > -->  gdb's "host-charset" must be set to "UTF-8" (if it's not, the
> > patch possibly can't recognize all UTF-8 strings correctly). As far as
> > I know, this is gdb's default setting on Linux (and Mac?), but not on
> > Windows. CDT users on Windows can set this via the .gdbinit file, but
> > a separate Eclipse preference option would probably be handier.
> > -->  gdb's "print sevenbit-strings" must be set to "on" (again, if it's
> > not, the patch possibly can't recognize all UTF-8 strings correctly).
> > This is the default setting for the gdb/mi interface on all platforms
> > as far as I know.
> >
> > I'd be happy about any feedback / comments. Thank you very much for
> > your valued time and work.
> >
> > Best regards,
> > Mathias Kunter
> > _______________________________________________
> > cdt-dev mailing list
> > cdt-dev@xxxxxxxxxxx
> > https://dev.eclipse.org/mailman/listinfo/cdt-dev
> _______________________________________________
> cdt-dev mailing list
> cdt-dev@xxxxxxxxxxx
> https://dev.eclipse.org/mailman/listinfo/cdt-dev