On Thu, May 3, 2012 at 11:24 AM, Michael Stahl <mst...@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 _______________________________________________ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice