Re: test of math previews

[Enrico Forestieri]

On Fri, Jun 05, 2015 at 09:40:39AM +0200, Jean-Marc Lasgouttes wrote:

 Le 05/06/2015 09:14, Enrico Forestieri a écrit :
 A possible way out would be outputting all the macros once at the
 beginning of the file, such that they are seen by all snippets, and then
 the specific one appearing in a math inset (as done in the patch I was
 proposing). I'll have a look whether this can be done in a simple way.
 
 Isn't it possible to output them as they are output by the normal latex
 export? Of course, this would mean that creating previews would be a very
 different process, more like a special version of our latex output code,
 instead of some registration done by the insets themselves.

I don't think that is possible. The point is that the previews should be
updated whenever they change. Now, they can change even if the math inset
was not modified, because one could have modified a macro. Thus, the
snippets have also to contain the macros they use.

I tried to solve this problem by including in a snippet only the macros that
are actually used. I almost succeeded with the attached patch for stable
(master would require more surgery). I say almost because it seems that
only the macros used by the current macro are spotted, but not the ones
that, in turn, are used by them. I mean, after

\newcommand{\first}{pluto}
\newcommand{\second}{goofy\,and\,\first}
\newcommand{\third}{mickey,\,\second}
$\third$

the macros \second and \third are spotted but not \first. See also the
attached examples. After activating instant previews, when loading
lyx-preview-macros-2.lyx you will correctly see

Found: first
Found: second

but loading lyx-preview-macros-3.lyx produces

Found: second
Found: third
Warning: Failed to produce 1 preview snippet(s)

Essentially, the patch goes recursively through the definition of the
macros found in an inset, but it seems that the definition of second
level macros are empty? I am not so familiar with the code dealing
with math macros, but I thought that their definition in terms of a
LyX inset should be held in MacroData::definition_. However, it does
not seem to (always) be so?

Any help or suggestion will be appreciated.

-- 
Enrico
diff --git a/src/mathed/InsetMathHull.cpp b/src/mathed/InsetMathHull.cpp
index d4bdf87..a7db85f 100644
--- a/src/mathed/InsetMathHull.cpp
+++ b/src/mathed/InsetMathHull.cpp
@@ -31,6 +31,7 @@
 #include Language.h
 #include LaTeXFeatures.h
 #include LyXRC.h
+#include MathMacro.h
 #include MacroTable.h
 #include output_xhtml.h
 #include Paragraph.h
@@ -622,6 +623,18 @@ void InsetMathHull::addPreview(DocIterator const  
inset_pos,
 }
 
 
