subject:"beta2\?"

On Mon, Apr 11, 2016 at 10:04:38PM +0100, Guillaume Munch wrote:
> Le 11/04/2016 21:48, Andrew Parsloe a écrit :
> >Jean-Marc earlier suggested that I make a trac ticket. Time zones have
> >obviously got in the way. Is a ticket still needed?
> >
> 
> I don't think so. Thanks for the report, I know I would have suffered from
> that bug at some point.

+1 thanks to both of you.

Scott


signature.asc
Description: PGP signature

Re: Regression in saving of flex insets in beta2?


Le 11/04/2016 21:48, Andrew Parsloe a écrit :

Jean-Marc earlier suggested that I make a trac ticket. Time zones have
obviously got in the way. Is a ticket still needed?



I don't think so. Thanks for the report, I know I would have suffered 
from that bug at some point.


Guillaume

Re: Regression in saving of flex insets in beta2?

2016-04-11 Thread Andrew Parsloe



On 12/04/2016 8:17 a.m., Guillaume Munch wrote:

Le 11/04/2016 20:53, Scott Kostyshak a écrit :

On Mon, Apr 11, 2016 at 08:27:42PM +0100, Guillaume Munch wrote:

Le 11/04/2016 20:10, Richard Heck a écrit :
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.



I am not sure that you understood my question. Are you making the
assumption that nobody ever wrote Flex:Flex: just to spare an "else"
branch?


This would be a bad idea, but it saves a few cycles to do "else if"
anyway, so it's fine.





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.



Sure, or whatever equivalent function. I can confirm that this gives
another regression, whereby

\begin_inset Flex x:y


>from 2.1.4 will get replaced by


\begin_inset Flex x

in 2.2 upon saving. In particular when the layout Flex:x:y is defined
and Flex:x is not (or to something entirely different). This 
should be

patched as well IMO.


I'm not sure I see why this is---or which version of which patch is 
causing

it---but I'm prepared to believe it.


Anytime support::token is used.



To summarise, I think the best solution in the long run is 
Jean-Marc's
but with these two issues above being corrected. In the short term 
it is
safer to go with my patch, which is essentially Richard's "safe" 
patch

plus these two issues corrected. Let me know what you think.


That's fine with me.


Waiting for +1.


Go ahead. Thanks for the quick action on this.



Done
Jean-Marc earlier suggested that I make a trac ticket. Time zones have 
obviously got in the way. Is a ticket still needed?


Andrew

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

Re: Regression in saving of flex insets in beta2?


Le 11/04/2016 20:53, Scott Kostyshak a écrit :

On Mon, Apr 11, 2016 at 08:27:42PM +0100, Guillaume Munch wrote:

Le 11/04/2016 20:10, Richard Heck a écrit :

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.



I am not sure that you understood my question. Are you making the
assumption that nobody ever wrote Flex:Flex: just to spare an "else"
branch?


This would be a bad idea, but it saves a few cycles to do "else if"
anyway, so it's fine.






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.



Sure, or whatever equivalent function. I can confirm that this gives
another regression, whereby

\begin_inset Flex x:y


>from 2.1.4 will get replaced by


\begin_inset Flex x

in 2.2 upon saving. In particular when the layout Flex:x:y is defined
and Flex:x is not (or to something entirely different). This should be
patched as well IMO.


I'm not sure I see why this is---or which version of which patch is causing
it---but I'm prepared to believe it.


Anytime support::token is used.




To summarise, I think the best solution in the long run is Jean-Marc's
but with these two issues above being corrected. In the short term it is
safer to go with my patch, which is essentially Richard's "safe" patch
plus these two issues corrected. Let me know what you think.


That's fine with me.


Waiting for +1.


Go ahead. Thanks for the quick action on this.



Done

Re: Regression in saving of flex insets in beta2?

On Mon, Apr 11, 2016 at 08:27:42PM +0100, Guillaume Munch wrote:
> Le 11/04/2016 20:10, Richard Heck a écrit :
> 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.
> >>
> >>
> >>I am not sure that you understood my question. Are you making the
> >>assumption that nobody ever wrote Flex:Flex: just to spare an "else"
> >>branch?
> >
> >This would be a bad idea, but it saves a few cycles to do "else if"
> >anyway, so it's fine.
> >
> >>
> >>>
> 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.
> >>>
> >>
> >>Sure, or whatever equivalent function. I can confirm that this gives
> >>another regression, whereby
> >>
> >>\begin_inset Flex x:y
> >>
> >>from 2.1.4 will get replaced by
> >>
> >>\begin_inset Flex x
> >>
> >>in 2.2 upon saving. In particular when the layout Flex:x:y is defined
> >>and Flex:x is not (or to something entirely different). This should be
> >>patched as well IMO.
> >
> >I'm not sure I see why this is---or which version of which patch is causing
> >it---but I'm prepared to believe it.
> 
> Anytime support::token is used.
> 
> >
> >>To summarise, I think the best solution in the long run is Jean-Marc's
> >>but with these two issues above being corrected. In the short term it is
> >>safer to go with my patch, which is essentially Richard's "safe" patch
> >>plus these two issues corrected. Let me know what you think.
> >
> >That's fine with me.
> 
> Waiting for +1.

