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
 

Attachment: pgpcvpod7vtB9.pgp
Description: PGP signature

Reply via email to