On Thu, Dec 01, 2016 at 12:35:49AM +0100, Enrico Forestieri wrote: > On Wed, Nov 30, 2016 at 05:23:59PM +0100, Enrico Forestieri wrote: > > On Wed, Nov 30, 2016 at 10:40:09AM -0500, Scott Kostyshak wrote: > > > > > > Thanks for the explanations. It seems we should report the LyX display > > > bug you mention above, and then just not worry about the compilation > > > failures. > > > > I found that enclosing the nested macros in brace insets circumvents > > both issues (display glitch and latex error). Unfortunately, lyx duly > > removes the braces when reloading the document, so that this only > > takes care of the display issue, in practice. See attached. > > The attached patch fixes the display glitch for me. > > > So, the conclusion is that the error is actually due to a limitation > > of xkeyval. > > The only way to avoid this error still remains enclosing a nested macro > and its parameters in a brace inset. However, this workaround will be > frustrated by lyx on reloading the document.
Or we can add the braces on output, as done in the attached patch (also including the previous one). The braces will be added anytime a macro has optional arguments (not only when it is nested), but, given that this avoids a latex error, I think it is acceptable. The test case now displays and compiles fine for me. -- Enrico
diff --git a/src/mathed/MathData.cpp b/src/mathed/MathData.cpp index f2100af..44a5055 100644 --- a/src/mathed/MathData.cpp +++ b/src/mathed/MathData.cpp @@ -659,12 +659,15 @@ void MathData::collectOptionalParameters(Cursor * cur, if (operator[](pos)->getChar() != '[') break; - // found possible optional argument, look for "]" + // found possible optional argument, look for pairing "]" + int count = 1; size_t right = pos + 1; for (; right < size(); ++right) { MathAtom & cell = operator[](right); - if (cell->getChar() == ']') + if (cell->getChar() == '[') + ++count; + else if (cell->getChar() == ']' && --count == 0) // found right end break; diff --git a/src/mathed/MathMacro.cpp b/src/mathed/MathMacro.cpp index 34b5160..af24634 100644 --- a/src/mathed/MathMacro.cpp +++ b/src/mathed/MathMacro.cpp @@ -1068,6 +1068,11 @@ void MathMacro::write(WriteStream & os) const // we should be ok to continue even if this fails. LATTEST(d->macro_); + // Enclose in braces to avoid errors with xargs in case the + // optional argument is another macro with optional arguments + if (d->optionals_) + os << '{'; + // Always protect macros in a fragile environment if (os.fragile()) os << "\\protect"; @@ -1106,8 +1111,10 @@ void MathMacro::write(WriteStream & os) const first = false; } - // add space if there was no argument - if (first) + // Close the opened brace or add space if there was no argument + if (d->optionals_) + os << '}'; + else if (first) os.pendingSpace(true); }