Go ahead. Thanks for the quick action on this.

Scott


signature.asc
Description: PGP signature

Re: Regression in saving of flex insets in beta2?


Le 11/04/2016 20:10, Richard Heck a écrit :

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.



I am not sure that you understood my question. Are you making the
assumption that nobody ever wrote Flex:Flex: just to spare an "else"
branch?


This would be a bad idea, but it saves a few cycles to do "else if"
anyway, so it's fine.






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.



Sure, or whatever equivalent function. I can confirm that this gives
another regression, whereby

\begin_inset Flex x:y

from 2.1.4 will get replaced by

\begin_inset Flex x

in 2.2 upon saving. In particular when the layout Flex:x:y is defined
and Flex:x is not (or to something entirely different). This should be
patched as well IMO.


I'm not sure I see why this is---or which version of which patch is causing
it---but I'm prepared to believe it.


Anytime support::token is used.




To summarise, I think the best solution in the long run is Jean-Marc's
but with these two issues above being corrected. In the short term it is
safer to go with my patch, which is essentially Richard's "safe" patch
plus these two issues corrected. Let me know what you think.


That's fine with me.


Waiting for +1.


Guillaume

Re: Regression in saving of flex insets in beta2?

2016-04-11 Thread Richard Heck

On 04/11/2016 02:25 PM, Guillaume Munch wrote:
> Le 11/04/2016 19:02, Richard Heck a écrit :
>> 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
 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.
>
>
> I am not sure that you understood my question. Are you making the
> assumption that nobody ever wrote Flex:Flex: just to spare an "else"
> branch?

This would be a bad idea, but it saves a few cycles to do "else if"
anyway, so it's fine.

>
>>
>>> 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.
>>
>
> Sure, or whatever equivalent function. I can confirm that this gives
> another regression, whereby
>
> \begin_inset Flex x:y
>
> from 2.1.4 will get replaced by
>
> \begin_inset Flex x
>
> in 2.2 upon saving. In particular when the layout Flex:x:y is defined
> and Flex:x is not (or to something entirely different). This should be
> patched as well IMO.

I'm not sure I see why this is---or which version of which patch is causing
it---but I'm prepared to believe it.

> To summarise, I think the best solution in the long run is Jean-Marc's
> but with these two issues above being corrected. In the short term it is
>

Re: Regression in saving of flex insets in beta2?


Le 11/04/2016 19:02, Richard Heck a écrit :

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
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.



I am not sure that you understood my question. Are you making the
assumption that nobody ever wrote Flex:Flex: just to spare an "else" branch?





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.



Sure, or whatever equivalent function. I can confirm that this gives
another regression, whereby

\begin_inset Flex x:y

from 2.1.4 will get replaced by

\begin_inset Flex x

in 2.2 upon saving. In particular when the layout Flex:x:y is defined
and Flex:x is not (or to something entirely different). This should be
patched as well IMO.


To summarise, I think the best solution in the long run is Jean-Marc's
but with these two issues above being corrected. In the short term it is
safer to go with my patch, which is essentially Richard's "safe" patch
plus these two issues corrected. Let me know what you think.


Guillaume

Re: Regression in saving of flex insets in beta2?

2016-04-11 Thread Richard Heck

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
>> 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

Re: Regression in saving of flex insets in beta2?


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
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?


Also I think it is safer to replace support::token with support::split.

I like your patch which is more logical, and I also agree that we might 
want something safer for 2.2.



Guillaume

Re: Regression in saving of flex insets in beta2?


Le 11/04/2016 17:39, Jean-Marc Lasgouttes a écrit :

Le 11/04/2016 18:33, Richard Heck a écrit :

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);


Definitely not code that we would want to keep, IMO, but maybe safer for
rc1, indeed.




This is also my understanding (provided we take my version). It makes a
bit more sense if we clarify that testing "undefined" I just a way to
ensure that the resolved layout is not plain_insetlayout_, otherwise we
should assume that no layout has been found.

Re: Regression in saving of flex insets in beta2?


Le 11/04/2016 17:33, Richard Heck a écrit :


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);



The idea in my patch is similar to Richard's, but I would not go with 
this particular one. Indeed, in the older code the "Flex:" prefix was 
not removed, so it should be "else if" before support::prefixIs.

Re: Regression in saving of flex insets in beta2?


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 
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

Re: Regression in saving of flex insets in beta2?


Le 11/04/2016 18:33, Richard Heck a écrit :

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);


Definitely not code that we would want to keep, IMO, but maybe safer for 
rc1, indeed.


JMarc

Re: Regression in saving of flex insets in beta2?

