Re: Commit: some of Larin's work

From: Marc Maurer (j.m.maurer@student.utwente.nl)
Date: Sat Mar 08 2003 - 06:06:28 EST

  • Next message: Marc Maurer: "Commit: Bug 4664"

    Hi Larin,

    Thx for doing all this cleanup work! Just a note: just ignore the WP
    exporter for now. Most of the mess will be cleaned up when the exporter
    is moved to use libwpd as well. A lot of the mess comes from reverse
    enginering the file format, and I cared more for the actual
    functionality of the code than I cared for the readability. But it will
    be cleaned up in the future, promise.

    Any cleanups to the importer are welcomed of course.

    Marc

    Op za 08-03-2003, om 02:08 schreef Dom Lachowicz:
    > Using this is fine (even encouraged by me):
    >
    > static const foo_t foo = somevalue;
    >
    > However, AbiWord's coding guidelines forbid the use of
    > C++ templates, and even if we do decide to use them,
    > it's not happening before 2.0 is out the door. All
    > patches to this effect will be sent to /dev/null
    >
    > Also, note that these types of macros:
    >
    > 1) Usually set some member variable to an error state
    > (this one does)
    > 2) Return from the calling method (this one does too)
    >
    > Template functions won't suffice here and aren't an
    > applicable solution to the problem at hand.
    >
    > Dom
    >
    > --- Larin Hennessy <larin@science.oregonstate.edu>
    > wrote:
    > > OK, another C -> C++ change comes to mind. What
    > > about
    > > changing #define to const and/or inline template
    > > functions
    > > depending on the situation?
    > >
    > > For example, the WordPerfect importer/exporter has a
    > > group
    > > of formatting codes in ie_impexp_WordPerfect.h:
    > >
    > > #define WP_TOP_SOFT_SPACE 128 // (0x80)
    > > ...
    > > #define WP_TOP_DISPLAY_NUMBER_REFERENCE_GROUP 0xDA
    > >
    > > Should these be converted to:
    > >
    > > const char WP_TOP_SOFT_SPACE = 0x80;
    > > ...
    > > const char WP_TOP_DISPLAY_NUMBER_REFERENCE_GROUP =
    > > 0xDA;
    > > ?
    > >
    > > It seems that if these values represent character
    > > values in files, using
    > > the hex
    > > representation would be more intuitive.
    > >
    > > I would also like to look at the feasability of
    > > changing some of the
    > > define statements
    > > to templates. For example in ie_imp_DocBook.cpp
    > > there is X_CheckDocument:
    > >
    > > #define X_CheckError(v) do { if
    > > (!(v)) \
    > >
    > > {
    > > UT_DEBUGMSG(("DOM: X_CheckError failed: %s\n", #v));
    > > \
    > >
    > >
    > > m_error = UT_ERROR; \
    > >
    > >
    > > return; } } while (0)
    > >
    > > into:
    > >
    > > UT_Error X_CheckError(char * v)
    > > {
    > > do
    > > {
    > > if (!v)
    > > {
    > > UT_DEBUGMSG("DOM:_CheckError failed:
    > > %s\n", v);
    > > return UT_ERROR;
    > > }
    > > } while (0);
    > > }
    > >
    > > Note that the above is just an idea and has not been
    > > tested.
    > >
    > > Any thoughts on wether this would be acceptable?
    > >
    > > -Larin
    > >
    > > Dom Lachowicz wrote:
    > >
    > > >>I think Larin meant ie_exp_WordPerfect.cpp, which
    > > >>does have such a list.
    > > >>Marc would know more about this (he wrote the
    > > code),
    > > >>but I believe the
    > > >>magic is only a temporary hack until we actually
    > > >>figure out what the
    > > >>magic means. :-) It should become history..
    > > >>eventually.
    > > >>
    > > >>
    > > >
    > > >Indeed it does. The (char) should stay for the
    > > above
    > > >reason, and the fact that 192
    > > reinterpret_cast<char>()
    > > >is much much worse than 192 (char) from a
    > > readability
    > > >perspective. reinterpret_cast gains us nothing
    > > here.
    > > >
    > > >On a related note, I just committed a dos2unix
    > > version
    > > >of exp_WordPerfect.cpp to remove the ^M problem.
    > > >
    > > >Dom
    > > >
    > > >__________________________________________________
    > > >Do you Yahoo!?
    > > >Yahoo! Tax Center - forms, calculators, tips, more
    > > >http://taxes.yahoo.com/
    > > >
    > > >
    > >
    > >
    >
    >
    > __________________________________________________
    > Do you Yahoo!?
    > Yahoo! Tax Center - forms, calculators, tips, more
    > http://taxes.yahoo.com/

    -- 
    Marc Maurer <j.m.maurer@student.utwente.nl>
    


    This archive was generated by hypermail 2.1.4 : Sat Mar 08 2003 - 06:07:16 EST