On Thu, Oct 12, 2006 at 05:09:36PM +0200, Michael Gerz wrote: > [EMAIL PROTECTED] wrote: > > >Change tracking: > > > > * src/paragraph.h: remove enum ChangeTracking; > > remove default parameters for insertChar and insertInset > > * src/BufferView.h: constify getCurrentChange() > > * src/changes.h: make Change constructor explicit > > * src/insets/*.C: > > * src/*.C: adjust accordingly; add FIXMEs > > > > > OK. I promised some comments. First, this patch is more or less in line > with my former CT branch. It makes the constructur of class Change > explicit and removes some default parameters. Moreover, I added FIXMEs > all over the place to spot all open issues in a structured way later. > Don't worry about them for the moment. I am very busy right now but I am > on vacation next week. > > Anyway, I think that I discovered some bugs in the 1.4 CT code: > > >URL: > >http://www.lyx.org/trac/file/lyx-devel/trunk/src/CutAndPaste.C?rev=15302 > >============================================================================== > >--- lyx-devel/trunk/src/CutAndPaste.C (original) > >+++ lyx-devel/trunk/src/CutAndPaste.C Wed Oct 11 22:01:32 2006 > >@@ -321,7 +321,7 @@ > > // FIXME: Change tracking (MG) > > bool const merge = !params.trackChanges || > > pars[pit].lookupChange(pars[pit].size()) == > >- Change::INSERTED; > >+ Change(Change::INSERTED); > > pos_type const left = ( pit == startpit ? startpos : 0 ); > > pos_type const right = ( pit == endpit ? endpos : > > pars[pit].size() + 1 ); > > > > > Question: Shouldn't we compare the change types only rather than the > complete changes (including changetime and author). There may be > problems if different authors work on the same document.
Hmmm, you're wiser than me here. Sounds plausible. Did you see any changed behaviour with this? > >Modified: lyx-devel/trunk/src/rowpainter.C > >URL: > >http://www.lyx.org/trac/file/lyx-devel/trunk/src/rowpainter.C?rev=15302 > >============================================================================== > >--- lyx-devel/trunk/src/rowpainter.C (original) > >+++ lyx-devel/trunk/src/rowpainter.C Wed Oct 11 22:01:32 2006 > >@@ -282,7 +282,8 @@ > > if (pos < font_span.first || pos > font_span.last) > > break; > > > >- if (prev_change != par_.lookupChange(pos)) > >+ // FIXME: change tracking (MG) > >+ if (Change(prev_change) != par_.lookupChange(pos)) > > break; > > > > char_type c = par_.getChar(pos); > > > > > Same here? > > >Modified: lyx-devel/trunk/src/text.C > >URL: http://www.lyx.org/trac/file/lyx-devel/trunk/src/text.C?rev=15302 > >============================================================================== > >--- lyx-devel/trunk/src/text.C (original) > >+++ lyx-devel/trunk/src/text.C Wed Oct 11 22:01:32 2006 > >@@ -1257,7 +1257,8 @@ > > BOOST_ASSERT(cur.pos() > 0); > > if ((par.isLineSeparator(cur.pos() - 1) > > || par.isNewline(cur.pos() - 1)) > >- && par.lookupChange(cur.pos() - 1) != Change::DELETED) { > >+ // FIXME: change tracking (MG) > >+ && par.lookupChange(cur.pos() - 1) != > >Change(Change::DELETED)) { > > static bool sent_space_message = false; > > if (!sent_space_message) { > > cur.message(_("You cannot type two spaces > > this way. " > >@@ -1501,8 +1503,9 @@ > > boost::next(plist.begin(), et.pit())); > > > > // Paragraph merge if appropriate: > >+ // FIXME: change tracking (MG) > > if (pars_[it.pit()].lookupChange(pars_[it.pit()].size()) > >- == Change::DELETED) { > >+ == Change(Change::DELETED)) { > > setCursorIntern(cur, it.pit() + 1, 0); > > backspacePos0(cur); > > } > >@@ -1537,8 +1540,9 @@ > > pars_.erase(boost::next(plist.begin(), it.pit() + 1), > > boost::next(plist.begin(), et.pit())); > > // Paragraph merge if appropriate: > >+ // FIXME: change tracking (MG) > > if (pars_[it.pit()].lookupChange(pars_[it.pit()].size()) > >- == Change::INSERTED) { > >+ == Change(Change::INSERTED)) { > > setCursorIntern(cur, it.pit() + 1, 0); > > backspacePos0(cur); > > } > >@@ -1662,7 +1666,8 @@ > > recordUndo(cur, Undo::DELETE, cur.pit()); > > setCursorIntern(cur, cur.pit(), cur.pos() + 1, false, > > cur.boundary()); > > needsUpdate = backspace(cur); > >- if (cur.paragraph().lookupChange(cur.pos()) == > >Change::DELETED) > >+ // FIXME: change tracking (MG) > >+ if (cur.paragraph().lookupChange(cur.pos()) == > >Change(Change::DELETED)) > > cur.posRight(); > > } else if (cur.pit() != cur.lastpit()) { > > LCursor scur = cur; > >@@ -1675,7 +1680,8 @@ > > // FIXME: Change tracking (MG) > > // move forward after the paragraph break is > > DELETED > > Paragraph & par = cur.paragraph(); > >- if (par.lookupChange(par.size()) == > >Change::DELETED) > >+ // FIXME: change tracking (MG) > >+ if (par.lookupChange(par.size()) == > >Change(Change::DELETED)) > > setCursorIntern(cur, cur.pit() + 1, > > 0); > > } > > } else { > >@@ -1782,8 +1788,10 @@ > > // deleted: > > Paragraph & par = pars_[cur.pit() - 1]; > > // Take care of a just inserted para break: > >- if (par.lookupChange(par.size()) != > >Change::INSERTED) { > >- par.setChange(par.size(), Change::DELETED); > >+ // FIXME: change tracking (MG) > >+ if (par.lookupChange(par.size()) != > >Change(Change::INSERTED)) { > >+ // FIXME: change tracking (MG) > >+ par.setChange(par.size(), > >Change(Change::DELETED)); > > setCursorIntern(cur, cur.pit() - 1, > > par.size()); > > return true; > > } > >@@ -2372,7 +2381,8 @@ > > Paragraph const & par = cur.paragraph(); > > std::ostringstream os; > > > >- bool const show_change = par.lookupChange(cur.pos()) != > >Change::UNCHANGED; > >+ // FIXME: change tracking (MG) > >+ bool const show_change = par.lookupChange(cur.pos()) != > >Change(Change::UNCHANGED); > > > > if (buf.params().trackChanges) > > os << "[C] "; > > > >Modified: lyx-devel/trunk/src/text2.C > >URL: http://www.lyx.org/trac/file/lyx-devel/trunk/src/text2.C?rev=15302 > >============================================================================== > >--- lyx-devel/trunk/src/text2.C (original) > >+++ lyx-devel/trunk/src/text2.C Wed Oct 11 22:01:32 2006 > >@@ -1260,10 +1261,12 @@ > > && old.pos() < oldpar.size() > > && oldpar.isLineSeparator(old.pos()) > > && oldpar.isLineSeparator(old.pos() - 1) > >- && oldpar.lookupChange(old.pos() - 1) != > >Change::DELETED) { > >+ // FIXME: change tracking (MG) > >+ && oldpar.lookupChange(old.pos() - 1) != > >Change(Change::DELETED)) { > > // We need to set the text to Change::INSERTED to > > // get it erased properly > >- oldpar.setChange(old.pos() -1, Change::INSERTED); > >+ // FIXME: change tracking (MG) > >+ oldpar.setChange(old.pos() -1, > >Change(Change::INSERTED)); > > oldpar.erase(old.pos() - 1); > >#ifdef WITH_WARNINGS > >#warning This will not work anymore when we have multiple views of the > >same buffer > > > > > > And here... ??? > > Michael - Martin
pgpcvpod7vtB9.pgp
Description: PGP signature