2016-04-11 Thread Richard Heck

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);

Richard

Re: Regression in saving of flex insets in beta2?


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



>From 86774cd7123cbb34c235bdcab9d11eb09c8fc6b8 Mon Sep 17 00:00:00 2001
From: Jean-Marc Lasgouttes 
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);
diff --git a/src/insets/InsetFlex.h b/src/insets/InsetFlex.h
index 8277002..86f2f18 100644
--- a/src/insets/InsetFlex.h
+++ b/src/insets/InsetFlex.h
@@ -27,6 +27,8 @@ public:
 	///
 	docstring layoutName() const { return from_utf8("Flex:" + name_); }
 	///
+	bool hasLayout() const;
+	///
 	InsetLayout const & getLayout() const;
 	///
 	InsetCode lyxCode() const { return FLEX_CODE; }
-- 
1.7.9.5

Re: Regression in saving of flex insets in beta2?

On Mon, Apr 11, 2016 at 04:45:02PM +0100, Guillaume Munch wrote:
> Le 11/04/2016 12:36, Jean-Marc Lasgouttes a écrit :
> >Le 11/04/2016 07:27, Andrew Parsloe a écrit :
> >>Suppose a module defines flex insets moo and baa (the locale-dependent
> >>version of foo and bah in New Zealand), and these are used in a document
> >>to which the module has been added. If the module is now deleted from
> >>the document, the flex insets become undefined. If the document is saved
> >>then closed with undefined insets, the saved file will contain lines like
> >>
> >>\begin_inset Flex undefined
> >
> >Please open a new ticket. I think it is a consequence of
> >
> >commit cfeddb9293bb1e3b45fe2f118a6ce35e37bbebf3
> 
> 
> Which is confirmed by git bisect.

Thanks for checking.

> If I understand correctly, one can now lose data just by having a module
> not properly installed.

I understand the same. And we can imagine other ways of triggering it. A
user can mistakenly delete a module.

Scott


signature.asc
Description: PGP signature

Re: Regression in saving of flex insets in beta2?

On Mon, Apr 11, 2016 at 01:36:28PM +0200, Jean-Marc Lasgouttes wrote:
> Le 11/04/2016 07:27, Andrew Parsloe a écrit :
> >Suppose a module defines flex insets moo and baa (the locale-dependent
> >version of foo and bah in New Zealand), and these are used in a document
> >to which the module has been added. If the module is now deleted from
> >the document, the flex insets become undefined. If the document is saved
> >then closed with undefined insets, the saved file will contain lines like
> >
> >\begin_inset Flex undefined
> 
> Please open a new ticket.

Yes and mark it with milestone 2.2.0.

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.

> I think it is a consequence of
> 
> commit cfeddb9293bb1e3b45fe2f118a6ce35e37bbebf3
> Author: Juergen Spitzmueller 
> Date:   Mon Dec 8 09:06:41 2014 +0100
> 
> Add ObsoletedBy tag to InsetLayout
> 
> Fixes: #9000.

Jürgen are you able to take a look?

Scott

signature.asc
Description: PGP signature

Re: Regression in saving of flex insets in beta2?


Le 11/04/2016 17:45, Guillaume Munch a écrit :

Le 11/04/2016 12:36, Jean-Marc Lasgouttes a écrit :

Le 11/04/2016 07:27, Andrew Parsloe a écrit :

Suppose a module defines flex insets moo and baa (the locale-dependent
version of foo and bah in New Zealand), and these are used in a document
to which the module has been added. If the module is now deleted from
the document, the flex insets become undefined. If the document is saved
then closed with undefined insets, the saved file will contain lines
like

\begin_inset Flex undefined


Please open a new ticket. I think it is a consequence of

commit cfeddb9293bb1e3b45fe2f118a6ce35e37bbebf3



Which is confirmed by git bisect.

If I understand correctly, one can now lose data just by having a module
not properly installed.


Exactly. It should not be difficult to solve, but I am not sure of what 
the best solution is.


JMarc

Re: Regression in saving of flex insets in beta2?


Le 11/04/2016 12:36, Jean-Marc Lasgouttes a écrit :

Le 11/04/2016 07:27, Andrew Parsloe a écrit :

Suppose a module defines flex insets moo and baa (the locale-dependent
version of foo and bah in New Zealand), and these are used in a document
to which the module has been added. If the module is now deleted from
the document, the flex insets become undefined. If the document is saved
then closed with undefined insets, the saved file will contain lines like

\begin_inset Flex undefined


Please open a new ticket. I think it is a consequence of

commit cfeddb9293bb1e3b45fe2f118a6ce35e37bbebf3



Which is confirmed by git bisect.

If I understand correctly, one can now lose data just by having a module
not properly installed.

Re: Regression in saving of flex insets in beta2?


Le 11/04/2016 07:27, Andrew Parsloe a écrit :

