>> --- 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, 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.
>> - MacroTable::globalMacros().get(name()).expand(cells_,
>expanded_);
>> - expanded_.metrics(mi, dim);
>> + const idx_type defArgs =
>static_cast<idx_type>(MacroTable::globalMacros().get(name()).numargs());
>
>That should work without the cast.
You're right. I did it originally to avoid a warning, but it's not required.
>> --- mathed/math_macro.h 5 Oct 2005 21:19:32 -0000 1.103
>> +++ mathed/math_macro.h 27 Jan 2006 02:06:39 -0000
>> @@ -55,10 +55,14 @@
>> ///
>> void infoize2(std::ostream &) const;
>>
>> -private:
>> - virtual std::auto_ptr<InsetBase> doClone() const;
>> + virtual MathMacro *asMacro() { return this; }
>> + virtual MathMacro const *asMacro() const { return this; }
>
>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.
>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. Prior
to implementing this patch, I read this thread
http://www.mail-archive.com/lyx-devel@lists.lyx.org/msg68709.html
in particular,
>The natural solution of a LaTeX-style macro implementation would be to
>store only the name of the macro in an inset whenever it is used and
>have the metrics() and draw() calls of the containing MathArray (which
>have access to a Buffer & after all!) pick up the correct number of
>parameters behind the macro name, build an 'expanded' version from
>these, and compute the metrics of and draw it, respectively. This is
>highly flexible, it is almost exactly what TeX does, and works as
>patch 2 shows. However, editing the thing is not nice for macros with
>arguments (yes, even uglier than the exploding box we have now...).
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.
cheers,
andrew
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.
