The branch, 2.0.x, has been updated.

- Log -----------------------------------------------------------------

commit 7b93cc5fc44caedb4e90aa12f849c3f4b6e08c93
Author: Jean-Marc Lasgouttes <lasgout...@lyx.org>
Date:   Tue Jul 17 23:59:04 2012 +0200

    Make sure that undo is recorded when magic tricks are played with 
InsetBibitem.
    
    Fixes bug #7111: Assertion with undo and InsetBibitem
    
      * add Paragraph::brokenBiblio(), with tells whether there is something to 
fix.
      * rename Paragraph::checkBiblio to fixBiblio; simplify it greatly by using
        InsetList methods
      * In TextMetrics::redoParagraph, call recordUndo before fixBiblio (only 
when
        there is something to fix obviously)
    
    Special care is taken to update cursor only when it makes sense. Note
    that there are cases where undo information is not recorded (current
    cursor not in the slice where action happens)

diff --git a/src/Paragraph.cpp b/src/Paragraph.cpp
index 29092b0..389e292 100644
--- a/src/Paragraph.cpp
+++ b/src/Paragraph.cpp
@@ -3167,70 +3167,61 @@ char_type Paragraph::transformChar(char_type c, 
pos_type pos) const
 }
 
 
-int Paragraph::checkBiblio(Buffer const & buffer)
+bool Paragraph::brokenBiblio() const
 {
-       // FIXME From JS:
-       // This is getting more and more a mess. ...We really should clean
-       // up this bibitem issue for 1.6.
+       // there is a problem if there is no bibitem at position 0 or
+       // if there is another bibitem in the paragraph.
+       return d->layout_->labeltype == LABEL_BIBLIO
+               && (d->insetlist_.find(BIBITEM_CODE) != 0
+                   || d->insetlist_.find(BIBITEM_CODE, 1) > 0);
+}
+
+
+int Paragraph::fixBiblio(Buffer const & buffer)
+{
+       // FIXME: What about the case where paragraph is not BIBLIO
+       // but there is an InsetBibitem?
+       // FIXME: when there was already an inset at 0, the return value is 1,
+       // which does not tell whether another inset has been remove; the
+       // cursor cannot be correctly updated.
 
-       // Add bibitem insets if necessary
        if (d->layout_->labeltype != LABEL_BIBLIO)
                return 0;
 
-       bool hasbibitem = !d->insetlist_.empty()
-               // Insist on it being in pos 0
-               && d->text_[0] == META_INSET
-               && d->insetlist_.begin()->inset->lyxCode() == BIBITEM_CODE;
-
-       bool track_changes = buffer.params().trackChanges;
-
-       docstring oldkey;
-       docstring oldlabel;
-
-       // remove a bibitem in pos != 0
-       // restore it later in pos 0 if necessary
-       // (e.g. if a user inserts contents _before_ the item)
-       // we're assuming there's only one of these, which there
-       // should be.
-       int erasedInsetPosition = -1;
-       InsetList::iterator it = d->insetlist_.begin();
-       InsetList::iterator end = d->insetlist_.end();
-       for (; it != end; ++it)
-               if (it->inset->lyxCode() == BIBITEM_CODE
-                     && it->pos > 0) {
-                       InsetCommand * olditem = it->inset->asInsetCommand();
-                       oldkey = olditem->getParam("key");
-                       oldlabel = olditem->getParam("label");
-                       erasedInsetPosition = it->pos;
-                       eraseChar(erasedInsetPosition, track_changes);
-                       break;
-       }
+       bool const track_changes = buffer.params().trackChanges;
+       int bibitem_pos = d->insetlist_.find(BIBITEM_CODE);
+       bool const hasbibitem0 = bibitem_pos == 0;
 
-       // There was an InsetBibitem at the beginning, and we didn't
-       // have to erase one.
-       if (hasbibitem && erasedInsetPosition < 0)
+       if (hasbibitem0) {
+               bibitem_pos = d->insetlist_.find(BIBITEM_CODE, 1);
+               // There was an InsetBibitem at pos 0, and no other one => OK
+               if (bibitem_pos == -1)
                        return 0;
+               // there is a bibitem at the 0 position, but since
+               // there is a second one, we copy the second on the
+               // first. We're assuming there are at most two of
+               // these, which there should be.
+               // FIXME: why does it make sense to do that rather
+               // than keep the first? (JMarc)
+               Inset * inset = d->insetlist_.release(bibitem_pos);
+               eraseChar(bibitem_pos, track_changes);
+               d->insetlist_.begin()->inset = inset;
+               return -bibitem_pos;
+       }
+
+       // We need to create an inset at the beginning
+       Inset * inset = 0;
+       if (bibitem_pos > 0) {
+               // there was one somewhere in the paragraph, let's move it
+               inset = d->insetlist_.release(bibitem_pos);
+               eraseChar(bibitem_pos, track_changes);
+       } else
+               // make a fresh one
+               inset = new InsetBibitem(const_cast<Buffer *>(&buffer),
+                                        InsetCommandParams(BIBITEM_CODE));
 
-       // There was an InsetBibitem at the beginning and we did have to
-       // erase one. So we give its properties to the beginning inset.
-       if (hasbibitem) {
-               InsetCommand * inset = 
d->insetlist_.begin()->inset->asInsetCommand();
-               if (!oldkey.empty())
-                       inset->setParam("key", oldkey);
-               inset->setParam("label", oldlabel);
-               return -erasedInsetPosition;
-       }
-
-       // There was no inset at the beginning, so we need to create one with
-       // the key and label of the one we erased.
-       InsetBibitem * inset =
-               new InsetBibitem(const_cast<Buffer *>(&buffer), 
InsetCommandParams(BIBITEM_CODE));
-       // restore values of previously deleted item in this par.
-       if (!oldkey.empty())
-               inset->setParam("key", oldkey);
-       inset->setParam("label", oldlabel);
-       insertInset(0, inset,
-                   Change(track_changes ? Change::INSERTED : 
Change::UNCHANGED));
+       insertInset(0, inset, Change(track_changes ? Change::INSERTED 
+                                                  : Change::UNCHANGED));
 
        return 1;
 }