Suppose a module defines flex insets moo and baa (the locale-dependent
version of foo and bah in New Zealand), and these are used in a document
to which the module has been added. If the module is now deleted from
the document, the flex insets become undefined. If the document is saved
then closed with undefined insets, the saved file will contain lines like

\begin_inset Flex undefined


Please open a new ticket. I think it is a consequence of

commit cfeddb9293bb1e3b45fe2f118a6ce35e37bbebf3
Author: Juergen Spitzmueller 
Date:   Mon Dec 8 09:06:41 2014 +0100

Add ObsoletedBy tag to InsetLayout

Fixes: #9000.


JMarc

Regression in saving of flex insets in beta2?

2016-04-10 Thread Andrew Parsloe

Suppose a module defines flex insets moo and baa (the locale-dependent 
version of foo and bah in New Zealand), and these are used in a document 
to which the module has been added. If the module is now deleted from 
the document, the flex insets become undefined. If the document is saved 
then closed with undefined insets, the saved file will contain lines like


\begin_inset Flex undefined

and be unable to restore the proper inset if the module is again added 
to the document.  This happens in beta2 on my system but not in 2.1.4 
where the corresponding line would be


\begin_inset Flex moo

(or \begin_inset Flex baa) so that adding the module again later 
recovers the fully functional insets. In beta2 it doesn't and there is 
no way of telling whether "Flex undefined" should be "Flex moo" or "Flex 
baa".


This could make things difficult if a LyX document requiring a custom 
module is shared. The recipient first has to open the document, read the 
instructions ("Please add the module"), follow them, then reconfigure. 
Perhaps they've tapped a key in the meantime. If they tend to 
reconfigure with an empty LyX window (which isn't necessary but I 
certainly do this), then it's quite possible their fingers will do 
Ctrl-S, Ctrl-W to close the document which will thus contain 
"\begin_inset Flex undefined" lines, rendering the module and insets 
useless.


This is on a recent clean reinstall of beta2 on windows 7. I've attached 
a MWE beta2 document using the logical markup module. Open the document, 
delete the module, then save and close the document. Now reopen it and 
add the logical markup module again. The character styles are not 
recognized. In 2.1.4, with a similar document and process, they are.


Andrew


---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus
#LyX 2.2 created this file. For more info see http://www.lyx.org/
\lyxformat 506
\begin_document
\begin_header
\save_transient_properties true
\origin unavailable
\textclass article
\use_default_options true
\begin_modules
logicalmkup
\end_modules
\maintain_unincluded_children false
\language english
\language_package default
\inputencoding auto
\fontencoding global
\font_roman "default" "default"
\font_sans "default" "default"
\font_typewriter "default" "default"
\font_math "auto" "auto"
\font_default_family default
\use_non_tex_fonts false
\font_sc false
\font_osf false
\font_sf_scale 100 100
\font_tt_scale 100 100
\graphics default
\default_output_format default
\output_sync 0
\bibtex_command default
\index_command default
\paperfontsize default
\spacing single
\use_hyperref false
\papersize default
\use_geometry false
\use_package amsmath 1
\use_package amssymb 1
\use_package cancel 1
\use_package esint 1
\use_package mathdots 1
\use_package mathtools 1
\use_package mhchem 1
\use_package stackrel 1
\use_package stmaryrd 1
\use_package undertilde 1
\cite_engine basic
\cite_engine_type default
\biblio_style plain
\use_bibtopic false
\use_indices false
\paperorientation portrait
\suppress_date false
\justification true
\use_refstyle 1
\index Index
\shortcut idx
\color #008000
\end_index
\secnumdepth 3
\tocdepth 3
\paragraph_separation indent
\paragraph_indentation default
\quotes_language english
\papercolumns 1
\papersides 1
\paperpagestyle default
\tracking_changes false
\output_changes false
\html_math_output 0
\html_css_as_file 0
\html_be_strict false
\end_header

\begin_body

\begin_layout Standard
A 
\begin_inset Flex Emph
status open

\begin_layout Plain Layout
quick
\end_layout

\end_inset

 
\begin_inset Flex Strong
status open

\begin_layout Plain Layout
brown
\end_layout

\end_inset

 
\begin_inset Flex Noun
status open

\begin_layout Plain Layout
fox
\end_layout

\end_inset

.
\end_layout

\end_body
\end_document

Re: Hyphens in LyX-Code in beta2

2016-02-26 Thread Jean-Marc Lasgouttes


Le 25/02/2016 21:53, Georg Baum a écrit :

Jean-Marc, thanks for looking at the issue, you have my +1 for the patch
posted at 
http://www.lyx.org/trac/attachment/ticket/9987/0001-Do-not-merge-consecutive-hyphens-in-LyX-Code.patch.


Thanks Georg. It is in now.

JMarc

Re: Hyphens in LyX-Code in beta2

2016-02-25 Thread Georg Baum

Andrew Parsloe wrote:

> I have created a ticket #9987.

