Andre Poenitz wrote:
> Have you considered changing that params2string interface to
...
> (or when we are at it even
>
> ostream & InsetBranchMailer::operator<<(ostream & os,
> InsetBranchParams const &)
> {
> os << name << ' ';
> os << name_ << ' ';
> params.write(os);
> // Add all_branches parameter to data:
> os << params.branchlist.allBranches() << "\n";
> return os;
> }
>
> one concept less to grasp and even better re-usable...)
Let's think about this a little more, and let's illustrate this with
examples of the code in use in the core.
1. Example usage of params2string, in LyXFunc::dispatch:
case LFUN_DIALOG_SHOW_NEW_INSET: {
string const name = func.getArg(0);
string data = trim(func.argument.substr(name.size()));
if (name == "foo") {
...
} else if (name == "box") {
// \c data == "Boxed" || "Frameless" etc
InsetBoxParams p(data);
data = InsetBoxMailer::params2string(p);
}
owner->getDialogs().show(name, data, 0);
break;
}
2. Example usage of string2params, in factory.C's createInset:
case LFUN_INSET_INSERT: {
string const name = cmd.getArg(0);
if (name == "bibitem") {
InsetCommandParams icp;
InsetCommandMailer::string2params(cmd.argument, icp);
return new InsetBibitem(icp);
} else ...
In (1) I am turning an 'empty' InsetBoxParams into a string that can
be 'mailed' to the dialog. Ie, it contains all the default values
needed to initialze the dialog.
In (2) I am using the string passed to dispatch from the frontend to
fill an instance of InsetCommandParams.
So, is the code that USES these interfaces ugly? I don't think it is.
Nonetheless, let's proceed and consider whether we could use a
stream-based interface rather than the current string-based one.
The first point to note is that the two functions params2string and
string2params are STATIC. Ie, really they are independent of the
concept of the Mailer which handles communication from the core to
the dialogs.
If we want to use operator>>() syntax then I think that we new classes
to do so:
struct InsetCommandStreamer {
// Note that these constructors must be passed \c name
// because many InsetCommand-derived insets will use this.
// InsetBoxStreamer would not need a \c name argument
// to be passed to the constructors.
// Use this constructor when filling \c params from
// a std::istream.
InsetCommandStreamer(string const & name_)
: name(name_) {}
// Use this constructor when filling a std::ostream
// from params.
InsetCommandStreamer(string const & name_,
InsetCommandParams const & params_)
: name(name_), params(params_) {}
string const name;
InsetCommandParams params;
};
std::ostream & operator<<(std::ostream & os,
InsetCommandStreamer const & ics)
{
os << ics.name << ' ';
ics.params.write(os);
os << "\\end_inset\n";
return os;
}
std::istream & operator>>(std::istream & is,
InsetCommandStreamer & ics)
{
ics.params = InsetCommandParams();
string name;
is >> name;
if (!is || name != ics.name) {
lyxerr << "Bogus data" << std::endl;
return is;
}
// This is part of the inset proper that is usually swallowed
// by LyXText::readInset
string id;
is >> id;
if (!is || id != "LatexCommand") {
lyxerr << "Bogus data" << std::endl;
return is;
}
// Inset::read takes a LyXLex, not an istream
LyXLex lex(0,0);
lex.setStream(is);
ics.params.read(lex);
return is;
}
So, the implementation code is quite neat (but little more so than it
would be if I used your new interface to LyXLex). How about the code
that would use this?
1. Replacement for params2string. Assume that Dialogs::show's second
arument is now 'ostream &' rather than 'string const &'
case LFUN_DIALOG_SHOW_NEW_INSET: {
string const name = func.getArg(0);
string data = trim(func.argument.substr(name.size()));
ostringstream ss;
if (name == "foo") {
...
} else if (name == "box") {
// \c data == "Boxed" || "Frameless" etc
ss << InsetBoxStreamer(InsetBoxParams(data));
}
owner->getDialogs().show(name, ss, 0);
break;
}
2. Replacement for string2params.
case LFUN_INSET_INSERT: {
string const name = cmd.getArg(0);
if (name == "bibitem") {
istringstream is(cmd.argument);
InsetCommandStreamer ics(name);
is >> ics;
return new InsetBibitem(ics.params);
} else ...
Is this demonstrably better than the interface above? I don't think
that there's much in it actually.
My personal conclusion is that the present interface is sufficient,
but I'm willing to hear your arguments too.
--
Angus