diff --git a/src/Paragraph.h b/src/Paragraph.h
index f5d6a23..0ea959f 100644
--- a/src/Paragraph.h
+++ b/src/Paragraph.h
@@ -440,6 +440,8 @@ public:
        ///
        ParagraphParameters const & params() const;
 
+       /// Check whether a call to fixBiblio is needed.
+       bool brokenBiblio() const;
        /// Check if we are in a Biblio environment and insert or
        /// delete InsetBibitems as necessary.
        /// \retval int 1, if we had to add an inset, in which case
@@ -447,7 +449,7 @@ public:
        /// an inset, in which case pos is the position from which the inset
        /// was deleted, and the cursor will need to be moved back one if it
        /// was previously past that position. Return 0 otherwise.
-       int checkBiblio(Buffer const & buffer);
+       int fixBiblio(Buffer const & buffer);
 
        /// For each author, set 'used' to true if there is a change
        /// by this author in the paragraph.
diff --git a/src/TextMetrics.cpp b/src/TextMetrics.cpp
index 10e8904..8831ae9 100644
--- a/src/TextMetrics.cpp
+++ b/src/TextMetrics.cpp
@@ -375,19 +375,28 @@ bool TextMetrics::redoParagraph(pit_type const pit)
        main_text_ = (text_ == &buffer.text());
        bool changed = false;
 
+       // Check whether there are InsetBibItems that need fixing
        // FIXME: This check ought to be done somewhere else. It is the reason
-       // why text_ is not     const. But then, where else to do it?
+       // why text_ is not const. But then, where else to do it?
        // Well, how can you end up with either (a) a biblio environment that
        // has no InsetBibitem or (b) a biblio environment with more than one
        // InsetBibitem? I think the answer is: when paragraphs are merged;
        // when layout is set; when material is pasted.
-       int const moveCursor = par.checkBiblio(buffer);
-       if (moveCursor > 0)
-               const_cast<Cursor &>(bv_->cursor()).posForward();
-       else if (moveCursor < 0) {
-               Cursor & cursor = const_cast<Cursor &>(bv_->cursor());
-               if (cursor.pos() >= -moveCursor)
-                       cursor.posBackward();
+       if (par.brokenBiblio()) {
+               Cursor & cur = const_cast<Cursor &>(bv_->cursor());
+               // In some cases, we do not know how to record undo
+               if (&cur.inset() == &text_->inset())
+                       cur.recordUndo(ATOMIC_UNDO, pit, pit);
+
+               int const moveCursor = par.fixBiblio(buffer);
+
+               // Is it necessary to update the cursor?
+               if (&cur.inset() == &text_->inset() && cur.pit() == pit) {
+                       if (moveCursor > 0)
+                               cur.posForward();
+                       else if (moveCursor < 0 && cur.pos() >= -moveCursor)
+                               cur.posBackward();
+               }
        }
 
        // Optimisation: this is used in the next two loops
diff --git a/status.20x b/status.20x
index 1611822..e994325 100644
--- a/status.20x
+++ b/status.20x
@@ -95,6 +95,8 @@ What's new
 - Fix various crashes when single document is edited in more windows
   (bug 8203).
 
+- Fix crash when using undo in a paragraph with layout Bibliography (bug 7111).
+
 - Replace current selection when pasting (bug 8027).
 
 - Make sure that undo restores environment depth correctly (bug 8159).

-----------------------------------------------------------------------

Summary of changes:
 src/Paragraph.cpp   |  103 +++++++++++++++++++++++---------------------------
 src/Paragraph.h     |    4 +-
 src/TextMetrics.cpp |   25 ++++++++----
 status.20x          |    2 +
 4 files changed, 69 insertions(+), 65 deletions(-)


hooks/post-receive
-- 
The LyX Source Repository

Reply via email to