Re: [cdt-dev] Fix for bug 307311

[Marc Khouzam <marc.khouzam@xxxxxxxxxxxx>]

> -----Original Message-----
> From: cdt-dev-bounces@xxxxxxxxxxx 
> [mailto:cdt-dev-bounces@xxxxxxxxxxx] On Behalf Of Mathias Kunter
> Sent: Saturday, October 22, 2011 11:22 AM
> To: CDT General developers list.
> Subject: Re: [cdt-dev] Fix for bug 307311
> 
> 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?

We are not trying to share code between DSF and CDI, and you will
find *a lot* of code duplication.  DSF was started in a separate
project, which may explain in part why things are the way they are.

Officially, we add new features to DSF and don't put too much
effort on CDI to avoid duplicating the effort.  But if you feel
like adding your feature to CDI, that is absolutly fine, as long as
you can find someone that has the time to review and include it.  

Thanks

Marc

> 
> 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
> _______________________________________________
> cdt-dev mailing list
> cdt-dev@xxxxxxxxxxx
> https://dev.eclipse.org/mailman/listinfo/cdt-dev
>