Fix #9201 (advanced find matches note in front of search term, dataloss)

[Alfredo Braunstein]

The problem is that InsetNote do not produce any text nor latex
output, so it can be matched freely by the regexp. The patch
adds an output_notes member of OutputParam, default to false, and
setting to true on advanced s&r. I set it to true also for the
stringification of the search buffer, so now one can also *search* for
notes.

Comments?

A/

[Alfredo Braunstein]

Post by Alfredo Braunstein
The problem is that InsetNote do not produce any text nor latex
output, so it can be matched freely by the regexp. The patch
adds an output_notes member of OutputParam, default to false, and
setting to true on advanced s&r. I set it to true also for the
stringification of the search buffer, so now one can also *search* for
notes.

Btw, are there other insets besides InsetNote that could produce no
output? If the answer if yes, we could call the flag force_output or
something and use it where needed.

A/

[Jürgen Spitzmüller]

Post by Alfredo Braunstein
Btw, are there other insets besides InsetNote that could produce no
output? If the answer if yes, we could call the flag force_output or
something and use it where needed.

Inactive branches, and maybe some kinds of FlexInsets.

JÃŒrgen

Post by Alfredo Braunstein
A/

[Alfredo Braunstein]

Thanks a lot Jürgen. The following insets do not have a ::latex
implementation and thus are at a risk of dataloss in advanced s&r
(although some of these maybe inherit it from some other one). Maybe
the safest thing would be to implement Inset::latex that outputs some
unmatchable string when the runparams.force_output flag is set. Would
this be a good idea? How can I build an unmatchable string?

A/

~/src/lyx/src/insets$ grep -L ::latex Inset*.cpp
InsetBibitem.cpp
InsetCollapsable.cpp
InsetCommandParams.cpp
Inset.cpp
InsetERT.cpp
InsetFlex.cpp
InsetFootlike.cpp
InsetGraphicsParams.cpp
InsetLabel.cpp
InsetListingsParams.cpp
InsetMarginal.cpp
InsetPreview.cpp
InsetScript.cpp
InsetTOC.cpp

Post by Jürgen Spitzmüller

Post by Alfredo Braunstein
Btw, are there other insets besides InsetNote that could produce no
output? If the answer if yes, we could call the flag force_output or
something and use it where needed.

Inactive branches, and maybe some kinds of FlexInsets.
Jürgen

Post by Alfredo Braunstein
A/

[Jean-Marc Lasgouttes]

Post by Alfredo Braunstein
Thanks a lot Jürgen. The following insets do not have a ::latex
implementation and thus are at a risk of dataloss in advanced s&r
(although some of these maybe inherit it from some other one). Maybe
the safest thing would be to implement Inset::latex that outputs some
unmatchable string when the runparams.force_output flag is set. Would
this be a good idea? How can I build an unmatchable string?

I don't understand: I guess many inset just inherit their ::latex
method. What is the risk exactly.

Also, you may want to take in account the following method:

/// Is the content of this inset part of the output document?
virtual bool producesOutput() const { return true; }

You could also check whether there have been some output after calling
::latex and add a special marker otherwise.

JMarc

[Alfredo Braunstein]

On Wed, Oct 15, 2014 at 10:31 AM, Jean-Marc Lasgouttes

Post by Alfredo Braunstein
Thanks a lot Jürgen. The following insets do not have a ::latex
implementation and thus are at a risk of dataloss in advanced s&r
(although some of these maybe inherit it from some other one). Maybe
the safest thing would be to implement Inset::latex that outputs some
unmatchable string when the runparams.force_output flag is set. Would
this be a good idea? How can I build an unmatchable string?

I don't understand: I guess many inset just inherit their ::latex method.
What is the risk exactly.

Advanced Search & replace all [math:x]->[math:y] in a document containing

[disabled branch: blah blah][math:x]

the result is

[math:y]

i.e. the branch inset is gone. (same with a LyX Note, etc)

/// Is the content of this inset part of the output document?
virtual bool producesOutput() const { return true; }
You could also check whether there have been some output after calling
::latex and add a special marker otherwise.

Where? *::latex gets called recursively... The advanced s&r stuff
seems pretty complicated to me (and, from a quick look, a bit of a
hack if you ask me. No offense to the authors I hope, it is actually
an amazing feature). Adding a force_output flag could be an easy fix
to the dataloss with no negative consequences that I can think of.
What is your concern?

A/

[Jean-Marc Lasgouttes]

Post by Alfredo Braunstein

I don't understand: I guess many inset just inherit their ::latex method.
What is the risk exactly.

Advanced Search & replace all [math:x]->[math:y] in a document containing
[disabled branch: blah blah][math:x]
the result is
[math:y]

