Dear list,
The patch below fixes bug #9854. (<http://www.lyx.org/trac/ticket/9854>)
Find more details in the commit log.
I have two questions:
* I had to remove a const qualifier in Text::readParToken (see patch
below). Do I need to put it back? If so, what's the proper way of
adding the dummy author?
* Georg, you told us an old story of "FIXME: UNICODE"s being placed in
the source when the developers decided that docstring would be the only
place where we see Unicode. I am happy to fix these as I go through the
code, but I am not sure that I understood what's the problem typically.
Here have a look at operator>> in Author.cpp below. What's the problem
and what should typically be done there?
Scott, this can wait until after alpha, as you want.
Sincerely,
Guillaume
>From d6a7ae1ab965f9cde4b0dd5df1716f50aafbe010 Mon Sep 17 00:00:00 2001
From: Guillaume Munch <g...@lyx.org>
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 replaced by the real author at the next
occasion. (Fixes the main cause of data loss.)
* Keep all \author lines that do not correspond to the current author. (Play
nice with git.)
---
src/Author.cpp | 42 +++++++++++++++++++++++++++++-------------
src/Author.h | 4 ++--
src/Buffer.cpp | 14 +++++++++++---
src/BufferParams.cpp | 11 ++++++++++-
src/BufferParams.h | 6 ++++--
src/Text.cpp | 17 +++++++++--------
6 files changed, 65 insertions(+), 29 deletions(-)
diff --git a/src/Author.cpp b/src/Author.cpp
index 79e7de0..24ab0c6 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,10 +37,15 @@ static int computeHash(docstring const & name,
Author::Author(docstring const & name, docstring const & email)
- : name_(name), email_(email), used_(true)
-{
- buffer_id_ = computeHash(name_, email_);
-}
+ : name_(name), email_(email), used_(true),
+ buffer_id_(computeHash(name, email))
+{}
+
+
+Author::Author(int buffer_id)
+ : name_(convert<docstring>(buffer_id)), email_(docstring()), used_(true),
+ buffer_id_(buffer_id)
+{}
bool operator==(Author const & l, Author const & r)
@@ -66,6 +72,7 @@ istream & operator>>(istream & is, Author & a)
// FIXME UNICODE
a.name_ = from_utf8(trim(token(s, '\"', 1)));
a.email_ = from_utf8(trim(token(s, '\"', 2)));
+ a.used_ = true;
return is;
}
@@ -77,7 +84,6 @@ bool author_smaller(Author const & lhs, Author const & rhs)
AuthorList::AuthorList()
- : last_id_(0)
{}
@@ -89,14 +95,24 @@ int AuthorList::record(Author const & a)
if (!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) {
+ Authors::const_iterator it = authors_.begin();
+ Authors::const_iterator const beg = it;
+ Authors::const_iterator const end = authors_.end();
+ for (; it != end; ++it) {
if (*it == a)
- return i;
+ return it - beg;
+ if (it->bufferId() == a.bufferId()) {
+ int id = it - beg;
+ // bug #9854
+ // If the name is equal to the buffer_id then it means that the
+ // \author line was missing. We restore this information.
+ if (it->name() == convert<docstring>(it->bufferId()))
+ record(id, a);
+ return id;
+ }
}
authors_.push_back(a);
- return last_id_++;
+ return authors_.size() - 1;
}
@@ -146,9 +162,9 @@ 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) {
+ AuthorList::Authors::const_iterator const a_end = sorted.end();
+
+ for (; a_it != a_end; ++a_it) {
if (a_it->used())
os << "\\author " << *a_it << "\n";
}
diff --git a/src/Author.h b/src/Author.h
index f798b04..9805218 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
+ Author(int buffer_id);
///
docstring name() const { return name_; }
///
@@ -79,8 +81,6 @@ public:
std::ostream & operator<<(std::ostream & os, AuthorList const & a);
private:
///
- int last_id_;
- ///
Authors authors_;
};
diff --git a/src/Buffer.cpp b/src/Buffer.cpp
index f2eae77..edf8466 100644
--- a/src/Buffer.cpp
+++ b/src/Buffer.cpp
@@ -1597,11 +1597,19 @@ bool Buffer::write(ostream & ofs) const
<< "\\lyxformat " << LYX_FORMAT << "\n"
<< "\\begin_document\n";
- /// For each author, set 'used' to true if there is a change
- /// by this author in the document; otherwise set it to 'false'.
+ /// For each author, we used to set 'used' to false if there is no change by
+ /// this author in the document. But 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. The new
+ /// behaviour is that only the \author line corresponding to the current
+ /// author is possibly removed. (Bug #9854 "Dataloss after git merge with
+ /// change tracking")
AuthorList::Authors::const_iterator a_it = params().authors().begin();
AuthorList::Authors::const_iterator a_end = params().authors().end();
- for (; a_it != a_end; ++a_it)
+ //for (; a_it != a_end; ++a_it)
+ if (a_it != a_end)//#9854
a_it->setUsed(false);
ParIterator const end = const_cast<Buffer *>(this)->par_iterator_end();
diff --git a/src/BufferParams.cpp b/src/BufferParams.cpp
index c7ec167..50103aa 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 6b2bb47..946ae90 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;
@@ -392,10 +392,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 67776a0..497dd03 100644
--- a/src/Text.cpp
+++ b/src/Text.cpp
@@ -360,7 +360,9 @@ 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();
+ //FIXME: put const back
+ BufferParams & bp = buf->params();
+ //BufferParams const & bp = buf->params();
if (token[0] != '\\') {
docstring dstr = lex.getDocString();
@@ -534,18 +536,17 @@ 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);
+ 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.1.4
