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

Reply via email to