Re: [patch] Experiment with c++11 unicode strings

[Guillaume Munch]

Le 30/08/2016 à 21:12, Georg Baum a écrit :

Guillaume Munch wrote:



* Why ascii_num_get_facet::do_get uses from_local8bit at some
point.


The encoding does not matter here: Our own numpunct_facet does not
override truename() and falsename(), so std::numpunct::truename() and
std::numpunct::falsename() are used. I don't know why Enrico chose
from_local8bit and not from_ascii() here.


* Is the numpunct facet correctly set with the way
 it occurs in the code?


I think so. Otherwise we would have the classical separator problem,
 e.g. outputting 2.3 as "2,3" when using a german locale. Do you see
 any specific problem?


The custom facets circumvent using std::numpunct, e.g.
ascii_num_get_facet converts from std::numpunct (which by the way
is always the C locale so one could just use from_ascii("true"), etc.).
This is the only place where there is a numpunct so I imagine that this
is the one your are referring to with "our own numpunct".

Still, we cannot assume that std::numpunct is available. So
either someone is confident that nothing else requires it, or it should
be implemented like the other three facets.




I do not trust versa_string, e.g. bug
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67557


You are right, it is more risky than I thought to use versa_string.

Thanks also for the explanations about iconv.



There is an additional conversion from char * to std::string. I
was once told that this is less efficient (here on the LyX
mailing list, but I forgot who wrote that), but I never tested
that myself.


Surely, but the combined amount of time and energy spent in this
conversion, over all user's computers present and future, is surely
less than the time and energy spent writing this reply...


Well, having these changes combined with the string type changes in
one patch causes all interested persons to spend more time. If you do
not want comments on the empty_string changes then please do such
changes in a separate patch next time (the string type changes can be
done independently of removing empty_string() and empty_docstring().


Don't get me wrong, I was not implying that the discussion is a loss of
time. That would not be a clever way to try to make the point that the
difference is negligible ;)




Unfortunately I cannot try the patch, since I get undefined refernces
to the std::codecvt functions that we previously defined for gcc on
windows:


Unlike std::codecvt, std::codecvt is part of
the spec so it's quite a different situation... Does it match with
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66030#c7 ?

Re: [patch] Experiment with c++11 unicode strings

[Georg Baum]

Guillaume Munch wrote:

> I made it work with libc++ too, which was not straightforward. In this
> case, the template is undefined and cannot be inherited from.
> 
> Could you or somebody else please double-check the following, which I am
> not sure to understand correctly:
> 
> * Why ascii_num_get_facet::do_get uses from_local8bit at some point.

The encoding does not matter here: Our own numpunct_facet does not override 
truename() and falsename(), so std::numpunct::truename() and 
std::numpunct::falsename() are used. I don't know why Enrico chose 
from_local8bit and not from_ascii() here.

> * Is the numpunct facet correctly set with the way it
> occurs in the code?

I think so. Otherwise we would have the classical separator problem, e.g. 
outputting 2.3 as "2,3" when using a german locale. Do you see any specific 
problem?

>> I would like it more simple as well, but it seems this is not easily
>> possible without a C++11 compliant std::string.
> 
> I agree for std::string, but for docstring I have demonstrated that it
> is very simple. Moreover I find it reassuring that versa_string is the
> precursor of the new basic_string.

If you look at docstring alone I agree, but if you look at LyX as a whole 
this would mean to use 3 string base classes (std::basic_string, 
ext::versa_string and lyx::trivial_string) instead of two (std::basic_string 
and lyx::trivial_string). This is more complicated IMHO.
Besides that I do not trust versa_string, e.g. bug 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67557 is likely to exist in gcc 
older than 5.1. If trivstring had a (yet unknown) bug this would be 
annoying, but its usage is mostly limited to work done in threads (e.g. 
export), so data loss is not that likely. If we based our central workhorse 
docstring on a class which we do not know how widely it is used and what 
bugs might exist then I would be nervous.

> There is also a built-in conversion function between utf-8 and utf-32 in
> C++11 which could replace iconv. But I don't know whether there are
> clear advantages in replacing iconv. Iconv might still be needed for
> other encodings.

We definitely need other conversions. Even if we decided to drop support for 
non-utf8 LaTeX output (which we might do one day) we would still need it for 
importing .tex files.

> But then I don't understand why for from_local8bit it
> uses QString::fromLocal8Bit instead of iconv.

Because it is easier. If you want to use iconv, you need to know the name of 
the local encoding, and determining it is platform dependent. With qt all 
this is nicely abstracted away.

 The only thing I would do differently is the replacement of
 empty_string() and empty_docstring(): Those were introduced to avoid
 including . Since  is now required, it would be better
 to initialize the default arguments with docstring() or std::string().
>>>
>>> There is not much a difference from what I did, other than matters of
>>> taste, is there?
>>
>> There is an additional conversion from char * to std::string. I was once
>> told that this is less efficient (here on the LyX mailing list, but I
>> forgot who wrote that), but I never tested that myself.
> 
> Surely, but the combined amount of time and energy spent in this
> conversion, over all user's computers present and future, is surely less
> than the time and energy spent writing this reply...