Thanks for the report Andrew. This was indeed not wanted and an oversight 
from my side.

Jean-Marc, thanks for looking at the issue, you have my +1 for the patch 
posted at 
http://www.lyx.org/trac/attachment/ticket/9987/0001-Do-not-merge-consecutive-hyphens-in-LyX-Code.patch.


Georg

Re: Hyphens in LyX-Code in beta2

2016-02-24 Thread Andrew Parsloe




On 25/02/2016 4:39 p.m., Pavel Sanda wrote:

Andrew Parsloe wrote:

In 2.1.4 it is possible to type a number of hyphens in sequence in
LyX-Code. Thus, discussing command lines, you can type "--help" in LyX-Code
and the two hyphens are *not* converted to an en dash. This behaviour has
been lost in beta2. The two hyphens are automatically converted to an en
dash (and three to an em dash). This defeats the purpose of LyX-Code (as I
understand it). I can't see anything in "New in LyX 2.2" to suggest this is
intended.  I suspect a bug.

Georg was refactoring the way we treat dashes, I guess this slipped in
but does not look right indeed.
Pavel


I have created a ticket #9987.

Andrew

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

Re: Hyphens in LyX-Code in beta2

2016-02-24 Thread Pavel Sanda

Andrew Parsloe wrote:
> In 2.1.4 it is possible to type a number of hyphens in sequence in 
> LyX-Code. Thus, discussing command lines, you can type "--help" in LyX-Code 
> and the two hyphens are *not* converted to an en dash. This behaviour has 
> been lost in beta2. The two hyphens are automatically converted to an en 
> dash (and three to an em dash). This defeats the purpose of LyX-Code (as I 
> understand it). I can't see anything in "New in LyX 2.2" to suggest this is 
> intended.  I suspect a bug.

Georg was refactoring the way we treat dashes, I guess this slipped in
but does not look right indeed. 
Pavel

Hyphens in LyX-Code in beta2

2016-02-24 Thread Andrew Parsloe

In 2.1.4 it is possible to type a number of hyphens in sequence in 
LyX-Code. Thus, discussing command lines, you can type "--help" in 
LyX-Code and the two hyphens are *not* converted to an en dash. This 
behaviour has been lost in beta2. The two hyphens are automatically 
converted to an en dash (and three to an em dash). This defeats the 
purpose of LyX-Code (as I understand it). I can't see anything in "New 
in LyX 2.2" to suggest this is intended.  I suspect a bug.


Andrew

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

Re: beta2?

2016-02-22 Thread Peter Kümmel

Am 21.02.2016 um 17:59 schrieb Scott Kostyshak:

On Mon, Feb 15, 2016 at 05:06:19PM +0100, Kornel Benko wrote:

Am Sonntag, 14. Februar 2016 um 23:39:00, schrieb Pavel Sanda

Peter Kümmel wrote:

Uwe still gets the following errors from the tar I sent to him:

D:\LyXGit\LyX22beta3\3rdparty\boost\boost/numeric/conversion/detail/numeric_cast_traits.hpp(12):
fatal error C1083: File (Include) cannot be opened:
"boost/numeric/conversion/detail/preprocessed/numeric_cast_traits_common.hpp":
No such file or directory
[D:\LyXGit\LyX22beta3\compile-2010\src\support\support.vcxproj]

I only tested with "build5-2010-installer.bat"

Is this script now used to build official installer?
In case of yes, would be our cmake gurus able to add routine
which spits out md5sum of installer binaries which could Uwe
posts in his email when publishing the binaries?

Pavel

For me it is not clear, how the binary zip-file is created on windows.
If I would know, I could create a new target to compute md5sum for this
zip-file.
I created and committed a cmake-script to be used for this task.

It would be really nice to get this working. Peter, do you have an idea
of how to accomplish this?

AFAIK the installer is build in two steps,
1. build LyX and install locally
2. point the install file creater (NSIS) to the local LyX installation and
start NSIS manually

So I don't know were we can calculate any sensible md5sum with cmake.

Peter

Note that this just came up on the news today:
http://blog.linuxmint.com/?p=2994

Scott

Re: beta2?

2016-02-21 Thread Scott Kostyshak

On Mon, Feb 15, 2016 at 05:06:19PM +0100, Kornel Benko wrote:
> Am Sonntag, 14. Februar 2016 um 23:39:00, schrieb Pavel Sanda 
> > Peter Kümmel wrote:
> >  Uwe still gets the following errors from the tar I sent to him:
> > 
> >  D:\LyXGit\LyX22beta3\3rdparty\boost\boost/numeric/conversion/detail/numeric_cast_traits.hpp(12):
> >  fatal error C1083: File (Include) cannot be opened:
> >  "boost/numeric/conversion/detail/preprocessed/numeric_cast_traits_common.hpp":
> >  No such file or directory
> >  [D:\LyXGit\LyX22beta3\compile-2010\src\support\support.vcxproj]
> > >
> > > I only tested with "build5-2010-installer.bat"
> > 
> > Is this script now used to build official installer?
> > In case of yes, would be our cmake gurus able to add routine
> > which spits out md5sum of installer binaries which could Uwe
> > posts in his email when publishing the binaries?
> > 
> > Pavel
> 
> For me it is not clear, how the binary zip-file is created on windows.
> If I would know, I could create a new target to compute md5sum for this 
> zip-file.
> I created and committed a cmake-script to be used for this task.

