Am Montag, 30. Januar 2006 03:49 schrieb Beck, Andrew Thomas - BECAT001:
> >> --- text3.C 31 Dec 2005 11:40:32 -0000 1.323
> >> +++ text3.C 27 Jan 2006 02:06:30 -0000
> >> @@ -1254,10 +1254,10 @@
> >> else {
> >> string s = cmd.argument;
> >> string const s1 = token(s, ' ', 1);
> >> - int const nargs = s1.empty() ? 0 :
> >convert<int>(s1);
> >> +// int const nargs = s1.empty() ? 0 :
> >convert<int>(s1);
> >> string const s2 = token(s, ' ', 2);
> >> string const type = s2.empty() ?
"newcommand" :
> >s2;
> >> - cur.insert(new MathMacroTemplate(token(s, '
',
> >0), nargs, type));
> >> + cur.insert(new MathMacroTemplate(token(s, '
',
> >0), s1, type));
> >
> >I think it is better not to make the internal storage of the number of
> >arguments visible here. Simply provide an additional constructor with
an
> >int argument that does the conversion.
> >
>
> I'm not certain how passing a string reveals that the value is stored
> internally in the cells_ member variable,
Sorry, I always tend to think that a cell is equivalent to a string.
> but in any event, this change
> was made in order to allow macros to be created dynamically with a
varying
> number of arguments, like this:
>
> \newcommand{\dynMacro}[3]{\newcommand{#1}[#2]{#3}}
>
> Passing an integer to the constructor would make this impossible.
I see. It might still be better to keep the int version of the constructor
for existing code.
> >Here you have one of the broken points that I mentioned in my earlier
> >mail: asMacro() is already defined in math_inset.h and used elsewere,
but
> >always returns 0!
>
> I'm a little confused on this point. I could only find it used in a
couple
> of spots that were #if 0'd out. In any event, those usagages (in
math_macro.C)
> seemed to require that asMacro() returned a pointer to the object in the
event
> that it was an instance of the MathMacro class and 0 otherwise. This is
the
> case with all similar functions declared virtual in MathInset and
overridden in
> the relavant derived class. I thought the fact that asMacro wasn't
declared
> in MathMacro was an accidental ommission.
You are right. Every math inset that needs to be indetifiable has such a
method. I simply wanted to point out one place where the existing macro
stuff is incomplete.
> >The overall approach looks sensible. Provided you get rid of the
> >const_casts and find a way to update existing macros not in the metrics
> >function this would be a nice feature for 1.5.
>
> I will implement this change, but I find it bemusing that people find
updating
> the macro instantiation within the metrics() function so objectionable.
The reason is very simple: Keep the overall design clean. The task of
metrics() is to compute on-screen dimensions and nothing else. Therefore
it is a const method. If we now start to add arbitrary stuff to it, just
because it happens to be called often enough, the result will be a design
that nobody understands anymore.
> Since everybody agreed that this was the best thing to do, I simply
> continued to implement new functions in a consistent way. To me,
changing
> the number of arguments is no different to changing the expansion
> template (& has been requested several times in bugzilla). I looked in
> bugzilla & couldn't find any mention of this being undesirable and the
> original patch was submitted without comment.
This is not at all undesireable.
> ps for what it's worth, I use macro's a lot and actually like the
current
> method of entering arguments. I don't see any problem with the box
expanding
> when you enter the macro usage, except that it's a tad weird when you
just
> cursor through a document, but this is a minor concern.
And it is no problem to change the UI later if somebody has a better idea.
Georg
