On Mon, May 14, 2007 at 04:58:56PM -0000, [EMAIL PROTECTED] wrote: > Modified: lyx-devel/branches/personal/sts/dynmacro/src/Cursor.cpp > URL: > http://www.lyx.org/trac/file/lyx-devel/branches/personal/sts/dynmacro/src/Cursor.cpp?rev=18315 > ============================================================================== > --- lyx-devel/branches/personal/sts/dynmacro/src/Cursor.cpp (original) > +++ lyx-devel/branches/personal/sts/dynmacro/src/Cursor.cpp Mon May 14 > 18:58:55 2007 > @@ -1345,4 +1345,5 @@ > } > > > + > } // namespace lyx
Why this extra line? We rarely use three empty lines for separation. > Modified: lyx-devel/branches/personal/sts/dynmacro/src/Cursor.h > URL: > http://www.lyx.org/trac/file/lyx-devel/branches/personal/sts/dynmacro/src/Cursor.h?rev=18315 > ============================================================================== > --- lyx-devel/branches/personal/sts/dynmacro/src/Cursor.h (original) > +++ lyx-devel/branches/personal/sts/dynmacro/src/Cursor.h Mon May 14 18:58:55 > 2007 > @@ -60,7 +60,7 @@ > void leaveInset(Inset const & inset); > /// sets cursor part > void setCursor(DocIterator const & it); > - > + > // > // selection > // There is no need for spaces at the end of lines, let alone a need for adding such. > @@ -402,24 +380,42 @@ > if (!detachedArgs[0].empty()) { > MathData optarg; > asArray(from_ascii("[]"), > optarg); > - // TODO: add check for ] in > detachedArgs[0]. Then we have to put braces around > - optarg.insert(1, > detachedArgs[0]); > + MathData & arg = > detachedArgs[0]; > + > + // look for "]", i.e. put a > brace around? > + InsetMathBrace * brace = 0; > + for( int q = 0; q < > (int)arg.size(); ++q ) { Spacing. Also, 'size_t q' seems to make sense her. > + if (arg[q]->getChar() > == ']') { > + // put brace > + brace = new > InsetMathBrace(); > + break; > + } > + } > + > + // put arg between [] > + if (brace) { > + brace->cell(0) = arg; > + optarg.insert(1, > MathAtom(brace)); > + } else > + optarg.insert(1, arg); > + > + // insert it into the array > insert(p, optarg); > p += optarg.size(); > > // cursor in optional argument > of macro? > - if (slice && slice->pos() == i > && nextSlice && nextSlice->idx() == 0) > - // place inside > optional argument > - placeCursorInThis(cur, > i+2); > - else if (slice && slice->pos() > > i) > + if (macroIdx == 0) { > + if (brace) { > + cur.append(0, > macroPos); > + > cur[macroSlice-1].pos() = i + 2; Spacing. > + } else > + > cur[macroSlice-1].pos() = i + 2 + macroPos; Spacing. > + cur.append(argSlices); > + } else if > (cur[macroSlice-1].pos() >= p) Spacing. > // cursor right of macro > - slice->pos() += > optarg.size(); > - } else { > - // cursor in optional argument > of macro? > - if (slice && slice->pos() == i > && nextSlice && nextSlice->idx() == 0) > - // place directly > behind the macro > - placeCursorInThis(cur, > i+1); > - } > + cur[macroSlice-1].pos() > += optarg.size(); Spacing. > + } else if (macroIdx == 0) > + cur[macroSlice-1].pos() > = i+1; Spacing. Spacing. And a lot more of them below. And again I seem to have missed a part of the discussion. I remember having commented the last patch I saw and there were a few open points. I saw no new patch to the list addressing these open points, but of course I might have missed it. Again: a pointer to the discussion would be appreciated. > +template<class F> > +void fixMacroInstancesFunctional(Cursor const & from, docstring const & > name, > + > F & fix) { > + Cursor dit = from; > + > + for (; dit; dit.forwardPos()) { > + // only until a macro is redefined > + if (dit.inset().lyxCode() == Inset::MATHMACRO_CODE) { > + MathMacroTemplate const & macroTemplate > + = static_cast<MathMacroTemplate const &>(dit.inset()); > + if (macroTemplate.name() == name) > + break; > + } > + > + // in front of macro instance? > + Inset * inset = dit.nextInset(); > + if (inset) { > + InsetMath * insetMath = inset->asInsetMath(); > + if (insetMath) { > + MathMacro * macro = insetMath->asMacro(); > + if (macro && macro->name() == name && > macro->folded()) > + F(macro); > + } > + } > + } > +} Is this function used somewhere in the code? > +void MathMacroTemplate::insertParameter(Cursor & cur, int pos, bool greedy) > +{ > + if (pos <= numargs_ && (pos >= 1 || (!hasOptional_ && pos == 0)) && > numargs_ < 9) { > + ++numargs_; > + shiftArguments(pos, 1); > + > + // append example #n > + cell(defIdx()).push_back(MathAtom(new > MathMacroArgument(pos+1))); > + if (!cell(displayIdx()).empty()) > + cell(displayIdx()).push_back(MathAtom(new > MathMacroArgument(pos+1))); > + > + if (!greedy) { > + Cursor dit = cur; > + dit.leaveInset(*this); > + dit.forwardPosNoDescend(); > + > + // fix macro instances > + fixMacroInstancesAddRemove(dit, name(), pos, true); > + } > + } > +} Spacing. Apart from that is an optional boolean parameter almost always bad design. No, I am not happy. Andre'