It would be really nice to get this working. Peter, do you have an idea
of how to accomplish this?
Note that this just came up on the news today:
http://blog.linuxmint.com/?p=2994

Scott


signature.asc
Description: PGP signature

Re: beta2?

2016-02-16 Thread Jean-Marc Lasgouttes


Le 15/02/2016 21:20, Uwe Stöhr a écrit :

Am 15.02.2016 um 11:18 schrieb Jean-Marc Lasgouttes:


BTW, why don't you use lzma for the compression of the installer?


I do. Otherwise the installer for the betas would be 69 MB large.


Does it mean that it is the
  SetCompressor /SOLID lzma
in include/declarations.nsh.cmake (but not in include/declarations.nsh, 
why?) that sets compression?


In this case the explicit setting (active and commented out) in the 3 
nsi files should be removed for clarity.


JMarc

Re: beta2?

2016-02-15 Thread Uwe Stöhr


Am 15.02.2016 um 11:18 schrieb Jean-Marc Lasgouttes:


BTW, why don't you use lzma for the compression of the installer?


I do. Otherwise the installer for the betas would be 69 MB large.

regards Uwe

Re: beta2?

2016-02-15 Thread Kornel Benko

Am Sonntag, 14. Februar 2016 um 23:39:00, schrieb Pavel Sanda 
> Peter Kümmel wrote:
>  Uwe still gets the following errors from the tar I sent to him:
> 
>  D:\LyXGit\LyX22beta3\3rdparty\boost\boost/numeric/conversion/detail/numeric_cast_traits.hpp(12):
>  fatal error C1083: File (Include) cannot be opened:
>  "boost/numeric/conversion/detail/preprocessed/numeric_cast_traits_common.hpp":
>  No such file or directory
>  [D:\LyXGit\LyX22beta3\compile-2010\src\support\support.vcxproj]
> >
> > I only tested with "build5-2010-installer.bat"
> 
> Is this script now used to build official installer?
> In case of yes, would be our cmake gurus able to add routine
> which spits out md5sum of installer binaries which could Uwe
> posts in his email when publishing the binaries?
> 
> Pavel

For me it is not clear, how the binary zip-file is created on windows.
If I would know, I could create a new target to compute md5sum for this 
zip-file.
I created and committed a cmake-script to be used for this task.

Kornel

signature.asc
Description: This is a digitally signed message part.

Re: beta2?

2016-02-15 Thread Jean-Marc Lasgouttes


Le 15/02/2016 11:18, Jean-Marc Lasgouttes a écrit :

BTW, why don't you use lzma for the compression of the installer? This
is potentially a big win for the end user. Or did it turn out to be
disappointing?


Now I see that you use it only for the bundle installer. I would have 
thought that the gain is not so high there, since it probably contains 
lots of stuff that is already compressed.


JMarc

Re: beta2?

2016-02-15 Thread Jean-Marc Lasgouttes


Le 15/02/2016 00:55, Uwe Stöhr a écrit :

Am 14.02.2016 um 19:44 schrieb Peter Kümmel:


So the unzip tool has a problem, maybe because of the long path length.

Maybe we should also distribute a zip file.


Yes, a zip would be fine. I don't think that we need to save a few kB by
using special formats like .xz.


As Pavel pointed out, this is an interesting understatement. But indeed 
the size of the source distribution is not vital on binary-loving 
platforms like windows.


BTW, why don't you use lzma for the compression of the installer? This 
is potentially a big win for the end user. Or did it turn out to be 
disappointing?


JMarc

Re: beta2?

2016-02-14 Thread Pavel Sanda

Peter Kümmel wrote:
 Uwe still gets the following errors from the tar I sent to him:

 D:\LyXGit\LyX22beta3\3rdparty\boost\boost/numeric/conversion/detail/numeric_cast_traits.hpp(12):
 fatal error C1083: File (Include) cannot be opened:
 "boost/numeric/conversion/detail/preprocessed/numeric_cast_traits_common.hpp":
 No such file or directory
 [D:\LyXGit\LyX22beta3\compile-2010\src\support\support.vcxproj]
>
> I only tested with "build5-2010-installer.bat"

Is this script now used to build official installer?
In case of yes, would be our cmake gurus able to add routine
which spits out md5sum of installer binaries which could Uwe
posts in his email when publishing the binaries?

Pavel

Re: beta2?

2016-02-14 Thread Pavel Sanda