OK, but what is the link between InsetERT not having a ::latex method
ans not outputting anything? The InsetText::latex is sufficiently
flexible to handle it. Likewise for many insets derived from
InsetCommand (label, bibitem...).

Post by Alfredo Braunstein

/// Is the content of this inset part of the output document?
virtual bool producesOutput() const { return true; }
You could also check whether there have been some output after calling
::latex and add a special marker otherwise.

Where? *::latex gets called recursively... The advanced s&r stuff
seems pretty complicated to me (and, from a quick look, a bit of a
hack if you ask me. No offense to the authors I hope, it is actually
an amazing feature).

It is a complet ehack. But this hack was easier to write than the proper
version (especially when one wants to do regex).

Post by Alfredo Braunstein
Adding a force_output flag could be an easy fix
to the dataloss with no negative consequences that I can think of.
What is your concern?

My concern is that we already have two pices of code where we override
the "no output property":

* in Inset::addToToc, because we sometimes want the headings inside
notes to be available inthe outline sidebar

* in Buffer::updateStatistics, because some people want to have the
words in their notes or inactive branches counted

Before we had yet another thing for yet another use, I was wondering
whether we could have a unique mechanism with correct semantics to do that.

One poiibility would be to be able to control what Inset::producesOutput
returns (maybe via a Buffer or BufferParams parameter, I don't know).
The explicit code in InsetNote should actually rely on producesOutput
instead of looking at type of note.

JMarc

[Alfredo Braunstein]

On Wed, Oct 15, 2014 at 11:37 AM, Jean-Marc Lasgouttes

Post by Alfredo Braunstein

I don't understand: I guess many inset just inherit their ::latex method.
What is the risk exactly.

Advanced Search & replace all [math:x]->[math:y] in a document containing
[disabled branch: blah blah][math:x]
the result is
[math:y]

OK, but what is the link between InsetERT not having a ::latex method ans
not outputting anything? The InsetText::latex is sufficiently flexible to
handle it. Likewise for many insets derived from InsetCommand (label,
bibitem...).

Post by Alfredo Braunstein

/// Is the content of this inset part of the output document?
virtual bool producesOutput() const { return true; }
You could also check whether there have been some output after calling
::latex and add a special marker otherwise.

Where? *::latex gets called recursively... The advanced s&r stuff
seems pretty complicated to me (and, from a quick look, a bit of a
hack if you ask me. No offense to the authors I hope, it is actually
an amazing feature).

It is a complet ehack. But this hack was easier to write than the proper
version (especially when one wants to do regex).

Post by Alfredo Braunstein
Adding a force_output flag could be an easy fix
to the dataloss with no negative consequences that I can think of.
What is your concern?

My concern is that we already have two pices of code where we override the
* in Inset::addToToc, because we sometimes want the headings inside notes to
be available inthe outline sidebar
* in Buffer::updateStatistics, because some people want to have the words in
their notes or inactive branches counted
Before we had yet another thing for yet another use, I was wondering whether
we could have a unique mechanism with correct semantics to do that.
One poiibility would be to be able to control what Inset::producesOutput
returns (maybe via a Buffer or BufferParams parameter, I don't know). The
explicit code in InsetNote should actually rely on producesOutput instead of
looking at type of note.

I see your point, but I'm not convinced. In your solution we will need
to change the Buffer / BufferParams property temporarily before
calling latex and restore it afterwards. It seems a bit hackish...
Because ultimately, it's not a buffer property what we need, it is
really a parameter for the latex export. It's the same for the two
examples you cite IIUC, a temporary change only related to that
specific operation.

A/

[Alfredo Braunstein]

OK, but what is the link between InsetERT not having a ::latex method ans
not outputting anything? The InsetText::latex is sufficiently flexible to
handle it. Likewise for many insets derived from InsetCommand (label,
bibitem...).

I forgot to answer this. Putting this default latex method in Inset is
just a quick and easy safeguard; so insets (current and future ones)
that don't produce any output because their latex method is not
defined will not be matched freely and thus won't be at risk of
dataloss. Those in which their latex method is not called AND they
don't produce output will not be saved automatically unfortunately.
[But if they don't handle their own latex output, probably they don't
store that much information anyway and the loss will not be so
serious].

If it bothers you, I can leave out the Inset::latex part and only fix
InsetNote and InsetBranch and see if something else comes out.

A/

[Richard Heck]

The advanced s&r stuff seems pretty complicated to me (and, from a
quick look, a bit of a hack if you ask me. No offense to the authors I
hope, it is actually an amazing feature).

The author would agree with you, I think. There's been talk for some
time of re-implementing so as to walk the actual document tree (or a
copy of it) instead of serializing, etc.

Richard