On Mon, Sep 24, 2007 at 02:36:29PM -0400, Richard Heck wrote:
> Martin Vermeer wrote:
> >On Mon, Sep 24, 2007 at 06:31:03AM +0300, Martin Vermeer wrote:
> >
> >The proper way for this is, it seems, to keep the hardwired InsetIndex,
> >but make it a collapsable. The attached patch does this. The inset is
> >now configurable from the layout files. It still works without feature
> >loss.
> >
> Right. This seems the right thing to do.
>
> Comments on the patch...
>
> >Index: src/insets/InsetIndex.h
> >===================================================================
> >--- src/insets/InsetIndex.h (revision 20348)
> >+++ src/insets/InsetIndex.h (working copy)
> >@@ -13,6 +13,7 @@
> > #define INSET_INDEX_H
> >
> >
> >+#include "InsetCollapsable.h"
> > #include "InsetCommand.h"
> >
> That last line can presumably be removed.
Nope... there's InsetPrintIndex too.
> >Index: src/factory.cpp
> >===================================================================
> >--- src/factory.cpp (revision 20348)
> >+++ src/factory.cpp (working copy)
> >@@ -289,11 +283,8 @@
> >- } else if (name == "index") {
> >- InsetCommandParams icp(name);
> >- InsetCommandMailer::string2params(name,
> >to_utf8(cmd.argument()),
> >- icp);
> >- return new InsetIndex(icp);
> >+ } else if (name == "Index") {
> >+ return new InsetIndex(params);
> >
> There's no real reason to change this to uppercase, and it could cause
> problems elsewhere to change it. These strings should be kept in sync
> with the list in Inset.cpp---that's really how they're being used---and
> I think it really ought to work a bit differently here, but that's what
> I'm working on now. The idea is to do this:
OK
> const Inset::Code code = Inset::translate(name);
> if (code == Inset::INDEX_CODE) {
> ...
> } else if (code == Inset::BIBITEM_CODE) {
> ...
> } else { ...error...}
> This long list of strings just looks like magic.
> > @@ -414,8 +405,9 @@
> > inset.reset(new InsetBibitem(inscmd));
> > } else if (cmdName == "bibtex") {
> > inset.reset(new InsetBibtex(inscmd));
> >+ // (Is this stuff obsolete)?
> > } else if (cmdName == "index") {
> >- inset.reset(new InsetIndex(inscmd));
> >+ inset.reset(new InsetIndex(buf.params()));
> > } else if (cmdName == "nomenclature") {
> > inset.reset(new InsetNomencl(inscmd));
> > } else if (cmdName == "include") {
> >
> Yes, all of that is obsolete. That part of the code is only called for
> command insets.
So... just leave index out here?
> >@@ -517,6 +509,8 @@
> > #endif
> > } else if (tmptok == "Caption") {
> > inset.reset(new InsetCaption(buf.params()));
> >+ } else if (tmptok == "Index") {
> >+ inset.reset(new InsetIndex(buf.params()));
> > } else if (tmptok == "FloatList") {
> > inset.reset(new InsetFloatList);
> > } else {
> >
> That is fine as uppercase. (Probably that was obvious, but don't want to
> cause you any more confusion than I caused my students this morning.)
Ah, you too?
;-)
- Martin
