On 04/11/2016 12:57 PM, Guillaume Munch wrote: > Le 11/04/2016 17:28, Jean-Marc Lasgouttes a écrit : >> 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. >> >> JMarc >> >> >> >> >> 0001-Fix-saving-of-unknown-flex-insets.patch >> >> >> From 86774cd7123cbb34c235bdcab9d11eb09c8fc6b8 Mon Sep 17 00:00:00 2001 >> From: Jean-Marc Lasgouttes<[email protected]> >> Date: Mon, 11 Apr 2016 18:16:05 +0200 >> Subject: [PATCH] Fix saving of unknown flex insets >> >> After commit cfeddb9293b, flex insets without a proper inset layout >> (for example because a module has been removed) are saved with name >> "undefined". This can result in a data loss. >> >> Introduce a new method InsetFlex::hasLayout that uses the same logic >> as getLayout, but only returns a bool. Use this to decide when >> outputting the raw inset name is a good idea. >> --- >> src/insets/InsetFlex.cpp | 18 ++++++++++++++---- >> src/insets/InsetFlex.h | 2 ++ >> 2 files changed, 16 insertions(+), 4 deletions(-) >> >> diff --git a/src/insets/InsetFlex.cpp b/src/insets/InsetFlex.cpp >> index 44b9b03..edff6cb 100644 >> --- a/src/insets/InsetFlex.cpp >> +++ b/src/insets/InsetFlex.cpp >> @@ -45,6 +45,18 @@ InsetFlex::InsetFlex(InsetFlex const & in) >> >> >> // special code for InsetFlex when there is not the explicit Flex:: >> prefix >> +bool InsetFlex::hasLayout() const >> +{ >> + if (!buffer_) >> + return false; >> + >> + DocumentClass const & dc = buffer().params().documentClass(); >> + return dc.hasInsetLayout(from_utf8(name_)) >> + || dc.hasInsetLayout(from_utf8("Flex:" + name_)); >> +} >> + >> + >> +// special code for InsetFlex when there is not the explicit Flex:: >> prefix >> InsetLayout const & InsetFlex::getLayout() const >> { >> if (!buffer_) >> @@ -68,13 +80,11 @@ InsetLayout::InsetDecoration >> InsetFlex::decoration() const >> void InsetFlex::write(ostream & os) const >> { >> os << "Flex "; >> - InsetLayout const & il = getLayout(); >> if (name_.empty()) >> os << "undefined"; >> else { >> - // use il.name(), since this resolves obsoleted >> - // InsetLayout names >> - string name = to_utf8(il.name()); >> + // Using InsetLayout::name() resolves obsoleted InsetLayout >> names >> + string name = hasLayout() ? to_utf8(getLayout().name()) : >> name_; >> // Remove the "Flex:" prefix, if it is present >> if (support::prefixIs(name, "Flex:")) >> name = support::token(name, ':', 1); > > > Again, why stripping "Flex:" off the beginning of name_? This is not > how it is done before the regression. Or did I miss something?
name_ wouldn't have contained "Flex:". It's there because InsetLayout::name() includes it. > Also I think it is safer to replace support::token with support::split. Actually, it would be easier just to use substr(5). We know what we're removing. Richard
