Fix bug #9854 "Dataloss after git merge with change tracking"

A plausible scenario is that change tracking is used together with a versioning
system. In this case, parallel modifications might remove an \author line on one
side, and add another change of this author on the other side. This scenario
causes a bad merge after which the added change has no associated author. In
this case, LyX used to display a list of errors on opening and deliberately
removed the corresponding change tracking information.

* If ever a tracked change refers to an author that does not exist, then add a
  dummy author. This dummy author is not saved to the file afterwards.

* Have a very clear error message on opening such a corrupt file.

diff --git a/src/Author.cpp b/src/Author.cpp
index 79e7de0864b942d059a5ec1502eccb475f17b792..d44af3df4994478716d10c6422b537a15411e7ed 100644 (file)
--- a/src/Author.cpp
+++ b/src/Author.cpp
@@ -12,6 +12,7 @@
 
 #include "Author.h"
 
+#include "support/convert.h"
 #include "support/lassert.h"
 #include "support/lstrings.h"
 
@@ -36,9 +37,21 @@ static int computeHash(docstring const & name,
 
 
 Author::Author(docstring const & name, docstring const & email)
-       : name_(name), email_(email), used_(true)
+       : name_(name), email_(email), used_(true),
+         buffer_id_(computeHash(name, email))
+{}
+
+
+Author::Author(int buffer_id)
+       : name_(convert<docstring>(buffer_id)), email_(docstring()), used_(false),
+         buffer_id_(buffer_id)
+{}
+
+
+bool Author::valid() const
 {
-       buffer_id_ = computeHash(name_, email_);
+       //this cannot be equal if the buffer_id was produced by the hash function.
+       return name_ != convert<docstring>(buffer_id_);
 }
 
 
@@ -77,26 +90,36 @@ bool author_smaller(Author const & lhs, Author const & rhs)
 
 
 AuthorList::AuthorList()
-       : last_id_(0)
 {}
 
 
 int AuthorList::record(Author const & a)
 {
+       bool const valid = a.valid();
        // If we record an author which equals the current
        // author, we copy the buffer_id, so that it will
        // keep the same id in the file.
-       if (!authors_.empty() && a == authors_[0])
+       if (valid && !authors_.empty() && a == authors_[0])
                authors_[0].setBufferId(a.bufferId());
 
-       Authors::const_iterator it(authors_.begin());
-       Authors::const_iterator itend(authors_.end());
-       for (int i = 0;  it != itend; ++it, ++i) {
-               if (*it == a)
-                       return i;
+       Authors::const_iterator it = authors_.begin();
+       Authors::const_iterator const beg = it;
+       Authors::const_iterator const end = authors_.end();
+       for (; it != end; ++it) {
+               if (valid && *it == a)
+                       return it - beg;
+               if (it->bufferId() == a.bufferId()) {
+                       int id = it - beg;
+                       if (!it->valid())
+                               // we need to handle the case of a valid author being registred
+                               // after an invalid one. For instance, because "buffer-reload"
+                               // does not clear the buffer's author list.
+                               record(id, a);
+                       return id;
+               }
        }
        authors_.push_back(a);
-       return last_id_++;
+       return authors_.size() - 1;
 }
 
 
@@ -146,10 +169,10 @@ ostream & operator<<(ostream & os, AuthorList const & a)
        sorted.sort();
 
        AuthorList::Authors::const_iterator a_it = sorted.begin();
-       AuthorList::Authors::const_iterator a_end = sorted.end();
-       
-       for (a_it = sorted.begin(); a_it != a_end; ++a_it) {
-               if (a_it->used())
+       AuthorList::Authors::const_iterator const a_end = sorted.end();
+
+       for (; a_it != a_end; ++a_it) {
+               if (a_it->used() && a_it->valid())
                        os << "\\author " << *a_it << "\n";     
        }
        return os;

diff --git a/src/Author.h b/src/Author.h
index f798b041b2df5e4b00e6eb62d8e794ae7e0357a3..6915318531802f2eea40536bc50ac0227ef984d9 100644 (file)
--- a/src/Author.h
+++ b/src/Author.h
@@ -25,6 +25,8 @@ public:
        Author() : used_(false), buffer_id_(0) {};
        ///
        Author(docstring const & name, docstring const & email);
+       /// For when the \author line is missing (#9854)
+       Author(int buffer_id);
        ///
        docstring name() const { return name_; }
        ///
@@ -37,6 +39,8 @@ public:
        void setUsed(bool u) const { used_ = u; }
        ///
        bool used() const { return used_; }
+       /// Was the author line not missing?
+       bool valid() const;
        ///
        friend std::istream & operator>>(std::istream & os, Author & a);
        ///
@@ -78,8 +82,6 @@ public:
        friend
        std::ostream & operator<<(std::ostream & os, AuthorList const & a);
 private:
-       ///
-       int last_id_;
        ///
        Authors authors_;
 };

diff --git a/src/BufferParams.cpp b/src/BufferParams.cpp
index 3af14272dbbf18673fe582d2366a0012e2bab2c5..029e5b524c27cbb1c2c900b99c57f76b5e7f8f64 100644 (file)
--- a/src/BufferParams.cpp
+++ b/src/BufferParams.cpp
@@ -429,6 +429,9 @@ BufferParams::BufferParams()
 
        output_sync = false;
        use_refstyle = true;
+
+       // map current author
+       author_map_[pimpl_->authorlist.get(0).bufferId()] = 0;
 }
 
 
@@ -504,6 +507,12 @@ AuthorList const & BufferParams::authors() const
 }
 
 
