On Fri, Mar 10, 2006 at 04:47:29PM +0200, Martin Vermeer wrote: > On Fri, 2006-03-10 at 15:03 +0100, Jean-Marc Lasgouttes wrote: > > >>>>> "Martin" == Martin Vermeer <[EMAIL PROTECTED]> writes:
...
> > bool Paragraph::Pimpl::erase(pos_type pos)
> > {
> > - BOOST_ASSERT(pos < size());
> > + BOOST_ASSERT(pos <= size());
> >
> > Maybe something like
> > + BOOST_ASSERT(pos < size() || pos == size() && tracking());
Actually no. Due to the changes in the rest of the patch, erase is
called with pos == size(), but with tracking off it doesn't do anything.
...
> > In general, I'd prefer to see smaller patches, especially if they are
> > going to land in 1.4.
It is smaller now with 2212 split out. Much of the bigness is an optical
illusion though, see what happened around backspacePos0.
> > Also, are you confident that you understand why eraseSelectionHelper
> > is much smaller now?
>
> Partly. Actually I see now that the hasSameLayout() test was dropped in
> the patch. That is one thing I sort-of understand but don't see how to
> carry it over to the patch code.
Now I am personally confident, having added and verified the hasSameLayout
mod. Third party testing is never a luxury though.
- Martin
Index: paragraph.h
===================================================================
--- paragraph.h (revision 13312)
+++ paragraph.h (working copy)
@@ -224,6 +224,9 @@
/// set change at pos
void setChange(lyx::pos_type pos, Change::Type type);
+
+ /// set full change at pos
+ void setChangeFull(lyx::pos_type pos, Change change);
/// accept change
void acceptChange(lyx::pos_type start, lyx::pos_type end);
Index: paragraph_pimpl.C
===================================================================
--- paragraph_pimpl.C (revision 13312)
+++ paragraph_pimpl.C (working copy)
@@ -99,7 +99,7 @@
lyxerr[Debug::CHANGES] << "track changes for par "
<< id_ << " type " << type << endl;
changes_.reset(new Changes(type));
- changes_->set(type, 0, size());
+ changes_->set(type, 0, size() + 1);
}
@@ -116,7 +116,7 @@
return;
changes_.reset(new Changes(Change::INSERTED));
- changes_->set(Change::INSERTED, 0, size());
+ changes_->set(Change::INSERTED, 0, size() + 1);
}
@@ -147,6 +147,14 @@
}
+void Paragraph::Pimpl::setChangeFull(pos_type pos, Change change)
+{
+ if (!tracking())
+ return;
+
+ changes_->set(change, pos);
+}
+
Change::Type Paragraph::Pimpl::lookupChange(pos_type pos) const
{
if (!tracking())
@@ -204,10 +212,14 @@
break;
case Change::DELETED:
- eraseIntern(i);
- changes_->erase(i);
- --end;
- --i;
+ // Suppress access to nonexistent
+ // "end-of-paragraph char":
+ if (i < size()) {
+ eraseIntern(i);
+ changes_->erase(i);
+ --end;
+ --i;
+ }
break;
}
}
@@ -235,15 +247,18 @@
break;
case Change::INSERTED:
- eraseIntern(i);
- changes_->erase(i);
- --end;
- --i;
+ if (i < size()) {
+ eraseIntern(i);
+ changes_->erase(i);
+ --end;
+ --i;
+ }
break;
case Change::DELETED:
changes_->set(Change::UNCHANGED, i);
- if (owner_->isInset(i))
+ // No real char at position size():
+ if (i < size() && owner_->isInset(i))
owner_->getInset(i)->markErased(false);
break;
}
@@ -351,7 +366,7 @@
bool Paragraph::Pimpl::erase(pos_type pos)
{
- BOOST_ASSERT(pos < size());
+ BOOST_ASSERT(pos <= size());
if (tracking()) {
Change::Type changetype(changes_->lookup(pos));
@@ -359,14 +374,19 @@
// only allow the actual removal if it was /new/ text
if (changetype != Change::INSERTED) {
- if (owner_->isInset(pos))
+ if (pos < size() && owner_->isInset(pos))
owner_->getInset(pos)->markErased(true);
return false;
}
}
- eraseIntern(pos);
- return true;
+ // Don't physically access nonexistent end-of-paragraph char
+ if (pos < size()) {
+ eraseIntern(pos);
+ return true;
+ }
+
+ return false;
}
Index: paragraph_pimpl.h
===================================================================
--- paragraph_pimpl.h (revision 13312)
+++ paragraph_pimpl.h (working copy)
@@ -54,6 +54,8 @@
bool isChangeEdited(lyx::pos_type start, lyx::pos_type end) const;
/// set change at pos
void setChange(lyx::pos_type pos, Change::Type type);
+ /// set full change at pos
+ void setChangeFull(lyx::pos_type pos, Change change);
/// mark as erased
void markErased(bool);
/// accept change
Index: CutAndPaste.C
===================================================================
--- CutAndPaste.C (revision 13312)
+++ CutAndPaste.C (working copy)
@@ -271,64 +271,35 @@
return PitPosPair(endpit, endpos);
}
- bool all_erased = true;
-
- // Clear fragments of the first par in selection
- pars[startpit].erase(startpos, pars[startpit].size());
- if (pars[startpit].size() != startpos)
- all_erased = false;
-
- // Clear fragments of the last par in selection
- endpos -= pars[endpit].erase(0, endpos);
- if (endpos != 0)
- all_erased = false;
-
- // Erase all the "middle" paragraphs.
- if (params.tracking_changes) {
- // Look through the deleted pars if any, erasing as needed
- for (pit_type pit = startpit + 1; pit != endpit;) {
- // "erase" the contents of the par
- pars[pit].erase(0, pars[pit].size());
- if (pars[pit].empty()) {
- // remove the par if it's now empty
- pars.erase(pars.begin() + pit);
- --endpit;
- } else {
- ++pit;
- all_erased = false;
- }
- }
- } else {
- pars.erase(pars.begin() + startpit + 1, pars.begin() + endpit);
- endpit = startpit + 1;
+ // A paragraph break has to be physically removed by merging, but
+ // only if either (1) change tracking is off, or (2) the para break
+ // is "blue"
+ for (pit_type pit = startpit; pit != endpit + 1;) {
+ bool const merge = !params.tracking_changes ||
+ pars[pit].lookupChange(pars[pit].size()) ==
+ Change::INSERTED;
+ pos_type const left = ( pit == startpit ? startpos : 0 );
+ pos_type const right = ( pit == endpit ? endpos :
+ pars[pit].size() + 1 );
+ // Logical erase only:
+ pars[pit].erase(left, right);
+ // Separate handling of para break:
+ if (merge && pit != endpit &&
+ pars[pit].hasSameLayout(pars[pit + 1])) {
+ pos_type const thissize = pars[pit].size();
+ if (doclear)
+ pars[pit + 1].stripLeadingSpaces();
+ mergeParagraph(params, pars, pit);
+ --endpit;
+ if (pit == endpit)
+ endpos += thissize;
+ } else
+ ++pit;
}
-
-#if 0 // FIXME: why for cut but not copy ?
- // the cut selection should begin with standard layout
- if (realcut) {
- buf->params().clear();
- buf->bibkey = 0;
-
buf->layout(textclasslist[buffer->params.textclass].defaultLayoutName());
- }
-#endif
- if (startpit + 1 == pit_type(pars.size()))
- return PitPosPair(endpit, endpos);
-
- if (doclear) {
- pars[startpit + 1].stripLeadingSpaces();
- }
-
- // Merge first and last paragraph, if possible
- if (all_erased &&
- (pars[startpit].hasSameLayout(pars[startpit + 1]) ||
- pars[startpit + 1].empty())) {
- mergeParagraph(params, pars, startpit);
- // This because endpar gets deleted here!
- endpit = startpit;
- endpos = startpos;
- }
-
+ // Ensure legal cursor pos:
+ endpit = startpit;
+ endpos = startpos;
return PitPosPair(endpit, endpos);
}
Index: lyxtext.h
===================================================================
--- lyxtext.h (revision 13312)
+++ lyxtext.h (working copy)
@@ -223,6 +223,8 @@
///
bool Delete(LCursor & cur);
///
+ bool backspacePos0(LCursor & cur);
+ ///
bool backspace(LCursor & cur);
///
bool selectWordWhenUnderCursor(LCursor & cur, lyx::word_location);
Index: paragraph_funcs.C
===================================================================
--- paragraph_funcs.C (revision 13312)
+++ paragraph_funcs.C (working copy)
@@ -213,6 +213,9 @@
if (moveItem(par, tmp, bparams, i, j - pos, change))
++j;
}
+ // Move over end-of-par change attr
+ tmp.setChange(tmp.size(), par.lookupChange(par.size()));
+
// If tracking changes, set all the text that is to be
// erased to Type::INSERTED.
for (pos_type k = pos_end; k >= pos; --k) {
@@ -233,12 +236,15 @@
pos_type pos_end = next.size() - 1;
pos_type pos_insert = par.size();
+ Change::Type cr = next.lookupChange(next.size());
// ok, now copy the paragraph
for (pos_type i = 0, j = 0; i <= pos_end; ++i) {
Change::Type change = next.lookupChange(i);
if (moveItem(next, par, bparams, i, pos_insert + j, change))
++j;
}
+ // Move the change status of "carriage return" over
+ par.setChange(par.size(), cr);
pars.erase(pars.begin() + par_offset + 1);
}
Index: lyxfind.C
===================================================================
--- lyxfind.C (revision 13312)
+++ lyxfind.C (working copy)
@@ -127,9 +127,8 @@
bool findChange(DocIterator & cur)
{
- for (; cur; cur.forwardChar())
- if (cur.inTexted() && cur.pos() != cur.paragraph().size() &&
- cur.paragraph().lookupChange(cur.pos())
+ for (; cur; cur.forwardPos())
+ if (cur.inTexted() && cur.paragraph().lookupChange(cur.pos())
!= Change::UNCHANGED)
return true;
return false;
@@ -344,25 +343,21 @@
if (!findChange(cur))
return false;
- Paragraph const & par = cur.paragraph();
- const pos_type pos = cur.pos();
+ bv->cursor().setCursor(cur);
+ bv->cursor().resetAnchor();
+
+ Change orig_change = cur.paragraph().lookupChangeFull(cur.pos());
- Change orig_change = par.lookupChangeFull(pos);
- const pos_type parsize = par.size();
- pos_type end = pos;
-
- for (; end != parsize; ++end) {
- Change change = par.lookupChangeFull(end);
+ DocIterator et = doc_iterator_end(cur.inset());
+ for (; cur != et; cur.forwardPosNoDescend()) {
+ Change change = cur.paragraph().lookupChangeFull(cur.pos());
if (change != orig_change) {
- // slight UI optimisation: for replacements, we get
- // text like : _old_new. Consider that as one change.
- if (!(orig_change.type == Change::DELETED &&
- change.type == Change::INSERTED))
- break;
+ break;
}
}
- pos_type length = end - pos;
- bv->putSelectionAt(cur, length, false);
+ // Now put cursor to end of selection:
+ bv->cursor().setCursor(cur);
+ bv->cursor().setSelection();
// if we used a lfun like in find/replace, dispatch would do
// that for us
bv->update();
Index: paragraph.C
===================================================================
--- paragraph.C (revision 13312)
+++ paragraph.C (working copy)
@@ -159,12 +159,15 @@
lyx::time_type const curtime(lyx::current_time());
int column = 0;
- for (pos_type i = 0; i < size(); ++i) {
+ for (pos_type i = 0; i <= size(); ++i) {
Change change = pimpl_->lookupChangeFull(i);
Changes::lyxMarkChange(os, column, curtime, running_change,
change);
running_change = change;
+ if (i == size())
+ break;
+
// Write font changes
LyXFont font2 = getFontSettings(bparams, i);
if (font2 != font1) {
@@ -223,15 +226,6 @@
}
}
- // to make reading work properly
- if (!size()) {
- running_change = pimpl_->lookupChange(0);
- Changes::lyxMarkChange(os, column, curtime,
- Change(Change::UNCHANGED), running_change);
- }
- Changes::lyxMarkChange(os, column, curtime,
- running_change, Change(Change::UNCHANGED));
-
os << "\n\\end_layout\n";
}
@@ -1639,14 +1633,14 @@
Change::Type Paragraph::lookupChange(lyx::pos_type pos) const
{
- BOOST_ASSERT(empty() || pos < size());
+ BOOST_ASSERT(pos <= size());
return pimpl_->lookupChange(pos);
}
Change const Paragraph::lookupChangeFull(lyx::pos_type pos) const
{
- BOOST_ASSERT(empty() || pos < size());
+ BOOST_ASSERT(pos <= size());
return pimpl_->lookupChangeFull(pos);
}
@@ -1669,6 +1663,12 @@
}
+void Paragraph::setChangeFull(lyx::pos_type pos, Change change)
+{
+ pimpl_->setChangeFull(pos, change);
+}
+
+
void Paragraph::markErased(bool erased)
{
pimpl_->markErased(erased);
Index: text.C
===================================================================
--- text.C (revision 13312)
+++ text.C (working copy)
@@ -315,8 +315,10 @@
} else if (token == "\\change_unchanged") {
// Hack ! Needed for empty paragraphs :/
// FIXME: is it still ??
+ /*
if (!par.size())
par.cleanChanges();
+ */
change = Change(Change::UNCHANGED);
} else if (token == "\\change_inserted") {
lex.eatLine();
@@ -375,6 +377,9 @@
break;
}
}
+ // Final change goes to paragraph break:
+ par.setChangeFull(par.size(), change);
+
// Initialize begin_of_body_ on load; redoParagraph maintains
par.setBeginOfBody();
}
@@ -1026,14 +1031,10 @@
void LyXText::breakParagraph(LCursor & cur, bool keep_layout)
{
BOOST_ASSERT(this == cur.text());
- // allow only if at start or end, or all previous is new text
+
Paragraph & cpar = cur.paragraph();
pit_type cpit = cur.pit();
- if (cur.pos() != 0 && cur.pos() != cur.lastpos()
- && cpar.isChangeEdited(0, cur.pos()))
- return;
-
LyXTextClass const & tclass = cur.buffer().params().getLyXTextClass();
LyXLayout_ptr const & layout = cpar.layout();
@@ -1088,6 +1089,12 @@
updateCounters(cur.buffer());
+ // Mark "carriage return" as inserted if change tracking:
+ if (cur.buffer().params().tracking_changes) {
+ cur.paragraph().setChange(cur.paragraph().size(),
+ Change::INSERTED);
+ }
+
// This check is necessary. Otherwise the new empty paragraph will
// be deleted automatically. And it is more friendly for the user!
if (cur.pos() != 0 || isempty)
@@ -1392,18 +1399,34 @@
if (!cur.selection() && cur.lastpos() != 0)
return;
- CursorSlice const & startc = cur.selBegin();
- CursorSlice const & endc = cur.selEnd();
- if (startc.pit() == endc.pit()) {
- recordUndoSelection(cur, Undo::INSERT);
- pars_[startc.pit()].acceptChange(startc.pos(), endc.pos());
- finishUndo();
- cur.clearSelection();
- setCursorIntern(cur, startc.pit(), 0);
+ recordUndoSelection(cur, Undo::INSERT);
+
+ DocIterator it = cur.selectionBegin();
+ DocIterator et = cur.selectionEnd();
+ pit_type pit = it.pit();
+ Change::Type const type = pars_[pit].lookupChange(it.pos());
+ for (; pit <= et.pit(); ++pit) {
+ pos_type left = ( pit == it.pit() ? it.pos() : 0 );
+ pos_type right =
+ ( pit == et.pit() ? et.pos() : pars_[pit].size() + 1 );
+ pars_[pit].acceptChange(left, right);
}
-#ifdef WITH_WARNINGS
-#warning handle multi par selection
-#endif
+ if (type == Change::DELETED) {
+ ParagraphList & plist = paragraphs();
+ if (it.pit() + 1 < et.pit())
+ pars_.erase(plist.begin() + it.pit() + 1,
+ plist.begin() + et.pit());
+
+ // Paragraph merge if appropriate:
+ if (pars_[it.pit()].lookupChange(pars_[it.pit()].size())
+ == Change::DELETED) {
+ setCursorIntern(cur, it.pit() + 1, 0);
+ backspacePos0(cur);
+ }
+ }
+ finishUndo();
+ cur.clearSelection();
+ setCursorIntern(cur, it.pit(), 0);
}
@@ -1413,18 +1436,33 @@
if (!cur.selection() && cur.lastpos() != 0)
return;
- CursorSlice const & startc = cur.selBegin();
- CursorSlice const & endc = cur.selEnd();
- if (startc.pit() == endc.pit()) {
- recordUndoSelection(cur, Undo::INSERT);
- pars_[startc.pit()].rejectChange(startc.pos(), endc.pos());
- finishUndo();
- cur.clearSelection();
- setCursorIntern(cur, startc.pit(), 0);
+ recordUndoSelection(cur, Undo::INSERT);
+
+ DocIterator it = cur.selectionBegin();
+ DocIterator et = cur.selectionEnd();
+ pit_type pit = it.pit();
+ Change::Type const type = pars_[pit].lookupChange(it.pos());
+ for (; pit <= et.pit(); ++pit) {
+ pos_type left = ( pit == it.pit() ? it.pos() : 0 );
+ pos_type right =
+ ( pit == et.pit() ? et.pos() : pars_[pit].size() + 1 );
+ pars_[pit].rejectChange(left, right);
}
-#ifdef WITH_WARNINGS
-#warning handle multi par selection
-#endif
+ if (type == Change::INSERTED) {
+ ParagraphList & plist = paragraphs();
+ if (it.pit() + 1 < et.pit())
+ pars_.erase(plist.begin() + it.pit() + 1,
+ plist.begin() + et.pit());
+ // Paragraph merge if appropriate:
+ if (pars_[it.pit()].lookupChange(pars_[it.pit()].size())
+ == Change::INSERTED) {
+ setCursorIntern(cur, it.pit() + 1, 0);
+ backspacePos0(cur);
+ }
+ }
+ finishUndo();
+ cur.clearSelection();
+ setCursorIntern(cur, it.pit(), 0);
}
@@ -1556,86 +1594,100 @@
}
-bool LyXText::backspace(LCursor & cur)
+bool LyXText::backspacePos0(LCursor & cur)
{
BOOST_ASSERT(this == cur.text());
bool needsUpdate = false;
- if (cur.pos() == 0) {
- // The cursor is at the beginning of a paragraph, so
- // the the backspace will collapse two paragraphs into
- // one.
- // but it's not allowed unless it's new
- Paragraph & par = cur.paragraph();
- if (par.isChangeEdited(0, par.size()))
- return false;
+ Paragraph & par = cur.paragraph();
+ // is it an empty paragraph?
+ pos_type lastpos = cur.lastpos();
+ if (lastpos == 0 || (lastpos == 1 && par.isSeparator(0))) {
+ // This is an empty paragraph and we delete it just
+ // by moving the cursor one step
+ // left and let the DeleteEmptyParagraphMechanism
+ // handle the actual deletion of the paragraph.
- // we may paste some paragraphs
-
- // is it an empty paragraph?
- pos_type lastpos = cur.lastpos();
- if (lastpos == 0 || (lastpos == 1 && par.isSeparator(0))) {
- // This is an empty paragraph and we delete it just
- // by moving the cursor one step
- // left and let the DeleteEmptyParagraphMechanism
- // handle the actual deletion of the paragraph.
-
- if (cur.pit() != 0) {
- // For KeepEmpty layouts we need to get
- // rid of the keepEmpty setting first.
- // And the only way to do this is to
- // reset the layout to something
- // else: f.ex. the default layout.
- if (par.allowEmpty()) {
- Buffer & buf = cur.buffer();
- BufferParams const & bparams =
buf.params();
-
par.layout(bparams.getLyXTextClass().defaultLayout());
- }
+ if (cur.pit() != 0) {
+ // For KeepEmpty layouts we need to get
+ // rid of the keepEmpty setting first.
+ // And the only way to do this is to
+ // reset the layout to something
+ // else: f.ex. the default layout.
+ if (par.allowEmpty()) {
+ Buffer & buf = cur.buffer();
+ BufferParams const & bparams = buf.params();
+
par.layout(bparams.getLyXTextClass().defaultLayout());
+ }
- cursorLeft(cur);
- return true;
- }
+ cursorLeft(cur);
+ return true;
}
+ }
- if (cur.pit() != 0)
- recordUndo(cur, Undo::DELETE, cur.pit() - 1);
+ if (cur.pit() != 0)
+ recordUndo(cur, Undo::DELETE, cur.pit() - 1);
- pit_type tmppit = cur.pit();
- // We used to do cursorLeftIntern() here, but it is
- // not a good idea since it triggers the auto-delete
- // mechanism. So we do a cursorLeftIntern()-lite,
- // without the dreaded mechanism. (JMarc)
- if (cur.pit() != 0) {
- // steps into the above paragraph.
- setCursorIntern(cur, cur.pit() - 1,
- pars_[cur.pit() - 1].size(),
- false);
- }
+ pit_type tmppit = cur.pit();
+ // We used to do cursorLeftIntern() here, but it is
+ // not a good idea since it triggers the auto-delete
+ // mechanism. So we do a cursorLeftIntern()-lite,
+ // without the dreaded mechanism. (JMarc)
+ if (cur.pit() != 0) {
+ // steps into the above paragraph.
+ setCursorIntern(cur, cur.pit() - 1,
+ pars_[cur.pit() - 1].size(),
+ false);
+ }
- // Pasting is not allowed, if the paragraphs have different
- // layout. I think it is a real bug of all other
- // word processors to allow it. It confuses the user.
- // Correction: Pasting is always allowed with standard-layout
- // Correction (Jug 20050717): Remove check about alignment!
- Buffer & buf = cur.buffer();
- BufferParams const & bufparams = buf.params();
- LyXTextClass const & tclass = bufparams.getLyXTextClass();
- pit_type const cpit = cur.pit();
+ // Pasting is not allowed, if the paragraphs have different
+ // layout. I think it is a real bug of all other
+ // word processors to allow it. It confuses the user.
+ // Correction: Pasting is always allowed with standard-layout
+ // Correction (Jug 20050717): Remove check about alignment!
+ Buffer & buf = cur.buffer();
+ BufferParams const & bufparams = buf.params();
+ LyXTextClass const & tclass = bufparams.getLyXTextClass();
+ pit_type const cpit = cur.pit();
- if (cpit != tmppit
- && (pars_[cpit].layout() == pars_[tmppit].layout()
- || pars_[tmppit].layout() == tclass.defaultLayout()))
- {
- mergeParagraph(bufparams, pars_, cpit);
- needsUpdate = true;
+ if (cpit != tmppit
+ && (pars_[cpit].layout() == pars_[tmppit].layout()
+ || pars_[tmppit].layout() == tclass.defaultLayout()))
+ {
+ mergeParagraph(bufparams, pars_, cpit);
+ needsUpdate = true;
- if (cur.pos() != 0 && pars_[cpit].isSeparator(cur.pos()
- 1))
+ if (cur.pos() != 0 && pars_[cpit].isSeparator(cur.pos() - 1))
--cur.pos();
- // the counters may have changed
- updateCounters(cur.buffer());
- setCursor(cur, cur.pit(), cur.pos(), false);
+ // the counters may have changed
+ updateCounters(cur.buffer());
+ setCursor(cur, cur.pit(), cur.pos(), false);
+ }
+ return needsUpdate;
+}
+
+
+bool LyXText::backspace(LCursor & cur)
+{
+ BOOST_ASSERT(this == cur.text());
+ bool needsUpdate = false;
+ if (cur.pos() == 0) {
+ // The cursor is at the beginning of a paragraph, so
+ // the the backspace will collapse two paragraphs into
+ // one.
+
+ if (cur.buffer().params().tracking_changes) {
+ // Previous paragraph, mark "carriage return" as
+ // deleted:
+ Paragraph & par = pars_[cur.pit() - 1];
+ par.setChange(par.size(), Change::DELETED);
+ setCursorIntern(cur, cur.pit() - 1, par.size());
+ return false;
}
+
+ needsUpdate = backspacePos0(cur);
+
} else {
// this is the code for a normal backspace, not pasting
// any paragraphs
@@ -2182,9 +2234,11 @@
std::ostringstream os;
bool const show_change = buf.params().tracking_changes
- && cur.pos() != cur.lastpos()
&& par.lookupChange(cur.pos()) != Change::UNCHANGED;
+ if (buf.params().tracking_changes)
+ os << "[C] ";
+
if (show_change) {
Change change = par.lookupChangeFull(cur.pos());
Author const & a = buf.params().authors().get(change.author);
pgpRsJoh6cn7O.pgp
Description: PGP signature
