Re: [PUSHED] Re: [PATCH] fdo#31005: Table Autoformats does not save/apply all properties

On Thu, May 3, 2012 at 11:24 AM, Michael Stahl <mstahl@redhat.com> wrote:
> sorry for taking so long, but i had hoped that somebody else would
> perhaps be more familiar with table autoformats and review this rather
> substantial patch :)

No problem. I'm improving my knowledge of AutoFormats and documenting
it as I go, so hopefully it'll become clearer to anyone who reads the
code.

> couldn't find anything obviously wrong with your patch; other than the
> email address which was invalid so i've replaced it with your gmail
> address, perhaps you have not configured git properly, try "git config
> user.email".

Ah, thanks for pointing it out. I updated my configuration.

> i did some trivial tweaks to your patch, to follow our coding standards
> added an m_ prefix to all new member variables, which makes things much
> easier to read (because with the implicit "this" you never know what
> kind of side-effects an assignment has); also i've split out the Undo
> thing into a separate commit.

Cool. I actually tried to follow the convention that already existed
in the code. I prefer member variable prefixes as well. (I have no
clue what the 'a' prefix means).
Is there an official style guideline somewhere?

> it is rather surprising and sub-optimal that all this autoformat stuff
> uses an awful binary format based on direct serialization of
> implementation details; this is very brittle and it would be nice
> for example, if i read the autotbl.fmt with your patch in an old LO
> without the patch, the old version cannot read it at all.
> a text-based format (e.g. XML) would be much nicer, in case you want to
> do further work in this area :)

Sure, I opened a bug:
"Writer/Calc: Use XML for serializing table autoformats instead of the
current brittle binary format"
https://bugs.freedesktop.org/show_bug.cgi?id=49437

Regards,
--Muhammad