Well, having these changes combined with the string type changes in one 
patch causes all interested persons to spend more time. If you do not want 
comments on the empty_string changes then please do such changes in a 
separate patch next time (the string type changes can be done independently 
of removing empty_string() and empty_docstring().


Unfortunately I cannot try the patch, since I get undefined refernces to the 
std::codecvt functions that we previously defined for gcc on windows:

g++ -fPIC -O2 -std=c++14  -std=c++14  -fmessage-length=0 -g -Wunused-result 
-Wuninitialized -Winit-self   -o lyx main.o   BiblioInfo.o Box.o Compare.o 
Dimension.o EnchantChecker.o  PersonalWordList.o LaTeXFonts.o 
PrinterParams.o Thesaurus.o  liblyxcore.a liblyxmathed.a liblyxinsets.a 
frontends/liblyxfrontends.a frontends/qt4/liblyxqt4.a liblyxgraphics.a 
support/liblyxsupport.a  -lmythes-1.2 -lenchant   -lmagic  -lz -
lQt5Concurrent -lQt5Svg -lQt5Widgets -lQt5X11Extras -lQt5Gui -lQt5Core -lxcb  
support/liblyxsupport.a(docstream.o):
(.data.rel.ro._ZTVSt7codecvtIDic11__mbstate_tE[_ZTVSt7codecvtIDic11__mbstate_tE]+0x20):
 
undefined reference to `std::codecvt::do_out(__mbstate_t&, char32_t const*, char32_t const*, 
char32_t const*&, char*, char*, char*&) const'
support/liblyxsupport.a(docstream.o):
(.data.rel.ro._ZTVSt7codecvtIDic11__mbstate_tE[_ZTVSt7codecvtIDic11__mbstate_tE]+0x28):
 
undefined reference to `std::codecvt::do_unshift(__mbstate_t&, char*, char*, char*&) const'
support/liblyxsupport.a(docstream.o):

Re: [LyX/master] Add feedback in status bar when zooming

[Scott Kostyshak]

On Tue, Aug 30, 2016 at 05:03:11PM +0200, Jean-Marc Lasgouttes wrote:
> Le 30/08/2016 à 16:55, Scott Kostyshak a écrit :
> > OK I pushed at 03684ae0. I wonder if the misunderstanding comes back to
> > what we agreed on before, which is that it is weird that both
> > 
> >   buffer-zoom-in 4
> >   buffer-zoom-out 4
> > 
> > have the same behavior.
> 
> No, it probably come from my laziness about reading your patch :)

Well it is rational. This trivial issue is not worth much time. Thank
you for the time you spent on it.

> > Perhaps we should change this so that
> > 
> >   buffer-zoom-in 4
> >   buffer-zoom-out -4
> > 
> > have the same behavior instead. But I don't feel like working on that
> > now. And the patch I committed at least makes things consistent given
> > the current behavior.
> 
> Yes the arguments should be changed one day, but I suspect that nobody uses
> the lfuns with an argument anyway...

I would say I'd put it on my TODO list but I already have enough things
on there I probably won't do.

Scott


signature.asc
Description: PGP signature

Re: [LyX/master] Add feedback in status bar when zooming

[Jean-Marc Lasgouttes]

Le 30/08/2016 à 16:55, Scott Kostyshak a écrit :

OK I pushed at 03684ae0. I wonder if the misunderstanding comes back to
what we agreed on before, which is that it is weird that both

  buffer-zoom-in 4
  buffer-zoom-out 4

have the same behavior.


No, it probably come from my laziness about reading your patch :)


Perhaps we should change this so that

  buffer-zoom-in 4
  buffer-zoom-out -4

have the same behavior instead. But I don't feel like working on that
now. And the patch I committed at least makes things consistent given
the current behavior.


Yes the arguments should be changed one day, but I suspect that nobody 
uses the lfuns with an argument anyway...


JMarc

Re: [LyX/master] Add feedback in status bar when zooming

[Scott Kostyshak]

On Tue, Aug 30, 2016 at 04:41:38PM +0200, Jean-Marc Lasgouttes wrote:
> Le 30/08/2016 à 16:36, Scott Kostyshak a écrit :
> > It would go down to 10%, both with my patch and without it.
> 
> It is OK, then.
> 
> Do what you think is best.

OK I pushed at 03684ae0. I wonder if the misunderstanding comes back to
what we agreed on before, which is that it is weird that both

  buffer-zoom-in 4
  buffer-zoom-out 4

have the same behavior.

Perhaps we should change this so that

  buffer-zoom-in 4
  buffer-zoom-out -4

have the same behavior instead. But I don't feel like working on that
now. And the patch I committed at least makes things consistent given
the current behavior.

Scott


signature.asc
Description: PGP signature

Re: [LyX/master] Add feedback in status bar when zooming

[Scott Kostyshak]

