Le 11/04/2016 17:33, Richard Heck a écrit :
On 04/11/2016 12:28 PM, Jean-Marc Lasgouttes wrote:
Le 11/04/2016 17:53, Scott Kostyshak a écrit :
I can confirm that I do not see the problem in 2.1.4 and I do see it
with 2.2.0dev.
I suppose this is a dataloss bug, and that there is a rule of not going
to an RC with a known dataloss bug that is a regression. Although the
bug only shows in a specific case and has gone unnoticed for more than a
year in 2.2.0dev, it is pretty easy to trigger and it would not be
surprising at all that a user comes across it. I would like to hear from
Jürgen to see if a quick fix can be made before proceeding with rc1.
Here is a very lightly tested patch. It looks OK to me, but this kind
of changes has potential to do more harm than good. So if we do not
have 1/ good user testing and 2/ a +1 from someone who understands
this code, I am not sure we want it in rc1. And rc1 really needs to
happen soon IMO.
This is exactly what I was going to do. A quicker and dirtier and very
safe (IMO) fix is:
@@ -75,6 +75,8 @@ void InsetFlex::write(ostream & os) const
// use il.name(), since this resolves obsoleted
// InsetLayout names
string name = to_utf8(il.name());
+ if (name == from_ascii("undefined"))
+ name == name_;
// Remove the "Flex:" prefix, if it is present
if (support::prefixIs(name, "Flex:"))
name = support::token(name, ':', 1);
You both beat me to it! Here's my take on it (with some cleanup and
replacing token with split).
Now on to commenting your patches.
>From 22f3371127079a4cb14c3ec8cb2edc3398149f66 Mon Sep 17 00:00:00 2001
From: Guillaume Munch <g...@lyx.org>
Date: Mon, 11 Apr 2016 17:33:14 +0100
Subject: [PATCH] Fix dataloss when flex inset undefined.
Regression at cfeddb929. If a flex inset is not defined upon saving (e.g. if a
module has been deleted) then its name became lost. I introduce a check of
whether the name resolution intorduced with the ObsoletedBy tag comes back
empty-handed (which it will if the layout is not defined). In this case we do as
was done before cfeddb929.
In addition I am a bit worried by the use of support::token to strip "Flex:" off
the beginning of the name, which looks incompatible to me with names containing
":". I replace it with support::split.
---
src/insets/InsetFlex.cpp | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/src/insets/InsetFlex.cpp b/src/insets/InsetFlex.cpp
index 44b9b03..0b14973 100644
--- a/src/insets/InsetFlex.cpp
+++ b/src/insets/InsetFlex.cpp
@@ -68,19 +68,24 @@ InsetLayout::InsetDecoration InsetFlex::decoration() const
void InsetFlex::write(ostream & os) const
{
os << "Flex ";
- InsetLayout const & il = getLayout();
+ string name;
if (name_.empty())
- os << "undefined";
+ name = "undefined";
else {
- // use il.name(), since this resolves obsoleted
- // InsetLayout names
- string name = to_utf8(il.name());
- // Remove the "Flex:" prefix, if it is present
- if (support::prefixIs(name, "Flex:"))
- name = support::token(name, ':', 1);
- os << name;
+ InsetLayout const & il = getLayout();
+ // use il.name(), since this resolves obsoleted InsetLayout names
+ if (il.name() == "undefined")
+ // This is the name of the plain_insetlayout_. We assume that the
+ // name resolution has failed.
+ name = name_;
+ else {
+ name = to_utf8(il.name());
+ // Remove the "Flex:" prefix, if it is present
+ if (support::prefixIs(name, "Flex:"))
+ name = support::split(name, ':');
+ }
}
- os << "\n";
+ os << name << "\n";
InsetCollapsable::write(os);
}
--
2.1.4