+void InsetMathHull::usedMacros(MathData const  md, MacroNameSet  macros) 
const
+{
+   for (size_t i = 0; i  md.size(); ++i) {
+   MathMacro * mi = const_castMathMacro 
*(md[i].nucleus()-asMacro());
+   if (mi) {
+   macros.insert(mi-name());
+   usedMacros(mi-definition(), macros);
+   }
+   }
+}
+
+
 void InsetMathHull::preparePreview(DocIterator const  pos,
bool forexport) const
 {
@@ -632,13 +645,17 @@ void InsetMathHull::preparePreview(DocIterator const  
pos,
 
Buffer const * buffer = pos.buffer();
 
-   // collect macros at this position
+   // collect macros used in this inset
MacroNameSet macros;
-   buffer-listMacroNames(macros);
+   for (row_type row = 0; row  nrows(); ++row)
+   for (col_type col = 0; col  ncols(); ++col)
+   usedMacros(cell(index(row, col)), macros);
+
MacroNameSet::iterator it = macros.begin();
MacroNameSet::iterator end = macros.end();
odocstringstream macro_preamble;
for (; it != end; ++it) {
+   lyxerr  Found:   *it  endl;
MacroData const * data = buffer-getMacro(*it, pos, true);
if (data) {
data-write(macro_preamble, true);
diff --git a/src/mathed/InsetMathHull.h b/src/mathed/InsetMathHull.h
index 0cd..65cb0ea 100644
--- a/src/mathed/InsetMathHull.h
+++ b/src/mathed/InsetMathHull.h
@@ -25,6 +25,7 @@ namespace lyx {
 class InsetLabel;
 class ParConstIterator;
 class RenderPreview;
+class MacroNameSet;
 
 
 /// This provides an interface between LyX insets and LyX math insets
@@ -190,6 +191,8 @@ private:
/// used by image export
void loadPreview(DocIterator const  pos) const;
///
+   void usedMacros(MathData const  md, MacroNameSet  macros) const;
+   ///
void setType(HullType type);
///
void validate1(LaTeXFeatures  features);
diff --git a/src/mathed/MathMacro.h b/src/mathed/MathMacro.h
index 239495e..18048d3 100644
--- a/src/mathed/MathMacro.h
+++ b/src/mathed/MathMacro.h
@@ -135,6 +135,7 @@ protected:
friend class MathData;
friend class ArgumentProxy;
friend class Cursor;
+   friend class InsetMathHull;
 
/// update the 

Re: test of math previews

[Enrico Forestieri]

On Fri, Jun 05, 2015 at 11:03:48PM +0100, Guillaume M-M wrote:

 Le 05/06/2015 08:14, Enrico Forestieri a écrit :
 On Thu, Jun 04, 2015 at 07:32:03PM -0400, Richard Heck wrote:
 On 06/04/2015 07:13 PM, Guillaume M-M wrote:
 
 Bad news, it seems to forget that macros can use macros. With the attached
 file the preview is as follows:
 
 \begin{preview}
 \global\long\def\b{\a}
 
 $\b$
 \end{preview}
 
 I suspect this is why all macros are included.
 
 Most probably. This leaves us pretty stuck with the problem caused by
 having hundreds of macros. I will not commit any change to stable until
 we find a good solution. After all, this is what already happens now
 and it is not a regression. I think the complaint is due to the fact
 that now also the previews get zoomed and this may cause relevant delays,
 
 Enrico mentions a complaint. There seems to be a misunderstanding. The
 freeze on zoom is a regression in stable, while the time explosion was
 already there. This is only a factual report of the issues trying to be the
 most helpful. The reports are the outcome of my tests of the preview
 mechanism following Enrico's request of having his patch at
 http://www.lyx.org/trac/ticket/9354 tested.
 
 In truth, I have no interest in full preview, only in the solution to #9354,
 and my tests concluded that there are no immediate issues with his patch,
 while I managed to find other issues---many thanks to Enrico and Jürgen for
 fixing them promptly. Fixing #9354 (preview inset does not work with math
 macros) in 2.1.4 would make the preview mechanism useful again in my
 context, avoiding the performance issues. (Especially because I cannot ask
 my collaborators to compile from git.) Apart from that, only this regression
 regarding the zoom/preview interaction remains regarding the stable branch
 (to summarise the thread, since you probably want to move forward with
 2.1.4).
 
 After this thread dies off, I may record the remaining important issues in
 the bug tracker, but again please do not regard such reports as complaints!

When you don't speak/write in your native language often you end up
conveying what you are able to and not what you actually mean.
And email doesn't help in this respect.

I didn't want to attach any negative meaning to the word complaint and,
morever, maybe I failed to actually properly acknowledge your help in this
matter. Indeed, you made a not so common testing work and helped discovering
issues that were escaping my (deemed) extensive testing.

That said, I was confident in applying all the changes involving only
touching the scripts because they solve some long standing issues without
the risk of introducing more severe issues. I think you tested the patch
for #9354 very well and I now feel confident that it is safe, but it is
the responsibility of the stable maintainer taking a decision about it.

Also take into account that, once the problem of the size explosion in the
presence of hundreds of macros is solved, it will have to be adapted, too.

-- 
Enrico

Re: test of math previews

[Kornel Benko]

Am Samstag, 6. Juni 2015 um 13:13:22, schrieb Enrico Forestieri for...@lyx.org
 +void InsetMathHull::usedMacros(MathData const  md, MacroNameSet  macros) 
 const
 +{
 +   for (size_t i = 0; i  md.size(); ++i) {
 +   MathMacro * mi = const_castMathMacro 
 *(md[i].nucleus()-asMacro());
 +   if (mi) {
 +   macros.insert(mi-name());
 +   usedMacros(mi-definition(), macros);

I would change the order, to get the used macro first.

 +   }
 +   }
 +}
 +

This may not help with empty second level macros though.

Kornel




signature.asc
Description: This is a digitally signed message part.

Re: include bibtexall in lib/scripts ?

[Jürgen Spitzmüller]

2015-06-06 4:07 GMT+02:00 Scott Kostyshak skost...@lyx.org:

 Would it make sense to ship bibtexall with LyX? It is used to run
 bibtex on all .aux files. It is especially useful with chapterbib,
 which allows a separate bibliography per chapter.


Hm, I would prefer proper chapterbib support, actually.

At least, we could implement the script's functionality in LaTeX.cpp rather
than using the script itself.



 We mention it here:
 http://wiki.lyx.org/BibTeX/Tips
 but unfortunately the instructions do not work because we now use a
 combo box for the bibliography generation processor and I don't think
 one can enter a custom command.


You can, though only in prefs.



 One can change the bibtex command (as Jürgen says to do in his version
 of bibtexall), but then the .lyx file is even less portable.

 variants of bibtexall are actually shipped by (atleast) two LaTeX packages:
 /usr/local/texlive/2015/texmf-dist/doc/latex/bibunits/bibtexall
 /usr/local/texlive/2015/texmf-dist/doc/latex/multibib/bibtexall


Interesting. I did not notice this rather ad-hoc script of mine was picked
up by distributions.

Jürgen



 I am attaching the script. It was written Jürgen and tweaked by me.

 Thoughts?

 Scott

Re: assertion on master when deleting an empty chunk from inside

[Kornel Benko]

Am Freitag, 5. Juni 2015 um 17:26:12, schrieb Scott Kostyshak skost...@lyx.org
 To reproduce, on master open mwe.lyx, place the cursor inside of the
 empty chunk, and press Backspace.
 
 Can others reproduce?
 
 Note also that when I export mwe.lyx to 2.1.x format, the separator is
 gone and there is an ERT with whitespace in it.

Yes to both.

 I don't know if this
 is incorrect and if so if it is related to the assertion.
 
 Scott

Kornel

signature.asc
Description: This is a digitally signed message part.

Re: test of math previews

[Enrico Forestieri]

On Sat, Jun 06, 2015 at 03:23:48PM +0200, Kornel Benko wrote:

 Am Samstag, 6. Juni 2015 um 13:13:22, schrieb Enrico Forestieri 
 for...@lyx.org
  +void InsetMathHull::usedMacros(MathData const  md, MacroNameSet  macros) 
  const
  +{
  +   for (size_t i = 0; i  md.size(); ++i) {
  +   MathMacro * mi = const_castMathMacro 
  *(md[i].nucleus()-asMacro());
  +   if (mi) {
  +   macros.insert(mi-name());
  +   usedMacros(mi-definition(), macros);
 
 I would change the order, to get the used macro first.

No, that is correct. The top level macro has to appear in the last position.

I failed to understand why that was not working but, most probably, it has
to do with the fact not all data is copied over to a cloned buffer.
However, I found a different way for collecting only the macros used
in a given math inset, so I did not investigate it further.

The attached patch for stable works for me and avoids the size explosion
when a very lot of macros are defined in a document. I count on Guillaume
to give it a thourough test. If he doesn't find glitches I will feel
confident about proposing to apply it for 2.1.4.

-- 
Enrico
diff --git a/src/mathed/InsetMathHull.cpp b/src/mathed/InsetMathHull.cpp
index d4bdf87..c23d2f4 100644
--- a/src/mathed/InsetMathHull.cpp
+++ b/src/mathed/InsetMathHull.cpp
@@ -31,6 +31,7 @@
 #include Language.h
 #include LaTeXFeatures.h
 #include LyXRC.h
+#include MathMacro.h
 #include MacroTable.h
 #include output_xhtml.h
 #include Paragraph.h
@@ -622,6 +623,28 @@ void InsetMathHull::addPreview(DocIterator const  
inset_pos,
 }
 
 
+void InsetMathHull::usedMacros(MathData const  md, DocIterator const  pos,
+   MacroNameSet  macros) const
+{
+   for (size_t i = 0; i  md.size(); ++i) {
+   MathMacro const * mi = md[i].nucleus()-asMacro();
+   if (mi) {
+   MathData ar(pos.buffer());
+   MacroData const * data =
+   pos.buffer()-getMacro(mi-name(), pos, true);
+   if (data) {
+   odocstringstream macro_def;
+   data-write(macro_def, true);
+   macro_def  endl;
+   macros.insert(macro_def.str());
+   asArray(data-definition(), ar);
+   }
+   usedMacros(ar, pos, macros);
+   }
+   }
+}
+
+
 void InsetMathHull::preparePreview(DocIterator const  pos,
bool forexport) const
 {
@@ -632,19 +655,17 @@ void InsetMathHull::preparePreview(DocIterator const  
pos,
 
Buffer const * buffer = pos.buffer();
 
-   // collect macros at this position
+   // collect macros used in this inset
MacroNameSet macros;
-   buffer-listMacroNames(macros);
+   for (row_type row = 0; row  nrows(); ++row)
+   for (col_type col = 0; col  ncols(); ++col)
+   usedMacros(cell(index(row, col)), pos, macros);
+
MacroNameSet::iterator it = macros.begin();
MacroNameSet::iterator end = macros.end();
-   odocstringstream macro_preamble;
-   for (; it != end; ++it) {
-   MacroData const * data = buffer-getMacro(*it, pos, true);
-   if (data) {
-   data-write(macro_preamble, true);
-   macro_preamble  endl;
-   }
-   }
+   docstring macro_preamble;
+   for (; it != end; ++it)
+   macro_preamble.append(*it);
 
docstring setcnt;
if (forexport  haveNumbers()) {
@@ -667,8 +688,7 @@ void InsetMathHull::preparePreview(DocIterator const  pos,
  '{' + convertdocstring(num) + '}';
}
}
-   docstring const snippet = macro_preamble.str() +
-   setcnt + latexString(*this);
+   docstring const snippet = macro_preamble + setcnt + latexString(*this);
LYXERR(Debug::MACROS, Preview snippet:   snippet);
preview_-addPreview(snippet, *buffer, forexport);
 }
diff --git a/src/mathed/InsetMathHull.h b/src/mathed/InsetMathHull.h
index 0cd..0b9e5fd 100644
--- a/src/mathed/InsetMathHull.h
+++ b/src/mathed/InsetMathHull.h
@@ -25,6 +25,7 @@ namespace lyx {
 class InsetLabel;
 class ParConstIterator;
 class RenderPreview;
+class MacroNameSet;
 
 
 /// This provides an interface between LyX insets and LyX math insets
@@ -152,6 +153,9 @@ public:
/// Recreates the preview if preview is enabled.
void reloadPreview(DocIterator const  pos) const;
///
+   void usedMacros(MathData const  md, DocIterator const  pos,
+   MacroNameSet  macros) const;
+   ///
void initUnicodeMath() const;
 
///

Re: test of math previews

[Guillaume M-M]

Le 06/06/2015 18:12, Enrico Forestieri a écrit :

On Sat, Jun 06, 2015 at 03:23:48PM +0200, Kornel Benko wrote:


Am Samstag, 6. Juni 2015 um 13:13:22, schrieb Enrico Forestieri for...@lyx.org

+void InsetMathHull::usedMacros(MathData const  md, MacroNameSet  macros) 
const
+{
+   for (size_t i = 0; i  md.size(); ++i) {
+   MathMacro * mi = const_castMathMacro 
*(md[i].nucleus()-asMacro());
+   if (mi) {
+   macros.insert(mi-name());
+   usedMacros(mi-definition(), macros);


I would change the order, to get the used macro first.


No, that is correct. The top level macro has to appear in the last position.

I failed to understand why that was not working but, most probably, it has
to do with the fact not all data is copied over to a cloned buffer.
However, I found a different way for collecting only the macros used
in a given math inset, so I did not investigate it further.

The attached patch for stable works for me and avoids the size explosion
when a very lot of macros are defined in a document. I count on Guillaume
to give it a thourough test. If he doesn't find glitches I will feel
confident about proposing to apply it for 2.1.4.



:)

lyx-preview-macros2-failure.lyx shows two cases where a macro is 
forgotten from the list. The third inset works and is meant as a 
control. All 3 work without the patch.


lyx-preview-macros2-lassert.lyx triggers an assertion when preview is 
activated. Does not assert without the patch.


Notice that my thourough test is basically loading my current paper 
with preview on.


lyx-preview-macros-2-failure.lyx
Description: application/lyx


lyx-preview-macros-2-lassert.lyx
Description: application/lyx

Re: test of math previews

[Guillaume M-M]

Le 06/06/2015 18:12, Enrico Forestieri a écrit :

On Sat, Jun 06, 2015 at 03:23:48PM +0200, Kornel Benko wrote:


Am Samstag, 6. Juni 2015 um 13:13:22, schrieb Enrico Forestieri for...@lyx.org

+void InsetMathHull::usedMacros(MathData const  md, MacroNameSet  macros) 
const
+{
+   for (size_t i = 0; i  md.size(); ++i) {
+   MathMacro * mi = const_castMathMacro 
*(md[i].nucleus()-asMacro());
+   if (mi) {
+   macros.insert(mi-name());
+   usedMacros(mi-definition(), macros);


I would change the order, to get the used macro first.


No, that is correct. The top level macro has to appear in the last position.

I failed to understand why that was not working but, most probably, it has
to do with the fact not all data is copied over to a cloned buffer.
However, I found a different way for collecting only the macros used
in a given math inset, so I did not investigate it further.

The attached patch for stable works for me and avoids the size explosion
when a very lot of macros are defined in a document. I count on Guillaume
to give it a thourough test. If he doesn't find glitches I will feel
confident about proposing to apply it for 2.1.4.



Obvious one to test for


lyx-preview-macros-2-sigsegv.lyx
Description: application/lyx

Re: test of math previews

[Kornel Benko]

Am Samstag, 6. Juni 2015 um 19:12:45, schrieb Enrico Forestieri for...@lyx.org
 On Sat, Jun 06, 2015 at 03:23:48PM +0200, Kornel Benko wrote:
 
  Am Samstag, 6. Juni 2015 um 13:13:22, schrieb Enrico Forestieri 
  for...@lyx.org
   +void InsetMathHull::usedMacros(MathData const  md, MacroNameSet  
   macros) const
   +{
   +   for (size_t i = 0; i  md.size(); ++i) {
   +   MathMacro * mi = const_castMathMacro 
   *(md[i].nucleus()-asMacro());
   +   if (mi) {
   +   macros.insert(mi-name());
   +   usedMacros(mi-definition(), macros);
  
  I would change the order, to get the used macro first.
 
 No, that is correct. The top level macro has to appear in the last position.

OK, I thought macros.insert() would insert at the end.

 I failed to understand why that was not working but, most probably, it has
 to do with the fact not all data is copied over to a cloned buffer.
 However, I found a different way for collecting only the macros used
 in a given math inset, so I did not investigate it further.
 
 The attached patch for stable works for me and avoids the size explosion
 when a very lot of macros are defined in a document. I count on Guillaume
 to give it a thourough test. If he doesn't find glitches I will feel
 confident about proposing to apply it for 2.1.4.
 

Works for me. That is, it does not crash with 
lyx-bug-lassert-preview-startup.lyx.

But I wonder why there are preview entries in the lyxpreview*.tex
...
\begin{document}
\begin{preview}
${\displaystyle {#1}}$
\end{preview}

\begin{preview}
$#1$
\end{preview}

\begin{preview}
${\scriptstyle {#1}}$
\end{preview}

\begin{preview}
${\scriptscriptstyle {#1}}$
\end{preview}


\end{document}
...

Kornel

signature.asc
Description: This is a digitally signed message part.

Re: include bibtexall in lib/scripts ?

[Jürgen Spitzmüller]

2015-06-06 4:07 GMT+02:00 Scott Kostyshak :

> Would it make sense to ship bibtexall with LyX? It is used to run
> bibtex on all .aux files. It is especially useful with chapterbib,
> which allows a separate bibliography per chapter.
>

Hm, I would prefer proper chapterbib support, actually.

At least, we could implement the script's functionality in LaTeX.cpp rather
than using the script itself.


>
> We mention it here:
> http://wiki.lyx.org/BibTeX/Tips
> but unfortunately the instructions do not work because we now use a
> combo box for the bibliography generation processor and I don't think
> one can enter a custom command.
>

You can, though only in prefs.


>
> One can change the bibtex command (as Jürgen says to do in his version
> of bibtexall), but then the .lyx file is even less portable.
>
> variants of bibtexall are actually shipped by (atleast) two LaTeX packages:
> /usr/local/texlive/2015/texmf-dist/doc/latex/bibunits/bibtexall
> /usr/local/texlive/2015/texmf-dist/doc/latex/multibib/bibtexall
>

Interesting. I did not notice this rather ad-hoc script of mine was picked
up by distributions.

Jürgen


>
> I am attaching the script. It was written Jürgen and tweaked by me.
>
> Thoughts?
>
> Scott
>

Re: test of math previews

[Enrico Forestieri]

On Fri, Jun 05, 2015 at 11:03:48PM +0100, Guillaume M-M wrote:

> Le 05/06/2015 08:14, Enrico Forestieri a écrit :
> >On Thu, Jun 04, 2015 at 07:32:03PM -0400, Richard Heck wrote:
> >>On 06/04/2015 07:13 PM, Guillaume M-M wrote:
> >>>
> >>>Bad news, it seems to forget that macros can use macros. With the attached
> >>>file the preview is as follows:
> >>>
> >>>\begin{preview}
> >>>\global\long\def\b{\a}
> >>>
> >>>$\b$
> >>>\end{preview}
> >>
> >>I suspect this is why all macros are included.
> >
> >Most probably. This leaves us pretty stuck with the problem caused by
> >having hundreds of macros. I will not commit any change to stable until
> >we find a good solution. After all, this is what already happens now
> >and it is not a regression. I think the complaint is due to the fact
> >that now also the previews get zoomed and this may cause relevant delays,
> 
> Enrico mentions a "complaint". There seems to be a misunderstanding. The
> freeze on zoom is a regression in stable, while the time explosion was
> already there. This is only a factual report of the issues trying to be the
> most helpful. The reports are the outcome of my tests of the preview
> mechanism following Enrico's request of having his patch at
>  tested.
> 
> In truth, I have no interest in full preview, only in the solution to #9354,
> and my tests concluded that there are no immediate issues with his patch,
> while I managed to find other issues---many thanks to Enrico and Jürgen for
> fixing them promptly. Fixing #9354 (preview inset does not work with math
> macros) in 2.1.4 would make the preview mechanism useful again in my
> context, avoiding the performance issues. (Especially because I cannot ask
> my collaborators to compile from git.) Apart from that, only this regression
> regarding the zoom/preview interaction remains regarding the stable branch
> (to summarise the thread, since you probably want to move forward with
> 2.1.4).
> 
> After this thread dies off, I may record the remaining important issues in
> the bug tracker, but again please do not regard such reports as complaints!

When you don't speak/write in your native language often you end up
conveying what you are able to and not what you actually mean.
And email doesn't help in this respect.

I didn't want to attach any negative meaning to the word "complaint" and,
morever, maybe I failed to actually properly acknowledge your help in this
matter. Indeed, you made a not so common testing work and helped discovering
issues that were escaping my (deemed) extensive testing.

That said, I was confident in applying all the changes involving only
touching the scripts because they solve some long standing issues without
the risk of introducing more severe issues. I think you tested the patch
for #9354 very well and I now feel confident that it is safe, but it is
the responsibility of the stable maintainer taking a decision about it.

Also take into account that, once the problem of the size explosion in the
presence of hundreds of macros is solved, it will have to be adapted, too.

-- 
Enrico

Re: test of math previews

[Enrico Forestieri]

On Fri, Jun 05, 2015 at 09:40:39AM +0200, Jean-Marc Lasgouttes wrote:

> Le 05/06/2015 09:14, Enrico Forestieri a écrit :
> >A possible way out would be outputting all the macros once at the
> >beginning of the file, such that they are seen by all snippets, and then
> >the specific one appearing in a math inset (as done in the patch I was
> >proposing). I'll have a look whether this can be done in a simple way.
> 
> Isn't it possible to output them as they are output by the normal latex
> export? Of course, this would mean that creating previews would be a very
> different process, more like a special version of our latex output code,
> instead of some registration done by the insets themselves.

I don't think that is possible. The point is that the previews should be
updated whenever they change. Now, they can change even if the math inset
was not modified, because one could have modified a macro. Thus, the
snippets have also to contain the macros they use.

I tried to solve this problem by including in a snippet only the macros that
are actually used. I almost succeeded with the attached patch for stable
(master would require more surgery). I say almost because it seems that
only the macros used by the current macro are spotted, but not the ones
that, in turn, are used by them. I mean, after

\newcommand{\first}{pluto}
\newcommand{\second}{goofy\,and\,\first}
\newcommand{\third}{mickey,\,\second}
$\third$

the macros \second and \third are spotted but not \first. See also the
attached examples. After activating instant previews, when loading
lyx-preview-macros-2.lyx you will correctly see

Found: first
Found: second

but loading lyx-preview-macros-3.lyx produces

Found: second
Found: third
Warning: Failed to produce 1 preview snippet(s)

Essentially, the patch goes recursively through the definition of the
macros found in an inset, but it seems that the definition of "second
level" macros are empty? I am not so familiar with the code dealing
with math macros, but I thought that their definition in terms of a
LyX inset should be held in MacroData::definition_. However, it does
not seem to (always) be so?

Any help or suggestion will be appreciated.

-- 
Enrico
diff --git a/src/mathed/InsetMathHull.cpp b/src/mathed/InsetMathHull.cpp
index d4bdf87..a7db85f 100644
--- a/src/mathed/InsetMathHull.cpp
+++ b/src/mathed/InsetMathHull.cpp
@@ -31,6 +31,7 @@
 #include "Language.h"
 #include "LaTeXFeatures.h"
 #include "LyXRC.h"
+#include "MathMacro.h"
 #include "MacroTable.h"
 #include "output_xhtml.h"
 #include "Paragraph.h"
@@ -622,6 +623,18 @@ void InsetMathHull::addPreview(DocIterator const & 
inset_pos,
 }
 
 
+void InsetMathHull::usedMacros(MathData const & md, MacroNameSet & macros) 
const
+{
+   for (size_t i = 0; i < md.size(); ++i) {
+   MathMacro * mi = const_cast(md[i].nucleus()->asMacro());
+   if (mi) {
+   macros.insert(mi->name());
+   usedMacros(mi->definition(), macros);
+   }
+   }
+}
+
+
 void InsetMathHull::preparePreview(DocIterator const & pos,
bool forexport) const
 {
@@ -632,13 +645,17 @@ void InsetMathHull::preparePreview(DocIterator const & 
pos,
 
Buffer const * buffer = pos.buffer();
 
-   // collect macros at this position
+   // collect macros used in this inset
MacroNameSet macros;
-   buffer->listMacroNames(macros);
+   for (row_type row = 0; row < nrows(); ++row)
+   for (col_type col = 0; col < ncols(); ++col)
+   usedMacros(cell(index(row, col)), macros);
+
MacroNameSet::iterator it = macros.begin();
MacroNameSet::iterator end = macros.end();
odocstringstream macro_preamble;
for (; it != end; ++it) {
+   lyxerr << "Found: " << *it << endl;
MacroData const * data = buffer->getMacro(*it, pos, true);
if (data) {
data->write(macro_preamble, true);
diff --git a/src/mathed/InsetMathHull.h b/src/mathed/InsetMathHull.h
index 0cd..65cb0ea 100644
--- a/src/mathed/InsetMathHull.h
+++ b/src/mathed/InsetMathHull.h
@@ -25,6 +25,7 @@ namespace lyx {
 class InsetLabel;
 class ParConstIterator;
 class RenderPreview;
+class MacroNameSet;
 
 
 /// This provides an interface between "LyX insets" and "LyX math insets"
@@ -190,6 +191,8 @@ private:
/// used by image export
void loadPreview(DocIterator const & pos) const;
///
+   void usedMacros(MathData const & md, MacroNameSet & macros) const;
+   ///
void setType(HullType type);
///
void validate1(LaTeXFeatures & features);
diff --git a/src/mathed/MathMacro.h b/src/mathed/MathMacro.h
index 239495e..18048d3 100644
--- a/src/mathed/MathMacro.h
+++ b/src/mathed/MathMacro.h
@@ -135,6 +135,7 @@ protected:
friend class MathData;
friend class ArgumentProxy;
friend class Cursor;
+   friend 

Re: test of math previews

[Kornel Benko]

Am Samstag, 6. Juni 2015 um 13:13:22, schrieb Enrico Forestieri 
> +void InsetMathHull::usedMacros(MathData const & md, MacroNameSet & macros) 
> const
> +{
> +   for (size_t i = 0; i < md.size(); ++i) {
> +   MathMacro * mi = const_cast *>(md[i].nucleus()->asMacro());
> +   if (mi) {
> +   macros.insert(mi->name());
> +   usedMacros(mi->definition(), macros);

I would change the order, to get the used macro first.

> +   }
> +   }
> +}
> +

This may not help with empty second level macros though.

Kornel




signature.asc
Description: This is a digitally signed message part.

Re: assertion on master when deleting an empty chunk from inside

[Kornel Benko]

Am Freitag, 5. Juni 2015 um 17:26:12, schrieb Scott Kostyshak 
> To reproduce, on master open mwe.lyx, place the cursor inside of the
> empty chunk, and press Backspace.
> 
> Can others reproduce?
> 
> Note also that when I export mwe.lyx to 2.1.x format, the separator is
> gone and there is an ERT with whitespace in it.

Yes to both.

> I don't know if this
> is incorrect and if so if it is related to the assertion.
> 
> Scott

Kornel

signature.asc
Description: This is a digitally signed message part.

Re: test of math previews

[Enrico Forestieri]

On Sat, Jun 06, 2015 at 03:23:48PM +0200, Kornel Benko wrote:

> Am Samstag, 6. Juni 2015 um 13:13:22, schrieb Enrico Forestieri 
> 
> > +void InsetMathHull::usedMacros(MathData const & md, MacroNameSet & macros) 
> > const
> > +{
> > +   for (size_t i = 0; i < md.size(); ++i) {
> > +   MathMacro * mi = const_cast > *>(md[i].nucleus()->asMacro());
> > +   if (mi) {
> > +   macros.insert(mi->name());
> > +   usedMacros(mi->definition(), macros);
> 
> I would change the order, to get the used macro first.

No, that is correct. The top level macro has to appear in the last position.

I failed to understand why that was not working but, most probably, it has
to do with the fact not all data is copied over to a cloned buffer.
However, I found a different way for collecting only the macros used
in a given math inset, so I did not investigate it further.

The attached patch for stable works for me and avoids the size explosion
when a very lot of macros are defined in a document. I count on Guillaume
to give it a thourough test. If he doesn't find glitches I will feel
confident about proposing to apply it for 2.1.4.

-- 
Enrico
diff --git a/src/mathed/InsetMathHull.cpp b/src/mathed/InsetMathHull.cpp
index d4bdf87..c23d2f4 100644
--- a/src/mathed/InsetMathHull.cpp
+++ b/src/mathed/InsetMathHull.cpp
@@ -31,6 +31,7 @@
 #include "Language.h"
 #include "LaTeXFeatures.h"
 #include "LyXRC.h"
+#include "MathMacro.h"
 #include "MacroTable.h"
 #include "output_xhtml.h"
 #include "Paragraph.h"
@@ -622,6 +623,28 @@ void InsetMathHull::addPreview(DocIterator const & 
inset_pos,
 }
 
 
+void InsetMathHull::usedMacros(MathData const & md, DocIterator const & pos,
+   MacroNameSet & macros) const
+{
+   for (size_t i = 0; i < md.size(); ++i) {
+   MathMacro const * mi = md[i].nucleus()->asMacro();
+   if (mi) {
+   MathData ar(pos.buffer());
+   MacroData const * data =
+   pos.buffer()->getMacro(mi->name(), pos, true);
+   if (data) {
+   odocstringstream macro_def;
+   data->write(macro_def, true);
+   macro_def << endl;
+   macros.insert(macro_def.str());
+   asArray(data->definition(), ar);
+   }
+   usedMacros(ar, pos, macros);
+   }
+   }
+}
+
+
 void InsetMathHull::preparePreview(DocIterator const & pos,
bool forexport) const
 {
@@ -632,19 +655,17 @@ void InsetMathHull::preparePreview(DocIterator const & 
pos,
 
Buffer const * buffer = pos.buffer();
 
-   // collect macros at this position
+   // collect macros used in this inset
MacroNameSet macros;
-   buffer->listMacroNames(macros);
+   for (row_type row = 0; row < nrows(); ++row)
+   for (col_type col = 0; col < ncols(); ++col)
+   usedMacros(cell(index(row, col)), pos, macros);
+
MacroNameSet::iterator it = macros.begin();
MacroNameSet::iterator end = macros.end();
-   odocstringstream macro_preamble;
-   for (; it != end; ++it) {
-   MacroData const * data = buffer->getMacro(*it, pos, true);
-   if (data) {
-   data->write(macro_preamble, true);
-   macro_preamble << endl;
-   }
-   }
+   docstring macro_preamble;
+   for (; it != end; ++it)
+   macro_preamble.append(*it);
 
docstring setcnt;
if (forexport && haveNumbers()) {
@@ -667,8 +688,7 @@ void InsetMathHull::preparePreview(DocIterator const & pos,
  '{' + convert(num) + '}';
}
}
-   docstring const snippet = macro_preamble.str() +
-   setcnt + latexString(*this);
+   docstring const snippet = macro_preamble + setcnt + latexString(*this);
LYXERR(Debug::MACROS, "Preview snippet: " << snippet);
preview_->addPreview(snippet, *buffer, forexport);
 }
diff --git a/src/mathed/InsetMathHull.h b/src/mathed/InsetMathHull.h
index 0cd..0b9e5fd 100644
--- a/src/mathed/InsetMathHull.h
+++ b/src/mathed/InsetMathHull.h
@@ -25,6 +25,7 @@ namespace lyx {
 class InsetLabel;
 class ParConstIterator;
 class RenderPreview;
+class MacroNameSet;
 
 
 /// This provides an interface between "LyX insets" and "LyX math insets"
@@ -152,6 +153,9 @@ public:
/// Recreates the preview if preview is enabled.
void reloadPreview(DocIterator const & pos) const;
///
+   void usedMacros(MathData const & md, DocIterator const & pos,
+   MacroNameSet & macros) const;
+   ///
void initUnicodeMath() const;
 
///

Re: test of math previews

[Kornel Benko]

Am Samstag, 6. Juni 2015 um 19:12:45, schrieb Enrico Forestieri 
> On Sat, Jun 06, 2015 at 03:23:48PM +0200, Kornel Benko wrote:
> 
> > Am Samstag, 6. Juni 2015 um 13:13:22, schrieb Enrico Forestieri 
> > 
> > > +void InsetMathHull::usedMacros(MathData const & md, MacroNameSet & 
> > > macros) const
> > > +{
> > > +   for (size_t i = 0; i < md.size(); ++i) {
> > > +   MathMacro * mi = const_cast > > *>(md[i].nucleus()->asMacro());
> > > +   if (mi) {
> > > +   macros.insert(mi->name());
> > > +   usedMacros(mi->definition(), macros);
> > 
> > I would change the order, to get the used macro first.
> 
> No, that is correct. The top level macro has to appear in the last position.

OK, I thought macros.insert() would insert at the end.

> I failed to understand why that was not working but, most probably, it has
> to do with the fact not all data is copied over to a cloned buffer.
> However, I found a different way for collecting only the macros used
> in a given math inset, so I did not investigate it further.
> 
> The attached patch for stable works for me and avoids the size explosion
> when a very lot of macros are defined in a document. I count on Guillaume
> to give it a thourough test. If he doesn't find glitches I will feel
> confident about proposing to apply it for 2.1.4.
> 

Works for me. That is, it does not crash with 
lyx-bug-lassert-preview-startup.lyx.

But I wonder why there are preview entries in the lyxpreview*.tex
...
\begin{document}
\begin{preview}
${\displaystyle {#1}}$
\end{preview}

\begin{preview}
$#1$
\end{preview}

\begin{preview}
${\scriptstyle {#1}}$
\end{preview}

\begin{preview}
${\scriptscriptstyle {#1}}$
\end{preview}


\end{document}
...

Kornel

signature.asc
Description: This is a digitally signed message part.

Re: test of math previews

[Guillaume M-M]

Le 06/06/2015 18:12, Enrico Forestieri a écrit :

On Sat, Jun 06, 2015 at 03:23:48PM +0200, Kornel Benko wrote:


Am Samstag, 6. Juni 2015 um 13:13:22, schrieb Enrico Forestieri 

+void InsetMathHull::usedMacros(MathData const & md, MacroNameSet & macros) 
const
+{
+   for (size_t i = 0; i < md.size(); ++i) {
+   MathMacro * mi = const_cast(md[i].nucleus()->asMacro());
+   if (mi) {
+   macros.insert(mi->name());
+   usedMacros(mi->definition(), macros);


I would change the order, to get the used macro first.


No, that is correct. The top level macro has to appear in the last position.

I failed to understand why that was not working but, most probably, it has
to do with the fact not all data is copied over to a cloned buffer.
However, I found a different way for collecting only the macros used
in a given math inset, so I did not investigate it further.

The attached patch for stable works for me and avoids the size explosion
when a very lot of macros are defined in a document. I count on Guillaume
to give it a thourough test. If he doesn't find glitches I will feel
confident about proposing to apply it for 2.1.4.



:)

lyx-preview-macros2-failure.lyx shows two cases where a macro is 
forgotten from the list. The third inset works and is meant as a 
control. All 3 work without the patch.


lyx-preview-macros2-lassert.lyx triggers an assertion when preview is 
activated. Does not assert without the patch.


Notice that my "thourough test" is basically loading my current paper 
with preview on.


lyx-preview-macros-2-failure.lyx
Description: application/lyx


lyx-preview-macros-2-lassert.lyx
Description: application/lyx

Re: test of math previews

[Guillaume M-M]

Le 06/06/2015 18:12, Enrico Forestieri a écrit :

On Sat, Jun 06, 2015 at 03:23:48PM +0200, Kornel Benko wrote:


Am Samstag, 6. Juni 2015 um 13:13:22, schrieb Enrico Forestieri 

+void InsetMathHull::usedMacros(MathData const & md, MacroNameSet & macros) 
const
+{
+   for (size_t i = 0; i < md.size(); ++i) {
+   MathMacro * mi = const_cast(md[i].nucleus()->asMacro());
+   if (mi) {
+   macros.insert(mi->name());
+   usedMacros(mi->definition(), macros);


I would change the order, to get the used macro first.


No, that is correct. The top level macro has to appear in the last position.

I failed to understand why that was not working but, most probably, it has
to do with the fact not all data is copied over to a cloned buffer.
However, I found a different way for collecting only the macros used
in a given math inset, so I did not investigate it further.

The attached patch for stable works for me and avoids the size explosion
when a very lot of macros are defined in a document. I count on Guillaume
to give it a thourough test. If he doesn't find glitches I will feel
confident about proposing to apply it for 2.1.4.



Obvious one to test for


lyx-preview-macros-2-sigsegv.lyx
Description: application/lyx