Uwe Stöhr wrote:
> Yes, a zip would be fine. I don't think that we need to save a few kB by 
> using special formats like .xz.

For the record .xz is pretty standard in open source releases and 
it saves 9mb from 20mb gzips (and 21mb from 31 .zip I just experimentally 
tried).
This is not few kb.

But this is most probably offtopic anyway, because the problem is with tar, not 
zip.
I seem to remember that we fixed this problem by deleting boost files which were
not by acident needed, the current approach looks much better.
Pavel

Re: beta2?

2016-02-14 Thread Scott Kostyshak

On Mon, Feb 15, 2016 at 12:55:47AM +0100, Uwe Stöhr wrote:
> Am 14.02.2016 um 19:44 schrieb Peter Kümmel:
> 
> >So the unzip tool has a problem, maybe because of the long path length.
> >
> >Maybe we should also distribute a zip file.
> 
> Yes, a zip would be fine. I don't think that we need to save a few kB by
> using special formats like .xz.
> 
> Scott, could you please upload a zip file so that I can see if this fixes te
> problems.

OK I sent it you.

Scott


signature.asc
Description: PGP signature

Re: beta2?

2016-02-14 Thread Uwe Stöhr


Am 14.02.2016 um 19:44 schrieb Peter Kümmel:


So the unzip tool has a problem, maybe because of the long path length.

Maybe we should also distribute a zip file.


Yes, a zip would be fine. I don't think that we need to save a few kB by 
using special formats like .xz.


Scott, could you please upload a zip file so that I can see if this 
fixes te problems.


thanks and regards
Uwe

Re: beta2?

2016-02-14 Thread Jean-Marc Lasgouttes


Le 14/02/16 19:54, Georg Baum a écrit :

Peter Kümmel wrote:


Am 14.02.2016 um 19:03 schrieb Scott Kostyshak:


Why is numeric_cast_traits_common.hpp not being found?


Have you checked if it's part of the tar you send to Uwe?


The files I listed above are part of the tar. I will send the tar to you
privately.


That explains why they did not show up in my comparison of the beta1 tar and
the git tree. I did not really understand why I could have missed them.


Probably related to this commit:

commit 5287f1c8b6a9e719492cf7350c304efb978c664c
Author: Peter Kümmel 
Date:   Sun Dec 20 13:32:33 2015 +0100

default tar-v7 format supports only 99 character pathes


We use tar-pax now, and it might not be handled very well by some old 
utilities. From what is said here


https://www.gnu.org/software/automake/manual/html_node/List-of-Automake-options.html
I believe that "ustar" would have been a better alternative.

JMarc

Re: beta2?




Am 14.02.2016 um 21:27 schrieb Jean-Marc Lasgouttes:

Le 14/02/16 19:54, Georg Baum a écrit :

Peter Kümmel wrote:


Am 14.02.2016 um 19:03 schrieb Scott Kostyshak:


Why is numeric_cast_traits_common.hpp not being found?


Have you checked if it's part of the tar you send to Uwe?


The files I listed above are part of the tar. I will send the tar to you
privately.


That explains why they did not show up in my comparison of the beta1 tar and
the git tree. I did not really understand why I could have missed them.


Probably related to this commit:

commit 5287f1c8b6a9e719492cf7350c304efb978c664c
Author: Peter Kümmel 
Date:   Sun Dec 20 13:32:33 2015 +0100

 default tar-v7 format supports only 99 character pathes


We use tar-pax now, and it might not be handled very well by some old 
utilities. From what is said here

https://www.gnu.org/software/automake/manual/html_node/List-of-Automake-options.html
I believe that "ustar" would have been a better alternative.


Thanks for this hint!
I've tested it with ustar and it is indeed the better choice.

I've committed the change to ustar.

Peter



JMarc

Re: beta2?

2016-02-14 Thread Georg Baum

Peter Kümmel wrote:

> Am 14.02.2016 um 19:03 schrieb Scott Kostyshak:
>>>>
>>>> Why is numeric_cast_traits_common.hpp not being found?
>>>
>>> Have you checked if it's part of the tar you send to Uwe?
>>
>> The files I listed above are part of the tar. I will send the tar to you
>> privately.

That explains why they did not show up in my comparison of the beta1 tar and 
the git tree. I did not really understand why I could have missed them.

> When I unpack with WinRAR it looks like this
> 
> $ ls -l 3rdparty/boost/boost/numeric/conversion/detail/preprocessed/
> total 56
> -rw-r--r-- 1 numeric_cast_traits_commo
> -rw-r--r-- 1 numeric_cast_traits_long_
> 
> and when I run "tar vxf lyx-2.2.0beta3.tar.gz" from the git console it is
> ok.
> 
> So the unzip tool has a problem, maybe because of the long path length.

Probably. The good news is that we do not need a beta3, simply unpacking 
beta2 with a different method should be OK.

Georg

Re: beta2?

Am 14.02.2016 um 19:03 schrieb Scott Kostyshak:

