Richard Heck wrote: > The change should be fairly simple, both in the code and in lyx2lyx: > It's mostly a matter of file format, not so much of internal > representation. Internal code concerned with writing and reading will be > all that changes. This will significantly simplify factory.cpp's > readInset() routine.
Only slightly: A few lines of hardcoded ref commands would vanish. > In particular, things like this: > // This strange command allows LyX to recognize "natbib" style > // citations: citet, citep, Citet etc. > // FIXME: We already have partial support for \\fullcite and > // the various \\footcite commands. We should increase the > // file format number and read these commands here, too. > // Then we should use is_possible_cite_command() in > // InsetCitation to test for valid cite commands. > if (compare_ascii_no_case(cmdName.substr(0,4), "cite") == 0) { > inset.reset(new InsetCitation(inscmd)); > } else... > go away. That is a separate issue that can be solved independently. Having said that, the proposed change InsetCommand->InsetCitation makes much sense, but not because of the factory.cpp simplification (it would still need two lines per inset which is OK IMHO), but because of the added flexibility. The bad news is that this change will require one controller class for each inset. Currently there is only one command controller, used by all command insets. That is the only reason I did not do this change already during the InsetCommandParams rewrite. > Note that the issue with is_possible_cite_command() is > essentially the one I've been discussing: Right now, it's hardcoded; and > if it isn't, you might find yourself reading a LyX file that contains a > cite command you didn't know about, so you misread the inset. > > Comments? I guess you probably know that already, but please keep in mind that InsetCommandParams/InsetCommand is in a state of flux: The goal was to decouple the children of InsetCommand from LaTeX, use the name/value interface exlusively in LyX, and concentrate all LaTeX stuff in InsetCommandParams. This was not finished because of the freeze. IMHO the legacy code using the old interface (marked withg FIXME remove) should be converted to the new one. That alone gets rid of quite a bit of ugly code. Another planned, but not implemented bit is support for keyval options (currently not used): An inset does not need to know that they need to be exported to TeX as [a=A,b=B]. It could use the same existing interface for setting/getting these options in InsetCommandParams. Only in the definition of the inset (currently in InsetCommandParams.cpp, but it should be straightforward to read this info from a text file) the type of these parameters would be specified. Then you would never need to parse a "a=A,b=B" string (which is quite complicated if you want to do it correctly with escaping), because in .lyx files these values would be written as separate parameters. Georg