+void BufferParams::addAuthor(Author a)
+{
+       author_map_[a.bufferId()] = pimpl_->authorlist.record(a);
+}
+
+
 BranchList & BufferParams::branchlist()
 {
        return pimpl_->branchlist;
@@ -876,7 +885,7 @@ string BufferParams::readToken(Lexer & lex, string const & token,
                istringstream ss(lex.getString());
                Author a;
                ss >> a;
-               author_map[a.bufferId()] = pimpl_->authorlist.record(a);
+               addAuthor(a);
        } else if (token == "\\paperorientation") {
                string orient;
                lex >> orient;

diff --git a/src/BufferParams.h b/src/BufferParams.h
index a0e0d459f3460652b6f74834becd762bbdec06a0..79af8f21a678bd60a28c5fd625fa5fbf8ff6e900 100644 (file)
--- a/src/BufferParams.h
+++ b/src/BufferParams.h
@@ -15,6 +15,7 @@
 #ifndef BUFFERPARAMS_H
 #define BUFFERPARAMS_H
 
+#include "Author.h"
 #include "Citation.h"
 #include "DocumentClassPtr.h"
 #include "Format.h"
@@ -33,7 +34,6 @@ namespace lyx {
 
 namespace support { class FileName; }
 
-class AuthorList;
 class BranchList;
 class Bullet;
 class DocumentClass;
@@ -394,10 +394,12 @@ public:
        /// the author list for the document
        AuthorList & authors();
        AuthorList const & authors() const;
+       void addAuthor(Author a);
 
        /// map of the file's author IDs to AuthorList indexes
        typedef std::map<int, int> AuthorMap;
-       AuthorMap author_map;
+       AuthorMap author_map_;
+
        /// the buffer's active font encoding
        std::string const font_encoding() const;
        /// all font encodings requested by the prefs/document/main language.

diff --git a/src/Text.cpp b/src/Text.cpp
index 67776a0d40df01a4cf8655c9f33a00188e29ccb3..80babde39a58a65b8361faab62e329891f5c9f1e 100644 (file)
--- a/src/Text.cpp
+++ b/src/Text.cpp
@@ -360,7 +360,7 @@ void Text::readParToken(Paragraph & par, Lexer & lex,
        string const & token, Font & font, Change & change, ErrorList & errorList)
 {
        Buffer * buf = const_cast<Buffer *>(&owner_->buffer());
-       BufferParams const & bp = buf->params();
+       BufferParams & bp = buf->params();
 
        if (token[0] != '\\') {
                docstring dstr = lex.getDocString();
@@ -534,18 +534,25 @@ void Text::readParToken(Paragraph & par, Lexer & lex,
                int aid;
                time_t ct;
                is >> aid >> ct;
-               BufferParams::AuthorMap const & am = bp.author_map;
+               BufferParams::AuthorMap const & am = bp.author_map_;
                if (am.find(aid) == am.end()) {
-                       errorList.push_back(ErrorItem(_("Change tracking error"),
-                                           bformat(_("Unknown author index for change: %1$d\n"), aid),
-                                           par.id(), 0, par.size()));
-                       change = Change(Change::UNCHANGED);
-               } else {
-                       if (token == "\\change_inserted")
-                               change = Change(Change::INSERTED, am.find(aid)->second, ct);
-                       else
-                               change = Change(Change::DELETED, am.find(aid)->second, ct);
+                       errorList.push_back(ErrorItem(
+                               _("Change tracking author index missing"),
+                               bformat(_("A change tracking author information for index "
+                                         "%1$d is missing. This can happen after a wrong "
+                                         "merge by a version control system. In this case, "
+                                         "either fix the merge, or have this information "
+                                         "missing until the corresponding tracked changes "
+                                         "are merged or this user edits the file again.\n"),
+                                       aid),
+                               par.id(), par.size(), par.size() + 1
+                               ));
+                       bp.addAuthor(Author(aid));
                }
+               if (token == "\\change_inserted")
+                       change = Change(Change::INSERTED, am.find(aid)->second, ct);
+               else
+                       change = Change(Change::DELETED, am.find(aid)->second, ct);
        } else {
                lex.eatLine();
                errorList.push_back(ErrorItem(_("Unknown token"),