In the existing codebase, there is a problem about how Insets get their
Buffers set after undo/redo. What we do at the moment is go through the
*entire Buffer* and set the Buffer for every single inset. This seems
like overkill. The attached patch fixes this by just setting the Buffers
for the newly created insets, i.e., those in the paragraphs we just pasted.
I'm proposing this at the moment for master only, but would appreciate
comments first.
The second patch is just a renaming, from Paragraph::setBuffer to
Paragraph::setInsetBuffers. The former is misleading as to what the
method does. I'd like to commit this to master and 2.3.x, the latter
simply to avoid conflicts when cherry-picking.
Richard
>From 2d77bd1c027b26efcb31691bb640d124e65a63c8 Mon Sep 17 00:00:00 2001
From: Richard Heck <rgh...@lyx.org>
Date: Sun, 15 Oct 2017 22:22:22 -0400
Subject: [PATCH 1/2] Fix the way that the Buffer gets set on undo.
Previously, we went through the entire Buffer and set it for every
single inset. Now we just do it for the insets we pasted.
---
src/Buffer.cpp | 6 ------
src/Buffer.h | 4 ----
src/Undo.cpp | 15 ++++++++++++---
3 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/src/Buffer.cpp b/src/Buffer.cpp
index c4ce8ea..89a76b5 100644
--- a/src/Buffer.cpp
+++ b/src/Buffer.cpp
@@ -4699,12 +4699,6 @@ void Buffer::bufferErrors(TeXErrors const & terr, ErrorList & errorList) const
}
-void Buffer::setBuffersForInsets() const
-{
- inset().setBuffer(const_cast<Buffer &>(*this));
-}
-
-
void Buffer::updateBuffer(UpdateScope scope, UpdateType utype) const
{
LBUFERR(!text().paragraphs().empty());
diff --git a/src/Buffer.h b/src/Buffer.h
index 165f5d8..6d50e70 100644
--- a/src/Buffer.h
+++ b/src/Buffer.h
@@ -719,10 +719,6 @@ public:
/// return a list of all used branches (also in children)
void getUsedBranches(std::list<docstring> &, bool const from_master = false) const;
- /// sets the buffer_ member for every inset in this buffer.
- // FIXME This really shouldn't be needed, but at the moment it's not
- // clear how to do it just for the individual pieces we need.
- void setBuffersForInsets() const;
/// Updates screen labels and some other information associated with
/// insets and paragraphs. Actually, it's more like a general "recurse
/// through the Buffer" routine, that visits all the insets and paragraphs.
diff --git a/src/Undo.cpp b/src/Undo.cpp
index a479728..4798737 100644
--- a/src/Undo.cpp
+++ b/src/Undo.cpp
@@ -497,6 +497,18 @@ void Undo::Private::doTextUndoOrRedo(CursorData & cur, UndoElementStack & stack,
for (; pit != end; ++pit)
pit->setInsetOwner(dit.realInset());
plist.insert(first, undo.pars->begin(), undo.pars->end());
+
+ // set the buffers for insets we created
+ // pargraphs were inserted before first, so first paragraph
+ // inserted is first - 1
+ ParagraphList::iterator fpit = first;
+ advance(fpit, -1);
+ // one more than last paragraph we inserted
+ ParagraphList::iterator fend = fpit;
+ advance(fend, undo.pars->size());
+ for (; fpit != fend; ++fpit)
+ fpit->setBuffer(buffer_);
+
delete undo.pars;
undo.pars = 0;
}
@@ -531,9 +543,6 @@ bool Undo::Private::textUndoOrRedo(CursorData & cur, bool isUndoOperation)
const size_t gid = stack.top().group_id;
while (!stack.empty() && stack.top().group_id == gid)
doTextUndoOrRedo(cur, stack, otherstack);
-
- // Adapt the new material to current buffer.
- buffer_.setBuffersForInsets(); // FIXME This shouldn't be here.
return true;
}
--
2.9.5
>From ff21a3cc972ce8b874f8023e1afac9544a7611c3 Mon Sep 17 00:00:00 2001
From: Richard Heck <rgh...@lyx.org>
Date: Sun, 15 Oct 2017 22:14:08 -0400
Subject: [PATCH 2/2] Rename Paragraph::setBuffer to
Paragraph::setInsetBuffers, to avoid confusion about what this routine does.
---
src/Compare.cpp | 2 +-
src/CutAndPaste.cpp | 6 +++---
src/Paragraph.cpp | 2 +-
src/Paragraph.h | 2 +-
src/Undo.cpp | 2 +-
src/insets/InsetText.cpp | 2 +-
6 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/Compare.cpp b/src/Compare.cpp
index 796eb19..214f25c 100644
--- a/src/Compare.cpp
+++ b/src/Compare.cpp
@@ -736,7 +736,7 @@ bool Compare::Impl::diff(Buffer const * new_buf, Buffer const * old_buf,
diff_i(rp_new);
for (pit_type p = 0; p < (pit_type)dest_pars_->size(); ++p) {
- (*dest_pars_)[p].setBuffer(const_cast<Buffer &>(*dest_buf));
+ (*dest_pars_)[p].setInsetBuffers(const_cast<Buffer &>(*dest_buf));
(*dest_pars_)[p].setInsetOwner(&dest_buf_->inset());
}
diff --git a/src/CutAndPaste.cpp b/src/CutAndPaste.cpp
index 05f9b90..b649d8f 100644
--- a/src/CutAndPaste.cpp
+++ b/src/CutAndPaste.cpp
@@ -422,7 +422,7 @@ pasteSelectionHelper(DocIterator const & cur, ParagraphList const & parlist,
// Set paragraph buffers. It's important to do this right away
// before something calls Inset::buffer() and causes a crash.
for (pit_type p = startpit; p <= pit; ++p)
- pars[p].setBuffer(const_cast<Buffer &>(buffer));
+ pars[p].setInsetBuffers(const_cast<Buffer &>(buffer));
// Join (conditionally) last pasted paragraph with next one, i.e.,
// the tail of the spliced document paragraph
@@ -631,7 +631,7 @@ void copySelectionHelper(Buffer const & buf, Text const & text,
// do not have a proper buffer reference. It makes
// sense to add them temporarily, because the
// operations below depend on that (acceptChanges included).
- it->setBuffer(const_cast<Buffer &>(buf));
+ it->setInsetBuffers(const_cast<Buffer &>(buf));
// PassThru paragraphs have the Language
// latex_language. This is invalid for others, so we
// need to change it to the buffer language.
@@ -827,7 +827,7 @@ vector<docstring> availableSelections(Buffer const * buf)
for (; pit != pend; ++pit) {
Paragraph par(*pit, 0, 46);
// adapt paragraph to current buffer.
- par.setBuffer(const_cast<Buffer &>(*buf));
+ par.setInsetBuffers(const_cast<Buffer &>(*buf));
textSel += par.asString(AS_STR_INSETS);
if (textSel.size() > 45) {
support::truncateWithEllipsis(textSel,45);
diff --git a/src/Paragraph.cpp b/src/Paragraph.cpp
index c9a234d..7620251 100644
--- a/src/Paragraph.cpp
+++ b/src/Paragraph.cpp
@@ -3673,7 +3673,7 @@ InsetList const & Paragraph::insetList() const
}
-void Paragraph::setBuffer(Buffer & b)
+void Paragraph::setInsetBuffers(Buffer & b)
{
d->insetlist_.setBuffer(b);
}
diff --git a/src/Paragraph.h b/src/Paragraph.h
index d462f4c..790c3f2 100644
--- a/src/Paragraph.h
+++ b/src/Paragraph.h
@@ -397,7 +397,7 @@ public:
///
InsetList const & insetList() const;
///
- void setBuffer(Buffer &);
+ void setInsetBuffers(Buffer &);
///
void resetBuffer();
diff --git a/src/Undo.cpp b/src/Undo.cpp
index 4798737..2ecda7f 100644
--- a/src/Undo.cpp
+++ b/src/Undo.cpp
@@ -507,7 +507,7 @@ void Undo::Private::doTextUndoOrRedo(CursorData & cur, UndoElementStack & stack,
ParagraphList::iterator fend = fpit;
advance(fend, undo.pars->size());
for (; fpit != fend; ++fpit)
- fpit->setBuffer(buffer_);
+ fpit->setInsetBuffers(buffer_);
delete undo.pars;
undo.pars = 0;
diff --git a/src/insets/InsetText.cpp b/src/insets/InsetText.cpp
index 8228fd2..cf41883 100644
--- a/src/insets/InsetText.cpp
+++ b/src/insets/InsetText.cpp
@@ -97,7 +97,7 @@ void InsetText::setBuffer(Buffer & buf)
{
ParagraphList::iterator end = paragraphs().end();
for (ParagraphList::iterator it = paragraphs().begin(); it != end; ++it)
- it->setBuffer(buf);
+ it->setInsetBuffers(buf);
Inset::setBuffer(buf);
}
--
2.9.5
