From 86bd1cd641daade1524a6edf7b3f733117e8afc7 Mon Sep 17 00:00:00 2001 From: Guillaume Munch Date: Wed, 11 Nov 2015 21:31:05 +0000 Subject: [PATCH] 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. --- src/Author.cpp | 51 ++++++++++++++++++++++++++++++++------------ src/Author.h | 6 ++++-- src/BufferParams.cpp | 11 +++++++++- src/BufferParams.h | 6 ++++-- src/Text.cpp | 29 +++++++++++++++---------- 5 files changed, 73 insertions(+), 30 deletions(-) diff --git a/src/Author.cpp b/src/Author.cpp index 79e7de0864..d44af3df49 100644 --- 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(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(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 f798b041b2..6915318531 100644 --- 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 3af14272db..029e5b524c 100644 --- 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 a0e0d459f3..79af8f21a6 100644 --- 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 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 67776a0d40..80babde39a 100644 --- 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(&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"), -- 2.39.2