On Tue, Aug 30, 2016 at 10:51:13AM +0200, Jean-Marc Lasgouttes wrote:
> Le 30/08/2016 à 03:52, Scott Kostyshak a écrit :
> > On current master, if zoom is at 10% and you run
> > 
> >   buffer-zoom-in -1
> > 
> > then the LFUN is not disabled and status shows:
> > 
> >   Zoom level is now 10% (buffer-zoom-in -1)
> > 
> > With my patch, I think the LFUN is disabled and status shows the same as
> > 
> >   buffer-zoom-out -1
> > 
> > which is:
> > 
> >   Zoom level cannot be less than 10%.
> 
> But then if one is at 12,% "buffer-zoom-in -3" would keep the zoom at 12%?
> That's weird.

It would go down to 10%, both with my patch and without it.

Scott


signature.asc
Description: PGP signature

Re: Insert inset should not remove tracked changes

[racoon]

On 30.08.2016 14:58, Guillaume Munch wrote:

Le 30/08/2016 à 12:52, racoon a écrit :

Say you have a passage with a lot of tracked changes. Now, however, you
would like to "comment it out" with a LyX note. Surely, you don't want
to loose the changes made but keep them for later evaluation.


+1

http://www.lyx.org/trac/ticket/10244
http://www.lyx.org/trac/ticket/10128


Thanks. I don't even remember my own tickets... need a checkup :)

Daniel

Re: Insert inset should not remove tracked changes

[Guillaume Munch]

Le 30/08/2016 à 12:52, racoon a écrit :

Say you have a passage with a lot of tracked changes. Now, however, you
would like to "comment it out" with a LyX note. Surely, you don't want
to loose the changes made but keep them for later evaluation.


+1

http://www.lyx.org/trac/ticket/10244
http://www.lyx.org/trac/ticket/10128

Re: Insert inset should not remove tracked changes

[racoon]

On 30.08.2016 11:06, Kornel Benko wrote:

Am Dienstag, 30. August 2016 um 10:40:55, schrieb racoon 

Hi,

I am wondering whether this is on purpose or not:

1. Enable Track Changes
2. Enter some text
3. Disable Track Changes
4. Select the text
5. Insert an inset, e.g. a LyX note

Result:
- The tracked changes are removed

Isn't it more intuitive to leave the changes as they are? After all I am
just adding a non-change tracked inset "around" the text.

Daniel


I think that would be cheating. Track changes is for a co-author. Either there 
are all changes visible or none.


Not sure whether this is a misunderstanding. Not fully sure what this 
has to do with co-authors.


Here are some use cases that might make more clear why I am wondering:

Say you have a passage with a lot of tracked changes. Now, however, you 
would like to "comment it out" with a LyX note. Surely, you don't want 
to loose the changes made but keep them for later evaluation. Both with 
and without tracked changes enabled this is currently impossible.


Or take another case. You want to put the passage into a footnote. Same 
problem. No way to do this without loosing your trackings.


Now that I think about it, the tracked changes should actually be kept 
in both cases - with and without change tracking enabled. The difference 
should just be that the insert of the inset is also tracked or not, 
respectively.


Again, it seems intuitive that the insets are placed around the selected 
text. LyX might internally remove the selected text and insert it into 
the inset. But then the inserted text should keep the trackings (and all 
other features as far as possible) of the removed text IMO.


Daniel

Re: [LyX/master] Add feedback in status bar when zooming

[Jean-Marc Lasgouttes]

Le 30/08/2016 à 03:52, Scott Kostyshak a écrit :

On current master, if zoom is at 10% and you run

  buffer-zoom-in -1

then the LFUN is not disabled and status shows:

  Zoom level is now 10% (buffer-zoom-in -1)

With my patch, I think the LFUN is disabled and status shows the same as

  buffer-zoom-out -1

which is:

  Zoom level cannot be less than 10%.


But then if one is at 12,% "buffer-zoom-in -3" would keep the zoom at 
12%? That's weird.


You could keep the function enabled but improve the messages.

JMarc

Re: Insert inset should not remove tracked changes

[Kornel Benko]

Am Dienstag, 30. August 2016 um 10:40:55, schrieb racoon 
> Hi,
> 
> I am wondering whether this is on purpose or not:
> 
> 1. Enable Track Changes
> 2. Enter some text
> 3. Disable Track Changes
> 4. Select the text
> 5. Insert an inset, e.g. a LyX note
> 
> Result:
> - The tracked changes are removed
> 
> Isn't it more intuitive to leave the changes as they are? After all I am 
> just adding a non-change tracked inset "around" the text.
> 
> Daniel

I think that would be cheating. Track changes is for a co-author. Either there 
are all changes visible or none.

Kornel

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

Insert inset should not remove tracked changes

[racoon]

Hi,

I am wondering whether this is on purpose or not:

1. Enable Track Changes
2. Enter some text
3. Disable Track Changes
4. Select the text
5. Insert an inset, e.g. a LyX note

Result:
- The tracked changes are removed

Isn't it more intuitive to leave the changes as they are? After all I am 
just adding a non-change tracked inset "around" the text.


Daniel