On Sun, Feb 14, 2016 at 06:36:15PM +0100, Peter Kümmel wrote:

Am 14.02.2016 um 17:17 schrieb Scott Kostyshak:

On Sat, Feb 13, 2016 at 01:44:30PM -0500, Scott Kostyshak wrote:

On Sat, Feb 13, 2016 at 01:24:20PM -0500, Scott Kostyshak wrote:

On Sat, Feb 13, 2016 at 07:11:48PM +0100, Uwe Stöhr wrote:

Am 13.02.2016 um 18:01 schrieb Scott Kostyshak:

Thanks for testing, Peter. I'll wait for a fix, and then I suppose we
move to beta3?

Sorry, but why do we act like beginners?

I wrote what is missing in the tarball. So why not add these files, send me
the tarball by private mail to check and then announce a new beta

You are right that this is what I should have done for beta2. I will do
this now before releasing beta3. I'll build the tar and send it to you
privately. After you confirm that you can compile the Windows build from
only the tar, I will post the tar and push the git tag and git commits.

I've sent the tars to you privately. Can you do a fresh build from them?

Uwe still gets the following errors from the tar I sent to him:

I only tested with "build5-2010-installer.bat"

I was hoping that 6de524e6 fixed these. Is there still something we
need to do? Looking in the tar, I see

./3rdparty/boost/boost/numeric/conversion/detail/preprocessed/numeric_cast_traits_long_long.hpp
./3rdparty/boost/boost/numeric/conversion/detail/preprocessed/numeric_cast_traits_common.hpp
./3rdparty/boost/boost/numeric/conversion/detail/numeric_cast_traits.hpp
./3rdparty/boost/boost/numeric/conversion/numeric_cast_traits.hpp

Why is numeric_cast_traits_common.hpp not being found?

Have you checked if it's part of the tar you send to Uwe?

The files I listed above are part of the tar. I will send the tar to you
privately.

Scott

Re: beta2?


Am 14.02.2016 um 19:03 schrieb Scott Kostyshak:


Why is numeric_cast_traits_common.hpp not being found?


Have you checked if it's part of the tar you send to Uwe?


The files I listed above are part of the tar. I will send the tar to you
privately.


When I unpack with WinRAR it looks like this

$ ls -l 3rdparty/boost/boost/numeric/conversion/detail/preprocessed/
total 56
-rw-r--r-- 1 numeric_cast_traits_commo
-rw-r--r-- 1 numeric_cast_traits_long_

and when I run "tar vxf lyx-2.2.0beta3.tar.gz" from the git console it is ok.

So the unzip tool has a problem, maybe because of the long path length.

Maybe we should also distribute a zip file.



Scott

Re: beta2?

2016-02-14 Thread Scott Kostyshak

On Sat, Feb 13, 2016 at 01:44:30PM -0500, Scott Kostyshak wrote:
> On Sat, Feb 13, 2016 at 01:24:20PM -0500, Scott Kostyshak wrote:
> > On Sat, Feb 13, 2016 at 07:11:48PM +0100, Uwe Stöhr wrote:
> > > Am 13.02.2016 um 18:01 schrieb Scott Kostyshak:
> > > 
> > > >Thanks for testing, Peter. I'll wait for a fix, and then I suppose we
> > > >move to beta3?
> > > 
> > > Sorry, but why do we act like beginners?
> > > 
> > > I wrote what is missing in the tarball. So why not add these files, send 
> > > me
> > > the tarball by private mail to check and then announce a new beta
> > 
> > You are right that this is what I should have done for beta2. I will do
> > this now before releasing beta3. I'll build the tar and send it to you
> > privately. After you confirm that you can compile the Windows build from
> > only the tar, I will post the tar and push the git tag and git commits.
> 
> I've sent the tars to you privately. Can you do a fresh build from them?

Uwe still gets the following errors from the tar I sent to him:

D:\LyXGit\LyX22beta3\3rdparty\boost\boost/numeric/conversion/detail/numeric_cast_traits.hpp(12):
fatal error C1083: File (Include) cannot be opened:
"boost/numeric/conversion/detail/preprocessed/numeric_cast_traits_common.hpp":
No such file or directory
[D:\LyXGit\LyX22beta3\compile-2010\src\support\support.vcxproj]

I was hoping that 6de524e6 fixed these. Is there still something we
need to do? Looking in the tar, I see

./3rdparty/boost/boost/numeric/conversion/detail/preprocessed/numeric_cast_traits_long_long.hpp
./3rdparty/boost/boost/numeric/conversion/detail/preprocessed/numeric_cast_traits_common.hpp
./3rdparty/boost/boost/numeric/conversion/detail/numeric_cast_traits.hpp
./3rdparty/boost/boost/numeric/conversion/numeric_cast_traits.hpp

Why is numeric_cast_traits_common.hpp not being found?

Scott


signature.asc
Description: PGP signature

Re: beta2?