Re: [LyX/master] Nonsense for whoever insists on using gcc4.6 & qt4.8 in 2017

2017-03-19 Thread Guillaume Munch

Le 11/03/2017 à 20:16, Pavel Sanda a écrit :

Guillaume Munch wrote:

On the other hand, for graphics, the
idea of the previous FileMonitor could be adapted to refresh graphics
when they appear on the screen


Perhaps. But it should do that when window gets active (we already catch that
signal for redrawing graphics).
Typical scenario I am in: plotting routine in terminal window changes figs
and I want to see them updated after I switch from terminal back to lyx.


It now checks for modifications using timestamp and checksum whenever
drawn (asynchronously) but not more than once every 10s.

I chose a big interval because it is only useful for cases where
QFileSystemWatcher does not work.

If QFileSystemWatcher works, you see the modification instantly. If it
does not, you now see it when you activate the window.





(but stop doing it when they leave the screen).


Of course, there is no reason for that.


Well, this is what the old code was doing.




In that case, I even wonder whether it is useful to use the file
monitor for graphics files.


It seems to me safer still use them, but don't have strong opinion.

BTW do you think it would be hard to paralelize the load of the figures
in the previewer machinery? I have lot of reports with hundreds of figs
and loading the document with all figs takes mins while only one of my
cores is busy.


I am not familiar enough with this part. Are they graphics or previews?
If graphics, does increasing s_numimages_ and decreasing s_millisecs_ in
graphics/GraphicsLoader.cpp help?


Guillaume



Re: Build failed in Jenkins: Build branch "master" » ubuntu-xenial-qt4-autotools-extended-parameterised #1

2017-03-19 Thread Guillaume Munch

Le 19/03/2017 à 13:17, ci-...@inria.fr a écrit :

Making distclean in .
make[3]: Entering directory '/build/workspace/lyx-2.3.0dev/_build/sub/src'
Makefile:2455: support/tests/.deps/check_ExternalTransforms-dummy_functions.Po: 
No such file or directory
Makefile:2456: support/tests/.deps/check_Length-dummy_functions.Po: No such 
file or directory
Makefile:2457: support/tests/.deps/check_ListingsCaption-dummy_functions.Po: No 
such file or directory
Makefile:2458: support/tests/.deps/check_layout-dummy_functions.Po: No such 
file or directory
make[3]: *** No rule to make target 
'support/tests/.deps/check_layout-dummy_functions.Po'.  Stop.


distclean has been broken here for some time, with the same messages.



Re: [PATCH] Output of en- and em-dashes

2017-03-18 Thread Guillaume Munch

Le 18/03/2017 à 14:59, Enrico Forestieri a écrit :

On Wed, Mar 15, 2017 at 06:12:51PM +0100, Enrico Forestieri wrote:


Apparently, nobody has a preference, so I am going to commit the
second patch, i.e., the one using a zero-width space character.


On second thoughts, I am not sure this is the best choice. I just
verified that this character is not searchable and only previewing
the latex source code can reveal its presence. So, it may be
inadvertently spread into a document by copy/paste.

I think we have to make do with the ugly zero-space inset.



Or special/invisible unicode characters could be made visible by 
changing the character before painting.




Re: cmake compilation error

2017-03-12 Thread Guillaume Munch

Le 12/03/2017 à 16:51, Kornel Benko a écrit :


Should we emit a warning (which probably will be ignored) or better refuse to 
build
with "5.0" <= QT < "5.4" ?


I don't think so, the indication INSTALL file should be enough in that case.



From my POV, if user selects QT5, we should use it.



Agreed. In addition, from my POV, it is better if Qt5 for big enough
values of 5 are selected as default, so that one does not have to select
it explicitly.




Re: cmake compilation error

2017-03-11 Thread Guillaume Munch

Le 11/03/2017 à 18:07, Jean-Pierre Chrétien a écrit :

Le 11/03/2017 à 17:57, Kornel Benko a écrit :


I see the same with installed QT5.2.1. But no problems with QT5.7.


Seems that I've got 5.3.2 here, Debian Jessie (up to date stable).


Indeed the function is introduced in qt5.4. It is now fixed. Note that
from what I remember, LyX suffers various bugs with early releases of
qt5, which is why the release notes strongly advises qt>=5.4. CMake
should select qt4.8 over qt5.3 when possible.

Guillaume



Re: [LyX/master] buffer-export without argument exports the default format

2017-03-11 Thread Guillaume Munch

Le 11/03/2017 à 08:37, Scott Kostyshak a écrit :


It looks like Guillaume made the change at 3dec5826.



Yes, and thanks for the feedback.




Re: [LyX/master] Nonsense for whoever insists on using gcc4.6 & qt4.8 in 2017

2017-03-11 Thread Guillaume Munch

Le 11/03/2017 à 08:34, Pavel Sanda a écrit :

Guillaume Munch wrote:

commit 24f68aff8d2ba9139017ca3927eda1f1aaf039af
Author: Guillaume Munch <g...@lyx.org>
Date:   Sat Mar 11 00:11:02 2017 +0100

Nonsense for whoever insists on using gcc4.6 & qt4.8 in 2017


Does it mean it is not supposed to work with those two or it should?


With this commit, it works with gcc4.6 and qt4.8, though it required
imaginative workarounds that I hope we can delete at some point. My only
worry is that QFileSystemWatcher has been rewritten in qt5, so it is
possible that it behaves differently with qt4.8.



I just quickly checked that the new monitor does not work on networked fs.
To reproduce: mount via sshfs to another machine and load lyx document
from there. After that change some image which was loaded within
the document. The new monitor machiner does not catch it anymore.

gcc 4.9.4 & qt 4.8.


Thanks for testing. This is a limitation of inotify. For the same reason
you will not be warned if the file is modified externally until you try
to save it. The same happens with gedit. MacOS and Windows use other
backends so it might be different (but at best they will only tell about
modifications made from the same machine).

The previous FileMonitor worked by querying the modification time every
two seconds per graphics, and computing the checksum if the timestamp
changed. This is expensive especially over the network, and there is a
reason why inotify does not do it. On the other hand, for graphics, the
idea of the previous FileMonitor could be adapted to refresh graphics
when they appear on the screen (but stop doing it when they leave the
screen). In that case, I even wonder whether it is useful to use the 
file monitor for graphics files.


Guillaume



Re: [LyX/master] buffer-export without argument exports the default format

2017-03-08 Thread Guillaume Munch

Le 08/03/2017 à 13:18, Kornel Benko a écrit :

Am Mittwoch, 8. März 2017 um 11:48:12, schrieb Guenter Milde 


Would you be opposed to using a different symbol that is not interpreted
by common shells?


I thought about it.



I prefer *, because

* it is established as wildcard symbol and even «'*'» is shorter than
  «default»


This was my reasoning. Already saw * used elsewhere for lfuns.


* it is clearly separable from a file name (in contrast to "default").

Günter


But we already use 'pdf2,dvi,...'. No difference to 'default' from my pov. The 
parameter
stays for format (not possible to mix with file name), so I do not see a 
problem.



I have no strong opinion. Just report to me if you end up agreeing on a 
solution, or feel free to change it yourself.


Thank you for the feedback.

Guillaume



Re: Ending selection on item inside a table erroneously interpreted as a "click", brings up dialog

2017-03-04 Thread Guillaume Munch

Le 03/03/2017 à 17:42, Helge Hafting a écrit :


I guess the same goes for anything that does something when clicked. If
it is inside a table cell and a mouse selection ends on top of it, a
(wrong) click action happens.



Hi Helge,

It's fixed at 354897f6.

Guillaume




Re: today's compilation errors and warnings

2017-03-04 Thread Guillaume Munch

Le 05/03/2017 à 03:27, Uwe Stöhr a écrit :

I compiled today's master and get an error:

xgettext: error while opening "src/frontends/qt4/LyXToolBox.cpp" for
reading: No such file or directory
C:\Program Files
(x86)\MSBuild\Microsoft.Cpp\v4.0\V140\Microsoft.CppCommon.targets(171,5): error
MSB6006: "cmd.exe" wurde mit dem Code 1 beendet.
[D:\LyXGit\Master\compile-2015\po\translations.vcxproj]


Hi Uwe,

git grep LyXToolBox does not return anything. Probably the build is not 
clean.




and a warning:

RowPainter.cpp
D:\LyXGit\Master\src\Row.cpp(116): warning C4244: 'initializing':
conversion from 'double' to 'int', possible loss of data
[D:\LyXGit\Master\compile-2015\src\LyX.vcxproj]
D:\LyXGit\Master\src\Row.cpp(116): warning C4244: 'initializing':
conversion from 'double' to 'const int', possible loss of data
[D:\LyXGit\Master\compile-2015\src\LyX.vcxproj]

Could anybody please have a look?



Thanks, fixed.


Guillaume




Re: [LyX/master] buffer-export without argument exports the default format

2017-03-04 Thread Guillaume Munch

Le 28/02/2017 à 04:50, Scott Kostyshak a écrit :

On Tue, Feb 28, 2017 at 12:47:12AM +0100, Guillaume Munch wrote:

commit a1faa41c839089a9f21323af6540926eae90ec6f
Author: Guillaume Munch <g...@lyx.org>
Date:   Mon Feb 27 20:43:11 2017 +0100

buffer-export without argument exports the default format

buffer-export is proposed as a default binding in the preferences so now it 
does
what a user expects when binding it to a key.


Thanks for this commit. What do you think of the attached patch that
extends it so that one can export to the default format from the command
line?

Do you agree that, because of how we pass arguments, there is no simple
way to get it to work with -E (see the FIXME in the patch)?

If you think the idea is good, and agree there is no simple way to get
it to work with -E, then I would just remove that part of the patch, and
would document in --help for -E that "default" does not work as
described for -e.

In any case, the user can easily use bash to extend -e to -E on their
own (i.e., they can rename the output file).



I am not sure to understand since I am not familiar with this part of 
LyX. But I will push a patch that makes "buffer-export *" synonymous to 
buffer-export so that your patch becomes simpler.


Guillaume



Re: How far is 2.3.0?

2017-03-04 Thread Guillaume Munch

Le 24/02/2017 à 18:45, Jean-Marc Lasgouttes a écrit :

Hi there,

A good subject before the week-end: what do we need to do before
declaring feature freeze for 2.3? Personally I have finished a cycle or
breaking/fixing the math editor and would like to refrain from doing new
big architectural changes. I only have two or three patches pending,
plus of course all the bugs that Guillaume keeps finding.

Who has Work In Progress that should go to 2.3? How long do we need?




I have finished a front-end to QFileSystemWatcher and fixed
http://www.lyx.org/trac/ticket/10072. The UI still requires some
polishing but it is nearly there.

There is the u32string patch that is almost complete. But I need to get
back to it.

I think Richard has a few pending patches I am interested in:
https://www.mail-archive.com/lyx-devel@lists.lyx.org/msg195246.html
https://www.mail-archive.com/lyx-devel@lists.lyx.org/msg195204.html

But the most important to me is if 2.3 could please be branched at
feature freeze so that I don't have a backlog of patches to rebase six
months later.

If you create a 2.4 milestone then I can already re-target the bugs that
I care about so as to have a better picture of 2.3.

Guillaume




Re: Regular expression search

2017-03-04 Thread Guillaume Munch

Le 27/02/2017 à 22:08, Georg Baum a écrit :

Guillaume Munch wrote:


AFAIR, the ECMAScript regexes are a proper subset of PCRE whereas there
are incompatibilities between POSIX and PCRE. Moreover ECMAScript is the
default for , so maybe it is better supported across compilers.
This makes ECMAScript the simplest for a transition.

Is the bug there with g++ 6? I remember Georg saying that the early
implementations of  in gcc were buggy.


The first usable version is in 4.9.0, so  in gcc should not be a
problem anymore on any recent distro.


On the other hand I do not remember that dynamically created regexes
were reviewed for adaptation to ECMAScript so maybe there is indeed
something to change, although not (?:.

Dear Georg, do you remember the same?


I do not remember any review of dynamically generated regexes. I also have
to admit that dynamically generated regexes are too complicated IMHO (too
many levels of indirection). Unfortunately I do not understand the advanced
search well enough to propose a better solution.





Thank you Georg for the reply.

Another solution if nobody finds how to fix it is to disable
std::regex until one does.

Guillaume




Re: Automatic configuration of forward search

2017-03-04 Thread Guillaume Munch

Le 25/02/2017 à 18:09, Scott Kostyshak a écrit :

On Wed, Aug 03, 2016 at 10:11:38PM -0400, Scott Kostyshak wrote:

It would be nice if users did not have to configure forward search
(although they should have the option to do so if they prefer). I see
two possibilities:

(1) Can configure.py do this easily, assuming that the PDF viewer that
is automatically set by configure.py is the one that will be used? I
think it is better to make a guess that is probably accurate 75% of the
time than to not guess at all.

(2) We can move the settings from
Tools > Preferences > Output > General
to
Tools > Preferences > File Handling > File Format.

Option (2) seems intuitive to me because the forward search command
should depend on the viewer of the file format. If there are different
viewers for different formats, then the forward search command would
automatically adapt because there would be a different forward search
command for each format.

Any thoughts?


For evince one could also do better by shipping and configuring 
https://github.com/SublimeText/LaTeXTools/tree/master/evince




Note that I'm not necessarily interested in working on this now myself.


Me neither


Since I am new to forward search, I'm trying to think through a few
issues with a fresh perspective before I set up things that work for me
and forget what is intuitive to new users.




I agree this could be improved





Re: [LyX/master] Generalise the deletion protection mechanism from math to text (#9540)

2017-02-28 Thread Guillaume Munch

Le 28/02/2017 à 11:22, Jean-Marc Lasgouttes a écrit :


I agree. My point is that the behavior should be predictable and as far
as possible uniform between text and math.

Note that I am actually not sure that special casing empty insets is a
good idea. I makes things less predictable, and I do not know when it is
useful.



Makes sense. What do people prefer?




Re: [LyX/master] Generalise the deletion protection mechanism from math to text (#9540)

2017-02-27 Thread Guillaume Munch

Le 27/02/2017 à 11:23, Jean-Marc Lasgouttes a écrit :


Small questions though:
- I create an empty \mathord inset in a formula, backspace does use
deletion confirmation. Is this normal?


This is the legacy behaviour, but can be changed.


- if there are several cells (empty fraction) that are empty, why should
there be a confirmation?


Some as above; in addition, for math hulls I decided that the counts of 
lines and columns was non-empty data.


All this can now be redefined very easily via confirmDeletion.



Re: EmDash Problems (patch)

2017-02-26 Thread Guillaume Munch

Le 25/02/2017 à 21:09, Enrico Forestieri a écrit :


Then, a decision has to still be taken as regards the original problem
in this thread. I think that the patch Günther proposed on Jan. 25 is
the less controversial one.



I think you mean
https://www.mail-archive.com/lyx-devel@lists.lyx.org/msg198620.html

From someone who does not have strong opinions about the dashes issues
this looks good to me.

The idea of introducing a LFUN so that the behaviour of the - key is not
hard-coded sounds good as well.

Guillaume



Re: [LyX/master] Generalise the deletion protection mechanism from math to text (#9540)

2017-02-25 Thread Guillaume Munch

Le 25/02/2017 à 21:12, Guillaume Munch a écrit :

Le 25/02/2017 à 20:07, Guenter Milde a écrit :


On a similar note: could the "backspace at begin of inset dissolves it"
feature be ported to mathed, too?




This is a good idea, for consistency with math macros. If you are
interested in implementing it, I can suggest to look how it is done in
MathMacro.cpp.




Sorry I am not sure I understood. It already works for mathed insets
inside mathed insets. Did you mean for converting math hulls into text?



Re: EmDash Problems (patch)

2017-02-25 Thread Guillaume Munch




../../../../src/frontends/qt4/GuiViewSource.cpp: In member function ‘void 
lyx::frontend::ViewSourceWidget::realUpdateView()’:
../../../../src/frontends/qt4/GuiViewSource.cpp:229:14: error: ambiguous 
overload for ‘operator!=’ (operand types are ‘const QChar’ and ‘char’)
   while (*oc != '\0' && *nc != '\0' && *oc == *nc) {
  ^


I can backport 6d375dde to fix this.




Re: [LyX/master] Generalise the deletion protection mechanism from math to text (#9540)

2017-02-25 Thread Guillaume Munch

Le 25/02/2017 à 20:07, Guenter Milde a écrit :

On 2017-02-18, Guillaume Munch wrote:

Generalise the deletion protection mechanism from math to text (#9540)



Now backspace and delete in text will select non-empty math and
text insets before deleting them. This is consistent with what
happens in math already.


Sounds good.

On a similar note: could the "backspace at begin of inset dissolves it"
feature be ported to mathed, too?




This is a good idea, for consistency with math macros. If you are
interested in implementing it, I can suggest to look how it is done in
MathMacro.cpp.

Guillaume



Re: Regular expression search

2017-02-24 Thread Guillaume Munch

Le 24/02/2017 à 20:54, Kornel Benko a écrit :

Am Freitag, 24. Februar 2017 um 17:30:39, schrieb Jean-Marc Lasgouttes 


src/lyxfind.cpp:escape_map.push_back(P("\\",
"(?:|backslash)"));
src/lyxfind.cpp:escape_map.push_back(P("~",
"(?:textasciitilde|sim)"));
src/lyxfind.cpp:escape_map.push_back(P("^",
"(?:\\^|textasciicircum\\{\\}|mathcircumflex)"));
src/lyxfind.cpp:escape_map.push_back(P("",
"(?:|backslash|textbackslash\\{\\})"));
src/lyxfind.cpp:escape_map.push_back(P("\\^",
"(?:\\^|textasciicircum\\{\\}|mathcircumflex)"));

So for now, we need to understand what is this syntax and use the
ECMAscript equivalent. AFAICS, the only for used is (?:, described as:


(?: is part of the ECMAScript std and should be supported:
https://www.ecma-international.org/ecma-262/5.1/#sec-15.10.2.8

Moreover it seems to be used to include the regex into another regex so
it cannot be removed.



Or use the POSIX-extension? See 
http://eli.thegreenplace.net/2012/11/14/some-notes-on-posix-regular-expressions
Shoudn't then the regexes be compatible with boost-POSIX?
In our sources we test for 'LYX_USE_STD_REGEX' only in 
src/frontends/qt4/GuiCitation.cpp,
but I'd expect to see this also in src/regex.cpp



AFAIR, the ECMAScript regexes are a proper subset of PCRE whereas there
are incompatibilities between POSIX and PCRE. Moreover ECMAScript is the
default for , so maybe it is better supported across compilers.
This makes ECMAScript the simplest for a transition.

Is the bug there with g++ 6? I remember Georg saying that the early
implementations of  in gcc were buggy.

On the other hand I do not remember that dynamically created regexes 
were reviewed for adaptation to ECMAScript so maybe there is indeed 
something to change, although not (?:.


Dear Georg, do you remember the same?


Guillaume



Re: [LyX/master] Generalise the deletion protection mechanism from math to text (#9540)

2017-02-24 Thread Guillaume Munch

Le 22/02/2017 à 14:59, Jean-Marc Lasgouttes a écrit :

Le 18/02/2017 à 23:05, Guillaume Munch a écrit :

commit 71623b88b2c613dd4ab826a9783a53e840bcd6e1
Author: Guillaume Munch <g...@lyx.org>
Date:   Sat Feb 18 19:12:55 2017 +0100

Generalise the deletion protection mechanism from math to text
(#9540)

Now backspace and delete in text will select non-empty math and
text insets
before deleting them. This is consistent with what happens in math
already.

This is implemented for InsetText as well but can be disabled in
case of
negative feedback.

This can be set for any sort of inset with the new virtual method
Inset::confirmDeletion.


I like it in theory (we'll see for the practice). It would have been
nice to implement this as much as possible at generic level
(BUfferView::dispatch) so that math and text really behave identically.


I am not sure to understand your suggestion. Insets are supposed to be 
able to customize LFUN_*_DELETE_* in whichever way they want. How would 
you make this mechanism general without introducing assumptions on how 
it has to be implemented?



I do not see confirmDeletion implemented there and I do not see a
special handling for empty insets.



The checks notion of empty inset is implemented in the
Inset::confirmDeletion themselves.

Thanks for the feedback.
Guillaume



Re: Memory leak in master?

2017-02-24 Thread Guillaume Munch

Le 22/02/2017 à 18:03, Jean-Marc Lasgouttes a écrit :

Le 21/02/2017 à 20:13, Guillaume Munch a écrit :


BTW, why don't you use Cache::contains in getLayout like you do for
other cache uses?



This is explained in the documentation of Cache::object. It is enough to
check for a null pointer in that case.


It would be nice to hide these subtleties in the Cache implementation,
if possible.



Hi Jean-Marc,

I am not sure what you mean with "hide these subtleties". The
specification is simple: if the key does not exist in the cache, you get
the default object. I thought about using boost::optional at first but
for the main use I had in mind (shared_ptr) I found that the code would
be heavier (you end up having to dereference twice) for little clarity gain.

Guillaume




Re: [LyX/master] Add tableaux to outliner

2017-02-21 Thread Guillaume Munch

Le 21/02/2017 à 19:28, Kornel Benko a écrit :

Am Dienstag, 21. Februar 2017 um 18:52:28, schrieb Guillaume Munch 
<g...@lyx.org>

Le 21/02/2017 à 18:49, Kornel Benko a écrit :


Could it be made translatable?



It should be already.



It is nether in po-file nor in layouttranslations.
lyx_pot.py searches
OutlinerName = 
re.compile(r'^[^#]*OutlinerName\s+(\S+|\"[^\"]*\")\s+(\S+|\"[^\"]*\")\s*$', 
re.IGNORECASE)
but it does not match the line
OutlinerName tableau"Tableaux" #no AddToToc (built-in)
It would match only
OutlinerName tableau"Tableaux"

Kornel



Thanks for pointing out this issue. It should be fixed now.



Re: Memory leak in master?

2017-02-21 Thread Guillaume Munch

Le 21/02/2017 à 07:19, Jean-Marc Lasgouttes a écrit :

Le 21/02/2017 à 00:08, Guillaume Munch a écrit :


Could you do the backport to stable? :)


What about that? Please check especially the code related to
LYX_USE_CXX11 in Cache.h. For the caching, I re-read the code to make
sure that my hand-merging was correct, I hope I did not miss anything.


Thanks for doing this. I thought there was some bigger adaptation to the
code to do but indeed it only looks like a matter of C++98 conversion.

I think it's all good except:

class Cache : private QCache<Key, Val> {
-#if !(defined(__GNUC__) && (__GNUC__ == 4) && (__GNUC_MINOR__ == 6))
+#if defined(LYX_USE_CXX11) && !(defined(__GNUC__) && (__GNUC__ == 4) && 
(__GNUC_MINOR__ == 6))





BTW, why don't you use Cache::contains in getLayout like you do for
other cache uses?



This is explained in the documentation of Cache::object. It is enough to
check for a null pointer in that case.

Guillaume



Re: Memory leak in master?

2017-02-21 Thread Guillaume Munch

Le 20/02/2017 à 18:25, Jean-Marc Lasgouttes a écrit :

Le 10/02/2017 à 17:58, Guillaume Munch a écrit :

There's more data, but I am not sure how to interpret the results that
massif-visualizer shows.


If you send the file or make it available, I can take a look.


Here it is. But if it is like callgrind, it might lack the symbols.


Interestingly, the massif output that you sent also points to
MathMacro::clone(). Besides the obvious getTextLayout leak that needs to
be plugged, would you have an idea on why it is specifically macros that
appear here (with an increasing memory use, like the other)? I do not
see other math objects doing that.

Note that there are other sources that point to MathAtom::MathAtom, as
the drop in percentage shows.



Interesting, massif-visualizer shows the results differently. With it
one sees that MathAtom quickly reaches its final space usage and even
decreases at some point. It does not look similar to the slow creep of
the memory leak. Again, I am not sure that I know how to interpret the
results.

Guillaume




Re: [LyX/master] Add tableaux to outliner

2017-02-21 Thread Guillaume Munch

Le 21/02/2017 à 18:49, Kornel Benko a écrit :


Could it be made translatable?



It should be already.




Re: [LyX/master] Add tableaux to outliner

2017-02-21 Thread Guillaume Munch

Le 21/02/2017 à 07:11, Juergen Spitzmueller a écrit :

commit ed233663008608e5e32a3c675ade790ea63f0a24
Author: Juergen Spitzmueller 
Date:   Tue Feb 21 07:11:23 2017 +0100

Add tableaux to outliner
---

+OutlinerName tableau"Tableaux" #no AddToToc (built-in)
+


Thanks, I missed that one.




Re: Standalone Math Editor

2017-02-21 Thread Guillaume Munch

Le 20/02/2017 à 07:44, Wei-Ting Lin a écrit :

Thanks a lot!

I'm trying Lyxserver and Lyx functions. They are amazing. I can just
send commands to open a document and insert a display math!

One problem seems that there is no Lyx function allowing me to get the
content of math. I can use select and copy to put it into clipboard,
then maybe read the clipboard to get the content. This seems not very
elegant and reliable?



I suggest creating a new LFUN that would be handled by InsetMathHull
(the topmost math inset).

Commit e91572a0 will show you an example for adding a new LFUN. Instead
of GuiView::dispatch and GuiView::getStatus you are looking to add to
the equivalent methods of InsetMathHull.

InsetMathHull::write gives you the LaTeX code for the math inside the
inset. Now what I don't know personally is how to send the result back
via the lyx server.

Guillaume




Re: Memory leak in master?

2017-02-20 Thread Guillaume Munch

Le 20/02/2017 à 18:27, Jean-Marc Lasgouttes a écrit :

Le 13/02/2017 à 20:55, Jean-Marc Lasgouttes a écrit :

What I mean is that you can easily force
QCache into shared ownership by replacing

QCache

with

QCache

and create the appropriate wrappers for insertion and retrieval.


Could you have a go at this solution?



Could you do the backport to stable? :)




Re: Standalone Math Editor

2017-02-18 Thread Guillaume Munch

Le 18/02/2017 à 06:44, Guenter Milde a écrit :

On 2017-02-18, Wei-Ting Lin wrote:


Your suggestions are valuable and important for me.



I never think Lyx is not lightweight. Actually, I'm always satisfied
with its speed. I just think to type math equations we don't need a
full Lyx. But you're right. The first step should be the input/output
function.



I'm thinking to implement the following function:



(1)When I'm editing an equation, say \[ f(x)= \], I can open Lyx with,
say Ctrl+C.



(2)Lyx is initiated with a display math mode, so I can edit the equation.



(3)After finishing editing, push some keybinding to hide the Lyx
window and return the Latex code to the editor.



(4)The editor can receive the Latex code and insert it into my file.



I think (1) and (4) involve the editor's functions, and should be put
aside at the beginning.



For (2) and (3), do you have any suggestions? I've checked the source
code, and kind of overwhelmed.


For 2), you can write a template file and open lyx with that.
I don't see a "--new-from" (or similar) command line option, so this
seems like a task for the "lyxserver"¹.

3) hiding the lyx window is a function for the window manager, again the
   lyxserver may help.

   pushing the content to the clipboard would be simply:
   command-sequence; select-all; copy;
   Alternatively, the lyxpipe can receive the content.



In addition, I am thinking about a LFUN that sends the LaTeX contents of 
the math hull at the cursor location to wherever LyX has been instructed 
to at step 2) (and will do the closing/hiding).


What I am not familiar with is the lyxserver side.






Re: Standalone Math Editor

2017-02-17 Thread Guillaume Munch

Le 17/02/2017 à 07:17, Wei-Ting Lin a écrit :

Hi all,

I have used Lyx for several years. This is a really amazing software.

However, many people still write LaTex with text editors. That usually
means when we cowork on some article, we have to write LaTex directly.

This is ok. However, sometimes we have very long and complicated
equations. One of amazing features of Lyx is its math editor. This makes
the equation visible, when you're editing. That has helped me a lot.

Therefore, I'm thinking if it is possible to separate the math editor,
as a standalone, lightweight editor.

I'm also thinking if it is possible to call the editor from vim, emacs
or whatever, when we need to edit an equation, and then return the LaTex
code back to vim or emacs.

It seems there is no such work yet. I'm wondering how difficulty to
implement this. If this is not too hard, I would like to try to do it.

Thank you very much,
Wei-Ting



Dear Wei-Ting,

Using LyX as a stand-alone math editor is definitely feasible. The key
design that makes this possible is that for math, LyX directly reads and
writes LaTeX code (unlike for the text). Essentially you would get what
you can already do by copy-pasting LaTeX math into LyX math and copying
the LaTeX preview back into your document. I would be happy to see such
a feature implemented.

It should be easy to implement basic input/output (what you call "call
the editor and then return"). Then there would be some moderate effort
for the UI/cosmetics ("separate the math editor"). As for "lightweight":
LyX is already pretty quick; start-up is slow but this could be improved
a lot with a small amount of work to fix a few known bottlenecks.
Lastly, some LaTeX features are currently unsupported, but nothing
opposes that they could get implemented over time.

If you manage to get started with a prototype for the input/output
aspects, the rest will follow from there (in particular the cosmetic
aspects, which can be time-consuming and are best left to an ulterior
time). Assume for instance that you want to use a tex file containing
the math equation as the medium between your text editor and LyX.
Essentially LaTeX import works fine in this case, and what you want to
do on the LyX side is make it convenient to write the LaTeX math back
into the original tex file. Do not hesitate to ask for directions.

Sincerely,
Guillaume



Re: alpha dot

2017-02-15 Thread Guillaume Munch

Le 15/02/2017 à 13:46, Guenter Milde a écrit :

On 2017-02-15, Guillaume Munch wrote:


[-- Type: text/plain, Encoding: 8bit --]



Le 15/02/2017 à 05:02, Scott Kostyshak a écrit :

On Wed, Jan 04, 2017 at 09:00:44AM +0100, Wolfgang Engelmann wrote:

On 03.01.2017 21:32, Scott Kostyshak wrote:

On Mon, Jan 02, 2017 at 11:25:07AM +0100, Wolfgang Engelmann wrote:



How could I get a dot on an alpha by using the math function of
lyx? Or do I have to use a tex insert:



$\dot \alpha$


Instead of a tex insert (ERT), use a mathed inset (Ctrl-M, say) and
insert \dot \alpha there. This is converted to the right symbol(s) in
both, LyX and output document.


Click on the "frame decorations" icon in the math toolbar. See the
screenshot. Does that work for you?


This gives the same result.


In the LyX math preview (i.e. without instant preview), the "dot" looks
more like a small line. Do you agree or does it look correct to you?



Yes, Scott, in the instant preview it does look like a short line, but its
correct in the pdf output.



I tried to fix it, but it seems that in this part of the code we can't
really draw circles; we can only trick the user into thinking its a
little circle by drawing a short line. We could draw a polygon with many
sides (I guess this is how all circles are drawn on screens?). Is there
a program that outputs coordinates in the format required by the code in
src/mathed/MathSupport.cpp ?



Another interesting issue is that the math decorations do not zoom with
the font; (because)? if they did, our approximations would look pretty
rough I guess. I'm actually amazed with how well most of them work. I
never realized a problem before.



I have a series of patches (attached) that make the decoration thickness
increase with zoom, to improve the appearance on high-dpi. The
decorations do not look worse than without the added thickness. I have
not committed it yet because I have not found a way to keep it simple,
and there are still some issues. But you might like to try them.



For \dot in particular it looks like a square rather than a line.



Did you think about using Unicode and, e.g., the STIX fonts for mathed?
I know this is a major change but it should solve issues like scaling and
curved lines.


Harfbuzz has been providing access to the the math table of opentype
fonts since very recently, and even computes the glyph assemblies for
decorations. So in theory it is possible to implement variable-size
decorations with opentype fonts. See
http://www.lyx.org/trac/ticket/9014#comment:7. But that's a big project.



For the "frame decorators", one could explore the use of combining
characters - maybe there are issues with placement but α̇ or 훼̇ looks better
in mathed than the current "square dot".


Indeed, for standard-sized decorations, it would be simpler to implement.



Re: alpha dot

2017-02-15 Thread Guillaume Munch

Le 15/02/2017 à 05:02, Scott Kostyshak a écrit :

On Wed, Jan 04, 2017 at 09:00:44AM +0100, Wolfgang Engelmann wrote:



On 03.01.2017 21:32, Scott Kostyshak wrote:

On Mon, Jan 02, 2017 at 11:25:07AM +0100, Wolfgang Engelmann wrote:

How could I get a dot on an alpha by using the math function of lyx? Or do I
have to use a tex insert:

$\dot \alpha$

Click on the "frame decorations" icon in the math toolbar. See the
screenshot. Does that work for you?

In the LyX math preview (i.e. without instant preview), the "dot" looks
more like a small line. Do you agree or does it look correct to you?

Scott

Yes, Scott, in the instant preview it does look like a short line, but its
correct in the pdf output.


I tried to fix it, but it seems that in this part of the code we can't
really draw circles; we can only trick the user into thinking its a
little circle by drawing a short line. We could draw a polygon with many
sides (I guess this is how all circles are drawn on screens?). Is there
a program that outputs coordinates in the format required by the code in
src/mathed/MathSupport.cpp ?

Another interesting issue is that the math decorations do not zoom with
the font; (because)? if they did, our approximations would look pretty
rough I guess. I'm actually amazed with how well most of them work. I
never realized a problem before.



I have a series of patches (attached) that make the decoration thickness
increase with zoom, to improve the appearance on high-dpi. The
decorations do not look worse than without the added thickness. I have
not committed it yet because I have not found a way to keep it simple,
and there are still some issues. But you might like to try them.

For \dot in particular it looks like a square rather than a line.

Guillaume
>From 45d8c5edc2c8e26065468c691f90a1d50ddc20a9 Mon Sep 17 00:00:00 2001
From: Guillaume Munch <g...@lyx.org>
Date: Mon, 28 Nov 2016 03:27:30 +0100
Subject: [PATCH 1/3] Increase thickness of math delimiter with zoom

---
 src/mathed/InsetMathAMSArray.cpp   | 15 ++-
 src/mathed/InsetMathBig.cpp| 19 +--
 src/mathed/InsetMathCases.cpp  |  4 +++-
 src/mathed/InsetMathDecoration.cpp | 15 +--
 src/mathed/InsetMathDelim.cpp  | 25 ++---
 src/mathed/InsetMathDots.cpp   | 13 +
 src/mathed/InsetMathFrac.cpp   | 11 +++
 src/mathed/InsetMathXArrow.cpp | 19 +--
 src/mathed/MathSupport.cpp | 26 +++---
 src/mathed/MathSupport.h   | 10 +-
 10 files changed, 110 insertions(+), 47 deletions(-)

diff --git a/src/mathed/InsetMathAMSArray.cpp b/src/mathed/InsetMathAMSArray.cpp
index 9a9a349..df022b6 100644
--- a/src/mathed/InsetMathAMSArray.cpp
+++ b/src/mathed/InsetMathAMSArray.cpp
@@ -88,19 +88,24 @@ void InsetMathAMSArray::metrics(MetricsInfo & mi, Dimension & dim) const
 	Changer dummy2 = mi.base.changeEnsureMath();
 	Changer dummy = mi.base.changeArray();
 	InsetMathGrid::metrics(mi, dim);
+	mathed_deco_metrics(mi.base, dim);
 }
 
 
 void InsetMathAMSArray::draw(PainterInfo & pi, int x, int y) const
 {
 	Changer dummy2 = pi.base.changeEnsureMath();
+	{
+		Changer dummy = pi.base.changeArray();
+		InsetMathGrid::draw(pi, x, y);
+	}
 	Dimension const dim = dimension(*pi.base.bv);
 	int const yy = y - dim.ascent();
-	// Drawing the deco after changeStyle does not work
-	mathed_draw_deco(pi, x + 1, yy, 5, dim.height(), from_ascii(name_left()));
-	mathed_draw_deco(pi, x + dim.width() - 8, yy, 5, dim.height(), from_ascii(name_right()));
-	Changer dummy = pi.base.changeArray();
-	InsetMathGrid::draw(pi, x, y);
+	int const t = mathed_deco_thickness(pi.base);
+	mathed_draw_deco(pi, x + 1 - t/2, yy,
+	 5, dim.height(), from_ascii(name_left()));
+	mathed_draw_deco(pi, x + dim.width() - t/2 - 8, yy,
+	 5, dim.height(), from_ascii(name_right()));
 }
 
 
diff --git a/src/mathed/InsetMathBig.cpp b/src/mathed/InsetMathBig.cpp
index 889f8d4..e5848aa 100644
--- a/src/mathed/InsetMathBig.cpp
+++ b/src/mathed/InsetMathBig.cpp
@@ -24,6 +24,10 @@
 #include "support/docstream.h"
 #include "support/lstrings.h"
 
+#include 
+
+using namespace std;
+
 
 namespace lyx {
 
@@ -90,10 +94,12 @@ void InsetMathBig::metrics(MetricsInfo & mi, Dimension & dim) const
 {
 	Changer dummy = mi.base.changeEnsureMath();
 	double const h = theFontMetrics(mi.base.font).ascent('I');
-	double const f = increase();
-	dim.wid = 6;
-	dim.asc = int(h + f * h);
-	dim.des = int(f * h);
+	double const height = h * (1 + 2 * increase());
+	int const axis = axis_height(mi.base);
+	dim.wid = max(6, mathed_mu(mi.base.font, 6.0));
+	dim.asc = int(height/2 + axis);
+	dim.des = int(height/2 - axis);
+	mathed_deco_metrics(mi.base, dim);
 }
 
 
@@ -115,8 +121,9 @@ void InsetMathBig::draw(PainterInfo & pi, int x, int y) const
 {
 	Changer dummy = pi.base.changeEnsureMath();
 	Dime

Re: Reproducible crash with attached template

2017-02-01 Thread Guillaume Munch
(Sending again this message from 21/01/2017 which neither bounced nor 
made it to the list.)


Le 26/01/2017 à 20:05, Richard Heck a écrit :


This looks familiar. There was a crash a month or two ago, maybe, that
also happened during reload (see #9 below) due to invalid cursors.
Guillaume, I think you were the one who fixed that other problem?




I think you are looking for 3d6a7a12. Well, the call to Cursor::sanitize
in the trace was precisely introduced to solve this other crash. Maybe
the more brutal fixIfBroken is necessary? As I cannot reproduce, please
tell me if it fixes the crash. Then I wonder whether it shows a bug in
sanitize (unrelated to the current shortfall of the inset cache
implementation discussed previously) or whether I just called sanitize
inappropriately for some undocumented notion of inappropriate.

Guillaume

diff --git a/src/frontends/qt4/GuiView.cpp b/src/frontends/qt4/GuiView.cpp
index a6b5877..7edbcc5 100644
--- a/src/frontends/qt4/GuiView.cpp
+++ b/src/frontends/qt4/GuiView.cpp
@@ -1655,7 +1655,7 @@ void GuiView::structureChanged()
 	// This is called from the Buffer, which has no way to ensure that cursors
 	// in BufferView remain valid.
 	if (documentBufferView())
-		documentBufferView()->cursor().sanitize();
+		documentBufferView()->cursor().fixIfBroken();
 	// FIXME: This is slightly expensive, though less than the tocBackend update
 	// (#9880). This also resets the view in the Toc Widget (#6675).
 	d.toc_models_.reset(documentBufferView());



Re: [LyX/master] Collect the outliner names for the children's tocs

2017-02-01 Thread Guillaume Munch

Le 21/01/2017 à 04:23, Scott Kostyshak a écrit :

LyX places the cursor at the correct spot, but the outliner is collapsed.
Before, the outliner was not collapsed.

There are other changes of behavior that can be seen, using this file.
For example, just clicking on "Section 2" or "Section 5", the outliner
does not highlight the section anymore like it used to.


For the record this is fixed at 15f64f84.




Re: [LyX/master] Collect the outliner names for the children's tocs

2017-01-25 Thread Guillaume Munch

Le 21/01/2017 à 04:23, Scott Kostyshak a écrit :

On Sat, Jan 14, 2017 at 11:16:29PM +0100, Guillaume Munch wrote:

commit 461fda9ca9a8079f235fe6d26031aa6b9c5ff36e
Author: Guillaume Munch <g...@lyx.org>
Date:   Sat Jan 14 18:40:58 2017 +0100

Collect the outliner names for the children's tocs

Fixes missing outliner names in various situations. Now if the warning 
"Missing
outliner name" appears in the console, this correctly hints at an actual 
issue
with the layout.


I think this commit caused a change in behavior that I'm not sure was
intended.


Rather, it is the fault of 3391fed3




Can you reproduce?


Yes



I imagine you will have higher priority issues to look at when you come back
before this one, so no worries if you don't get to it for a while.



Indeed, not today. Thanks for your patience.




Re: row-breaking question

2017-01-25 Thread Guillaume Munch

Le 21/01/2017 à 20:22, Jean-Marc Lasgouttes a écrit :

Le 21/01/2017 à 19:33, Scott Kostyshak a écrit :

The attached sequence of screenshots show the phases of row-breaking
that happens as I keep typing " abc" in the footnote. The word "OLS"
seems to jump around a lot and I find that (mildly) annoying.

Are all six phases of the row breaking desired?


They are mostly not desired.
* 3 is clearly a bug
* 2 and 5 should be avoidable, but it requires to have some knowledge
that breaking before the inset is more desirable than in the middle of
the text. It might be doable. I have code somewhere to allow each inset
to declare whether it allows breaking before/after.



This is the sort of behaviour which is fixed (IIRC) by the improvement
to row breaking described in
.




Re: [CONFIRMED] Display Problem in Stable with \notin, etc.

2017-01-25 Thread Guillaume Munch

Le 19/01/2017 à 16:04, Jean-Marc Lasgouttes a écrit :


Here is what happened
1/ I change randomly (!) the spacing of math equations in the MathRow work
2/ as a consequence the formulas in lib/symbols are often wrong
3/ I fix the problem with arabic test at  e832d2e90f
4/ I do a new round of modifications of lib/symbols at  7335ee8e,
without realizing that what I am fixing is a consequence of the commit
3/ and not of the ongoing work 1/

Does this make sense to you? The fact that the right value now is
without explicit kern is a proof that all this work on math formula
spacing has been done correctly (almost!)


Makes sense to me. Lots of changes to spacing, and lib/symbols that lags
behind. A bug slips by the new features.

Nice that this odd kern is not needed after all. The value has been
found by trial-and-error. However may I suggest: what's good for display
is not necessarily the best for structural interaction—can we
reintroduce a kern to make it slightly imperfect, so that there is
feedback during selection (as you did in current master for \Arrownot)?

Another thing I would have mentioned is indeed that \{A,a}rrownot works
on the same principle, but I saw that you already took care of it.

(speaking about master, did not try stable)



Re: EmDash Problems (patch)

2017-01-25 Thread Guillaume Munch

Le 25/01/2017 à 22:51, Guenter Milde a écrit :

On 2017-01-25, Enrico Forestieri wrote:


The appearance on screen is the same but we always output "--" and
"---", except on text exports where the unicode characters are output.
This should ensure maximum compatibility without cluttering the
preamble with strange constructs. After all, what matters is the
final pdf output and I very much prefer to have "--" and "---" in
the latex code rather than \textendash and \textemdash.


...
I recommend the lib/unicodesymbols patch below as simple fix for the line
wrapping problem.

This would still not enable input of a line of hyphen characters in
"normal" text, though but maybe pressing CTRL-M (or CTRL-P C) *before*
the input is an acceptable workaround.

With an LFUN instead of the hard-coded binding to "-" for the input
convention, a user can turn off the auto-dash feature without need for
an new special setting. Then, also the input of - becomes
easy --- at the expense of more complicated input of the dashes.


+1 for the idea of a LFUN set up as a default. In addition it would be
nice if EM-dash + dash = 4 dashes... if only it was clear that EM-dash =
---.

A problem seems to be, in LyX 2.1 there was a use for both
\texte(n/m)dash and --(-) according to
https://marc.info/?l=lyx-users=140982011101908=2 because of the
different line breaking behaviour. By assigning U+2014 to --- it
regresses wrt 2.1 for ease of insertion of \textemdash (resp. endash) 
which was done by inserting —.


Also it seems that \textemdash + break point has advantages wrt ---, 
e.g. hyphenation 
https://tex.stackexchange.com/questions/56657/hyphenation-problem-with-versus-textemdash


So, a LFUN for - → – → — → ... is nice; one has to take seriously these 
line breaking issues (e.g. introduce blue dashes vs red dashes... this 
is getting complicated); and even with this distinction, --- is not 
without flaws...


No strong opinions. Looks like a difficult situation :)





Re: [LyX/2.2.x] Assure long tooltip is correctly formatted.

2017-01-14 Thread Guillaume Munch

Le 14/01/2017 à 16:26, Kornel Benko a écrit :


I don't see it. Here it displays as a 6-liner in Slovak translation.
QT5.7
OS Mint17.3
desktop MATE

Kornel



Thanks Kornel and Jürgen for the replies. I will stop my investigation
but do not hesitate to report issues like this in the future.

Guillaume



Re: [LyX/master] Add an "Automatic" bibliography processor pref option

2017-01-14 Thread Guillaume Munch

Le 13/01/2017 à 23:28, Richard Heck a écrit :


Format change?



It does not look like it to me. However:


/// Return the actual bibtex command (lyxrc or buffer param)
-   std::string const & bibtexCommand() const;
+   std::string const bibtexCommand() const;


I saw const-value return types a few times in the code. It seems that
this was advised in certain circumstances in old C++, but this is now
discouraged (essentially because it prevents automatic move operations
and serves no good enough purpose). See for instance
.

Guillaume




Re: [LyX/2.2.x] Assure long tooltip is correctly formatted.

2017-01-14 Thread Guillaume Munch

Le 14/01/2017 à 10:07, Jürgen Spitzmüller a écrit :

Am Freitag, den 13.01.2017, 22:02 +0100 schrieb Guillaume Munch:

Weird, it displays fine here at 119dfcb1. Are there precise steps
that I
must follow to reproduce the bug?


* Open the BibTeX inset dialog
* Hit "Add"
* On the sub-dialog, hover over the list widget

I get a long one-liner tooltip.




Here it formats as expected. In case this hides a serious
shortcoming, can anyone else please try to reproduce this issue?
(between ffb195b5 and 6b214804 excluded)

Have you noticed this issue at other places?

What is the Qt version, locale, OS, and desktop environment?

Thanks
Guillaume



Re: [LyX/2.2.x] Assure long tooltip is correctly formatted.

2017-01-13 Thread Guillaume Munch

Le 13/01/2017 à 11:32, Jürgen Spitzmüller a écrit :

Am Freitag, den 13.01.2017, 11:11 +0100 schrieb Guillaume Munch:

(Independently,
I will try to see why this was not called automatically in master.)


OK. Maybe because this is a sub-dialog?



Weird, it displays fine here at 119dfcb1. Are there precise steps that I
must follow to reproduce the bug?

Guillaume



Re: [LyX/2.2.x] Assure long tooltip is correctly formatted.

2017-01-13 Thread Guillaume Munch

Le 13/01/2017 à 09:09, Juergen Spitzmueller a écrit :

commit ca9c8dbde1d8a3d4ceeba990bbfcab17bfecb940
Author: Juergen Spitzmueller 
Date:   Fri Jan 13 09:07:44 2017 +0100

Assure long tooltip is correctly formatted.

Amendment to ffb195b5e9fa

...

+   add_->bibLW->setToolTip(formatToolTip(qt_("This list consists of all 
databases that are indexed by LaTeX and thus are found without a file path. "

...

Hi Jürgen, I do not remember backporting formatToolTip to stable. Either
one can backport it, or this should be only for master. (Independently,
I will try to see why this was not called automatically in master.)



Re: dynamic quotes

2017-01-07 Thread Guillaume Munch

Le 05/01/2017 à 06:39, Jürgen Spitzmüller a écrit :

Am Mittwoch, den 04.01.2017, 23:27 +0100 schrieb Guillaume Munch:

There I do not follow anymore: if without the checkbox you would need
12
key bindings, how is the checkbox helping? Or do you mean that even
the
checkbox does not address your needs?


Well, it is easier to alter the checkbox and style combo temporarily
than the bindings.



If you are ready to alter the style combo-box temporarily, then two
pairs of bindings should be enough too, isn't it? The only thing that
still worries me is that the checkbox is less flexible and makes the
assumption that one always want dynamic quote in foreign language.

I tested the latest GUI-related commit and I have a bug to report.
Create a new document, Language: French, Quotes style: language default.
Close and reopen the properties. Then instead of language default, here
the selected item is "»outer» and 'inner'".

Guillaume



Re: dynamic quotes

2017-01-04 Thread Guillaume Munch

Le 04/01/2017 à 17:54, Jürgen Spitzmüller a écrit :

Let me rephrase: This is style-dependent. Some styles require you to
use "foreign" quotes for all foreign text, some for "whole sentences".
Most German style sheets I know, however, require you to use the same
quotation marks (of the matrix language) for all used languages.

Your suggestion, if I understood it correctly, would fix one and
exclude all other options.

My suggestions would leave both happy: If you need different quotes,
uncheck the setting, if you need just one overall style, have it
checked. This is an argument all for the setting. And note that this
mostly depends on your publisher or editor, so it is ultimately
document-dependent, not author-dependent.


Indeed I did not imagine that the current behaviour of dynamic quotes
was on purpose, and I am a bit surprised that it is a thing. In turn,
dynamic quotes cannot be as general-purpose as I thought.




* Assuming that the above aspect is fixed by implementing one of my
   suggestions, then I wonder what corner cases remain. You do not
   say.


I hope it is clearer now.


Yes, thank you for your explanation.



The use-cases you described would be covered by letting you assign
two
different keys, one for static and one for dynamic.


That would be four keys: static outer, static inner, dynamic outer,
dynamic inner.


Yes, I actually meant two pairs of bindings.


And 12 for my personal use case: German inverted commas
outer, German inverted commas inner, German guillemets outer, German
guillemets inner, Swiss German outwards pointing guillemets outer,
Swiss German outwards pointing guillemets inner. And yes, I use those
in one document, and the language setting does not help here (even in
the Swiss German case).


There I do not follow anymore: if without the checkbox you would need 12
key bindings, how is the checkbox helping? Or do you mean that even the
checkbox does not address your needs?




Re: dynamic quotes

2017-01-04 Thread Guillaume Munch

Le 04/01/2017 à 15:42, Jürgen Spitzmüller a écrit :

Am Mittwoch, den 04.01.2017, 15:02 +0100 schrieb Guillaume Munch:


Starting with the important point:


So, here are two suggestions for improving dynamic quotes, good
enough (AFAIU) to be a default behaviour, sparing the need for a
document setting.


What is so bad about a document setting?


What is so bad about this document setting? Two things:

* If I check dynamic quotes, then it behaves in an unfriendly way
  inside foreign language: it becomes hard to insert foreign language
  quotes inside foreign language. My two suggestions are meant to
  address this (independently of whether a document setting is
  retained).

* Assuming that the above aspect is fixed by implementing one of my
  suggestions, then I wonder what corner cases remain. You do not
  say. There must be a good reason. The occasional use-case is already
  covered thanks to your new contextual menu, and the expert user with
  a very specific and permanent use-case can set his custom key
  bindings.

If the first point is fixed, but that convincing use-cases for
unchecking the box still existed, then I would agree that a document
setting has a place.


But for dynamic quotes you wrote:


There is even the need for dynamic and static quotes within the
same document.


which the document setting does not address, but additional LFUN
bindings (default or custom) does. This is why I see the argument
for mixing static and dynamic quotes, but not one for a document
setting.


And then users will have to change the default lfun every time they
want to switch quotation marks? This is most user unfriendly.


The use-cases you described would be covered by letting you assign two
different keys, one for static and one for dynamic. I agree that letting
the user define their LFUN is less user-friendly and this is why I
suggested other solutions.



BTW this would also mean they would also need to change the lfun
when switching from (static) guillemets to (static) inverted commas
for German, since this uses the same principle (change of default
lfun behavior).


If you read carefully, I am not arguing against the principle, only that
I am not convinced that it is justified in this case.



Your own argument why static quotes are still needed was about
foreign language.


I said it is _one_ reason where they might be needed. I did not say
that it is the only one, neither that language change always entails
quote style change.

You cannot make a one-for-all rule here, hence the user must be able
to set it on document level.


I understood that it is the root of the issue, but I do not see you
explain this point.




I am really discussing whether it is a "special character",


It is.


This really does not contribute to the discussion.




and how it should be presented to all users. The idea is that if it
becomes the default, then there is the possibility of making the
change transparent to users.


And how do you distinguish a static guillemet from a dynamic one?



Depends on what is chosen eventually: if static quotes become a rare
use-case as with my second suggestion, then they would become the
"special" one.




Re: dynamic quotes

2017-01-04 Thread Guillaume Munch

Le 04/01/2017 à 14:23, Jürgen Spitzmüller a écrit :

Am Mittwoch, den 04.01.2017, 13:52 +0100 schrieb Guillaume Munch:

I find too that the above does not make the case for a new document
setting that changes the default behaviour of a LFUN.


Note that the quote document setting has done that since always.


But for dynamic quotes you wrote:


There is even the need for dynamic and static quotes within the same
document.


which the document setting does not address, but additional LFUN
bindings (default or custom) does. This is why I see the argument for
mixing static and dynamic quotes, but not one for a document setting.



What about having quote-insert input dynamic quotes by default except
in
foreign language?


I don't see how this is connected to foreign languages.


Your own argument why static quotes are still needed was about foreign
language. So, here are two suggestions for improving dynamic quotes,
good enough (AFAIU) to be a default behaviour, sparing the need for a
document setting. The one below makes the more sense, I find. But maybe 
I misunderstood the specific use-cases you have in mind.





Or, have the dynamic quote as default, but have it behave like a
foreign
code's default quote in foreign language. This default is not
configurable, but it is the same with the static quote.





If it is going to be the default then please do not have it in blue.


Well, blue is our color for "special characters" (which this is).

Of course, you can alter the special characters color in the prefs, if
you don't like it.



I am really discussing whether it is a "special character", and how it
should be presented to all users. The idea is that if it becomes the
default, then there is the possibility of making the change transparent
to users.




Re: [LyX/master] Add ability to use refstyle's plural and capitalization features.

2017-01-04 Thread Guillaume Munch

Le 03/01/2017 à 04:40, Richard Heck a écrit :

commit 73f59e87bd62e00bbfbceee59b8876aa59e75f58
Author: Richard Heck 
Date:   Sat Jun 18 19:29:15 2016 -0400

Add ability to use refstyle's plural and capitalization features.
---
 lib/lyx2lyx/lyx_2_3.py|   67 +++--
 lib/lyx2lyx/parser_tools.py   |7 ++-
 src/frontends/qt4/GuiRef.cpp  |   27 ++-
 src/frontends/qt4/GuiRef.h|2 +
 src/frontends/qt4/ui/RefUi.ui |  112 -
 src/insets/InsetRef.cpp   |   34 +++--
 src/insets/InsetRef.h |2 +-
 src/version.h |2 +-
 8 files changed, 181 insertions(+), 72 deletions(-)



Thank you for this. I suggest adding tooltips explaining the role of the
two new checkboxes and how to have them enabled.




Re: dynamic quotes

2017-01-04 Thread Guillaume Munch

Le 01/01/2017 à 18:29, Guenter Milde a écrit :

On 2016-12-31, Jürgen Spitzmüller wrote:

Is there a use case for the document specific setting?
(I.e. is there someone in need for one keybinding inserting "dynamic"
quote insets in one document but "static" quote insets in another
document?)



Yes. I prefer to use static quotes in some documents, dynamic in the
other.




There is even the need for dynamic and static quotes within the same
document. If I need different quote styles within one document (e.g.,
English style within English quotations within a German document), I
need to use static quotes. Nevertheless I could still need to use a
dynamic type for the main quotes. The latter lets me switch from
inverted commas to guillemets (or vice versa) if my publisher requests
this.


Yes, this is why we need static quotes (or just a convenient way to insert
Unicode quote characters) besides the "dynamic" quotes.

But in any case the "foreign" quotes require either modifying the
once-inserted quote (via context menu) or insertion via LFUN with arguments
or the Insert>Character>... menu.

This would be independent of the value of an eventual (and in my view not
necessary) document setting.


I find too that the above does not make the case for a new document
setting that changes the default behaviour of a LFUN.




Also note that an LFUN change is all but user friendly. This would hide
the dynamic quotes from most users.


I propose to change the pre-set keybinding for " from "quote-insert" to
"quote-insert * * dynamic".


What about having quote-insert input dynamic quotes by default except in
foreign language?

Or, have the dynamic quote as default, but have it behave like a foreign
code's default quote in foreign language. This default is not
configurable, but it is the same with the static quote.



This would expose the new quote inset variant to users, especially as it is
now blue.



If it is going to be the default then please do not have it in blue.




Re: [LyX/master] TocWidget: fix an erroneous collapse and optimise updates based on profiling

2017-01-03 Thread Guillaume Munch

Le 31/12/2016 à 16:13, Jean-Marc Lasgouttes a écrit :

Le 31/12/2016 à 15:48, Guillaume Munch a écrit :

Using weak pointers instead of naked pointers could be used to let you
know when an inset has been destroyed. The cursor could then check if
all slices are valid, and if not then fix itself. This would implement
the original intent that the pointer is "some kind of cache" (according
to a comment) for the slower thing you mention.


We can maybe measure how useful this cache is, and remove it if the
difference is not important (which is probably the case).



Removing the cached pointer requires some thinking as well, since 
CursorSlice has no way of finding the inset on its own.




Re: [LyX/master] TocWidget: fix an erroneous collapse and optimise updates based on profiling

2016-12-31 Thread Guillaume Munch

Le 30/12/2016 à 21:02, Richard Heck a écrit :

On 12/30/2016 12:15 PM, Guillaume Munch wrote:


Thanks for the patch. It is the same as calling fixIfBroken in
GuiView::structureChanged (as attached) which I agree becomes even
simpler. But, provided one accepts the idea that DocIterators hold
perishable pointers that need to be fixed (I do not see any way out of
it), I find it better to fix them the moment we know that they are
broken.


This seems fine to me, though maybe a bit of overkill.


I went for the simpler patch, not because the other is overkill but
because it does not address the real issue anyway.

I went for sanitize instead of fixIfBroken, because the latter truncates
the cursor.




However, I still miss how and when all the cursors not connected
to the GuiView are being fixed, though. Any idea?


I worried about this, too, and I'm not sure they are fixed. One case to
worry
about would be if Scott's file were open in two windows. But my patch still
seems to work. You might want to try this one, too.


It works, but truncates the cursor, so I imagine that fixIfBroken is
called when switching buffer views.




Re: [LyX/master] TocWidget: fix an erroneous collapse and optimise updates based on profiling

2016-12-31 Thread Guillaume Munch

Le 30/12/2016 à 22:48, Jean-Marc Lasgouttes a écrit :

At some time where DEPM was causing issues, there was the idea of
registering cursors with their buffer. This would allow to keep track
of what is happening.

Another possibility would be to register insets, so that one can know
when cursor.inset() is not valid anymore. Or to turn this method into
some slower thing that reads the structures to find the inset.



Using weak pointers instead of naked pointers could be used to let you
know when an inset has been destroyed. The cursor could then check if
all slices are valid, and if not then fix itself. This would implement
the original intent that the pointer is "some kind of cache" (according
to a comment) for the slower thing you mention.

I do not see any other realistic solution. Indeed, non-trivial work
seems to be done in some Inset destructors, which makes any solution
that alters the lifetime of the insets risky.




Re: Memory leak with WordList

2016-12-31 Thread Guillaume Munch

Le 31/12/2016 à 13:16, Jean-Marc Lasgouttes a écrit :

WordList leaks all its contents on exit, which is very impolite (even
though the memory will be reclaimed anyway). This hides real memory
leaks that may happen elsewhere.

Snippet of Valgrind output for stable below. The WordList object created
in theWordList() never gets deleted, although QThreadStorage is supposed
to be automagic.



According to the docs, the GlobalWordLists were deleted with the 
QApplication, that is, before the call to cleanUpWordLists.





Re: dynamic quotes

2016-12-30 Thread Guillaume Munch

Le 30/12/2016 à 23:39, Guenter Milde a écrit :

Dear Jürgen, dear LyX developers,

I am glad to see the "dynamic quotes" and optional arguments to the
quote-insert LFUN are implemented. Now,

  M-x quote-insert * * dynamic

inserts a quote-inset that

* is rendered blue in the GUI, so we can see it is not a "normal" character

* can be changed:
   + on a case-by-case basis via the context menu,
   + globally by changing the "quote style" in Document>Settings>Language.


Currently, there are two ways to activate a keybinding for inserting
"dynamic" quote insets:

a) add or change a keybinding in Tools>Preferences>Editing>Shortcuts

b) change the default behaviour of the "quote-insert" LFUN with the new
   buffer setting "dynamic_quotes"
   (The setting does not change the behaviour of existing quote insets.)

Variant a) works LyX-wide, variant b) document specific.


Now my question: do we need b) or could we keep things more simple.

Is there a use case for the document specific setting?
(I.e. is there someone in need for one keybinding inserting "dynamic"
quote insets in one document but "static" quote insets in another
document?)



Thanks for your detailed explanation. I was wondering the same. I find
the new buffer parameter redundant with a LFUN. One possible answer,
though, is that a buffer parameter helps homogenize a document in the
case of a collaborative work.




Re: [LyX/master] TocWidget: fix an erroneous collapse and optimise updates based on profiling

2016-12-30 Thread Guillaume Munch

Le 30/12/2016 à 17:11, Richard Heck a écrit :

On 12/29/2016 01:21 AM, Guillaume Munch wrote:

Le 27/12/2016 à 23:11, Scott Kostyshak a écrit :

On Tue, May 31, 2016 at 01:15:38AM +0200, Guillaume Munch wrote:

commit 1cc14a31ca8320d881b674f93c34a09cf1666cca
Author: Guillaume Munch <g...@lyx.org>
Date:   Mon May 30 21:42:08 2016 +0100

TocWidget: fix an erroneous collapse and optimise updates based
on profiling

TocModels::reset() in GuiView::structureChanged() collapses the
TocWidget, and
therefore requires an update right after, which was missing.

In fact, profiling TocWidget::updateView() shows that delaying
the update is
good only for fast keypresses (essentially movement). It costs
5% of a
char-forward operation in a document with approx. 100 table of
contents
items. The update optimisation has been rewritten to take this
data into
account.


A git bisect suggests this causes a SIGSEGV. To reproduce:

1. open the attached file
2. open the outline pane
3. put the cursor just to the left of "Hello" (but inside the caption
inset)
4. "Save As" to a new filename.

Can you reproduce?

Backtrace is attached.



Hi,

The root of the issue is in Buffer::reload() called during "Save As".
After loadLyXFile() there, all the insets have been deleted, and
therefore the Inset pointer
GuiView::documentBufferView()->cursor().inset() is dangling. Immediately
after loadLyXFile(), reload() calls updateBuffer() from your backtrace
which then crashes.

While debugging I got other segfaults caused by the same dangling Inset
pointer in Cursor, notably: 1) a trace identical to the second one from
<http://www.lyx.org/trac/ticket/10520>, and 2) a repeated segfault in
the critical path (a call to inMathed()) which closes LyX instantly
after the first segfault is caught (luckily after the emergency save is
complete).

I am trying to find a proper fix for this issue, but the strategy for
managing these Inset pointers in CursorSlice escapes me. Does anyone
know which assumptions are made on when these pointers must be valid?
If there are such assumptions, how are they supposed to be enforced?

The attached patch fixes this crash, but I find very unsatisfactory to
have to update the cursors by hand like this.


I don't know the answer to those questions, but I think that something
like this
solution is all right. The problem is that we would ordinarily suppose
that, after
reading the document, we have a brand new cursor in the open view. But
that's
not true in this case, because the document has been reloaded.

It's probably fine to put the updateCursors() call in readDocument, as
you have
it. But it can also be put into Buffer::reload, right before the call to
updateBuffer.
That's where we actually need it, it seems to me.



Thanks for the patch. It is the same as calling fixIfBroken in
GuiView::structureChanged (as attached) which I agree becomes even
simpler. But, provided one accepts the idea that DocIterators hold
perishable pointers that need to be fixed (I do not see any way out of
it), I find it better to fix them the moment we know that they are
broken. However, I still miss how and when all the cursors not connected
to the GuiView are being fixed, though. Any idea?

diff --git a/src/frontends/qt4/GuiView.cpp b/src/frontends/qt4/GuiView.cpp
index 90b43eb..76911f8 100644
--- a/src/frontends/qt4/GuiView.cpp
+++ b/src/frontends/qt4/GuiView.cpp
@@ -1652,6 +1652,8 @@ void GuiView::updateTocItem(string const & type, DocIterator const & dit)
 
 void GuiView::structureChanged()
 {
+	if (documentBufferView())
+		documentBufferView()->cursor().fixIfBroken();
 	// FIXME: This is slightly expensive, though less than the tocBackend update
 	// (#9880). This also resets the view in the Toc Widget (#6675).
 	d.toc_models_.reset(documentBufferView());


Re: [LyX/master] TocWidget: fix an erroneous collapse and optimise updates based on profiling

2016-12-30 Thread Guillaume Munch

Le 30/12/2016 à 17:05, Jean-Marc Lasgouttes a écrit :

Le 29/12/2016 à 07:21, Guillaume Munch a écrit :

Le 27/12/2016 à 23:11, Scott Kostyshak a écrit :

On Tue, May 31, 2016 at 01:15:38AM +0200, Guillaume Munch wrote:

commit 1cc14a31ca8320d881b674f93c34a09cf1666cca
Author: Guillaume Munch <g...@lyx.org>
Date:   Mon May 30 21:42:08 2016 +0100

TocWidget: fix an erroneous collapse and optimise updates based
on profiling

TocModels::reset() in GuiView::structureChanged() collapses the
TocWidget, and
therefore requires an update right after, which was missing.

In fact, profiling TocWidget::updateView() shows that delaying
the update is
good only for fast keypresses (essentially movement). It costs 5%
of a
char-forward operation in a document with approx. 100 table of
contents
items. The update optimisation has been rewritten to take this
data into
account.


A git bisect suggests this causes a SIGSEGV. To reproduce:

1. open the attached file
2. open the outline pane
3. put the cursor just to the left of "Hello" (but inside the caption
inset)
4. "Save As" to a new filename.

Can you reproduce?

Backtrace is attached.



Hi,

The root of the issue is in Buffer::reload() called during "Save As".
After loadLyXFile() there, all the insets have been deleted, and
therefore the Inset pointer
GuiView::documentBufferView()->cursor().inset() is dangling. Immediately
after loadLyXFile(), reload() calls updateBuffer() from your backtrace
which then crashes.


In general this is when we add a cursor.fixIfBroken() somewhere.


Ok, I saw that DocIterator::sanitize and DocIterator::updateInsets both
fix the pointers, but I missed that fixIfBroken does that too, and
indeed there's a lot of calls to that. Why do we need three different
functions?




The attached patch fixes this crash, but I find very unsatisfactory to
have to update the cursors by hand like this.


What's with all the indirections? Updating directly one cursor seems
more straightforward.


The buffer has no direct access to these cursors, it has to communicate
with the view...




Re: [LyX/master] TocWidget: fix an erroneous collapse and optimise updates based on profiling

2016-12-28 Thread Guillaume Munch

Le 27/12/2016 à 23:11, Scott Kostyshak a écrit :

On Tue, May 31, 2016 at 01:15:38AM +0200, Guillaume Munch wrote:

commit 1cc14a31ca8320d881b674f93c34a09cf1666cca
Author: Guillaume Munch <g...@lyx.org>
Date:   Mon May 30 21:42:08 2016 +0100

TocWidget: fix an erroneous collapse and optimise updates based on profiling

TocModels::reset() in GuiView::structureChanged() collapses the TocWidget, 
and
therefore requires an update right after, which was missing.

In fact, profiling TocWidget::updateView() shows that delaying the update is
good only for fast keypresses (essentially movement). It costs 5% of a
char-forward operation in a document with approx. 100 table of contents
items. The update optimisation has been rewritten to take this data into
account.


A git bisect suggests this causes a SIGSEGV. To reproduce:

1. open the attached file
2. open the outline pane
3. put the cursor just to the left of "Hello" (but inside the caption inset)
4. "Save As" to a new filename.

Can you reproduce?

Backtrace is attached.



Hi,

The root of the issue is in Buffer::reload() called during "Save As".
After loadLyXFile() there, all the insets have been deleted, and
therefore the Inset pointer
GuiView::documentBufferView()->cursor().inset() is dangling. Immediately
after loadLyXFile(), reload() calls updateBuffer() from your backtrace
which then crashes.

While debugging I got other segfaults caused by the same dangling Inset
pointer in Cursor, notably: 1) a trace identical to the second one from
<http://www.lyx.org/trac/ticket/10520>, and 2) a repeated segfault in
the critical path (a call to inMathed()) which closes LyX instantly
after the first segfault is caught (luckily after the emergency save is
complete).

I am trying to find a proper fix for this issue, but the strategy for
managing these Inset pointers in CursorSlice escapes me. Does anyone
know which assumptions are made on when these pointers must be valid?
If there are such assumptions, how are they supposed to be enforced?

The attached patch fixes this crash, but I find very unsatisfactory to
have to update the cursors by hand like this.

Guillaume
diff --git a/src/Buffer.cpp b/src/Buffer.cpp
index 3652e7a..e45efd1 100644
--- a/src/Buffer.cpp
+++ b/src/Buffer.cpp
@@ -1044,6 +1044,9 @@ bool Buffer::readDocument(Lexer & lex)
 	bool const res = text().read(lex, errorList, d->inset);
 	d->old_position.clear();
 
+	// Insets have been deleted; update the inset_ cache of Cursors.
+	updateCursors();
+
 	// inform parent buffer about local macros
 	if (parent()) {
 		Buffer const * pbuf = parent();
@@ -3816,6 +3819,13 @@ ErrorList & Buffer::errorList(string const & type) const
 }
 
 
+void Buffer::updateCursors() const
+{
+	if (d->gui_)
+		d->gui_->updateCursors();
+}
+
+
 void Buffer::updateTocItem(std::string const & type,
 	DocIterator const & dit) const
 {
diff --git a/src/Buffer.h b/src/Buffer.h
index 0cb7026..ba68a45 100644
--- a/src/Buffer.h
+++ b/src/Buffer.h
@@ -682,6 +682,8 @@ public:
 	bool lastPreviewError() const;
 
 private:
+	/// This function is called when inset pointers have changed.
+	void updateCursors() const;
 	///
 	ExportStatus doExport(std::string const & target, bool put_in_tempdir,
 		std::string & result_file) const;
diff --git a/src/BufferView.cpp b/src/BufferView.cpp
index d017db0..81b6d46 100644
--- a/src/BufferView.cpp
+++ b/src/BufferView.cpp
@@ -2447,6 +2447,12 @@ int BufferView::workHeight() const
 }
 
 
+void BufferView::updateCursorInsets()
+{
+	cursor().updateInsets(().inset());
+}
+
+
 void BufferView::setCursor(DocIterator const & dit)
 {
 	d->cursor_.reset();
diff --git a/src/BufferView.h b/src/BufferView.h
index ee2de82..f17b57e 100644
--- a/src/BufferView.h
+++ b/src/BufferView.h
@@ -247,6 +247,8 @@ public:
 	Cursor & cursor();
 	/// access to full cursor.
 	Cursor const & cursor() const;
+	/// update the cursor inset cache
+	void updateCursorInsets();
 	/// sets cursor.
 	/// This will also open all relevant collapsable insets.
 	void setCursor(DocIterator const &);
diff --git a/src/frontends/Delegates.h b/src/frontends/Delegates.h
index d13af7e..0f969e6 100644
--- a/src/frontends/Delegates.h
+++ b/src/frontends/Delegates.h
@@ -72,6 +72,8 @@ public:
 	virtual void setBusy(bool) = 0;
 	/// Reset autosave timers for all users.
 	virtual void resetAutosaveTimers() = 0;
+	/// This function is called when inset pointers have changed.
+	virtual void updateCursors() = 0;
 };
 
 } // namespace frontend
diff --git a/src/frontends/qt4/GuiView.cpp b/src/frontends/qt4/GuiView.cpp
index 90b43eb..8bb699a 100644
--- a/src/frontends/qt4/GuiView.cpp
+++ b/src/frontends/qt4/GuiView.cpp
@@ -1661,6 +1661,15 @@ void GuiView::structureChanged()
 }
 
 
+void GuiView::updateCursors()
+{
+	if (documentBufferView())
+		documentBufferView()->updateCursorInsets();
+	if (currentBufferView() &

Re: master fails to compile with Qt 5.8dev

2016-12-27 Thread Guillaume Munch

Le 25/11/2016 à 05:20, Scott Kostyshak a écrit :

I occassionally build the Qt development version to test various things.

I currently get the following error when building LyX with Qt 5.8dev:

/home/vbox/lyxbuilds/master/repo/src/frontends/qt4/GuiViewSource.cpp:224:14: 
error: ambiguous overload for ‘operator!=’ (operand types are ‘const QChar’ and 
‘char’)
   while (*oc != '\0' && *nc != '\0' && *oc == *nc) {
  ^
Is this a problem with Qt's development version or is it indeed a valid error?



I am not sure, but it is fixed LyX-wise at 6d375dde.





Re: Hardening LyX - AppArmor patch

2016-12-12 Thread Guillaume Munch

Le 11/12/2016 à 01:16, Tommaso Cucinotta a écrit :

Hi,

please, find attached a rework of the AppArmor patch to harden/confine
possible side effects of converters via an AppArmor profile on Linux.

The major challenge here is to ship with a meaningful AA profile -- I'd
be happy to hear feedback about this very point, namely what do we need
to deny-vs-grant access to, while executing external converters. Due to
the fact that converters tend to handle their own temporary and
preference files et al., I'd be for a permissive settings, where we deny
more and more things as we get awareness of potentially sensitive files.


Hi Tommaso,

Thank you for investigating this approach. I have seen that according to
,
AppArmor profiles are meant to be based on white lists instead of
black lists. But I agree with you that writing a white list is going to
be complicated, if only because converters are user-configurable and
AppArmor profiles less user-friendly. This suggests the question, should
the converters themselves not each have an AppArmor profile instead?

Guillaume




Re: #10481: Hardening LyX against potential misuse

2016-12-08 Thread Guillaume Munch

Le 05/12/2016 à 08:53, Tommaso Cucinotta a écrit :

On 04/12/2016 18:51, Guillaume Munch wrote:

Le 04/12/2016 à 18:06, Tommaso Cucinotta a écrit :

On 28/11/2016 00:42, Tommaso Cucinotta wrote:

On 27/11/2016 13:52, Guillaume Munch wrote:

* Converters>Security is located below the converter configuration,
which leads to think that they are converter properties instead of
global settings. What about placing it above the converter list?


Same problem with the Converter Cache option already, isn't it?

Let me propose the attached fix for both at once.


just pushed as [f0f555b5/lyxgit], would be good if anyone could confirm
it shows up as intended ...



How is it supposed to show up? Can you send a screenshot? Here only the
font of "Converters Definition" changed, and I still find it unclear.


Here you go: screenshots before and after the patch attached.

With the "Converter Definitions" label now at the same highlight/logical
level
as "Converter File Cache" and "Security" ones, I think there is no more the
confusionabout which options in the dialog are specific to a single
converter.



Thanks. I see the same. I agree that it is already better.




Re: #10481: Hardening LyX against potential misuse

2016-12-08 Thread Guillaume Munch

Le 05/12/2016 à 08:36, Tommaso Cucinotta a écrit :

On 04/12/2016 18:37, Guillaume Munch wrote:

If there are n graphics, then are there n dialogs when opening the file
for the first time?


it asks as many times as there are (uncached) graphics needing 'needauth'
converters,unless you hit the "Run, and don't ask again for the same doc"
button.



I guess this is better than nothing.

Is this why you wanted the "don't warn again" checkbox on the other dialog?




Re: compilation error with current master

2016-12-08 Thread Guillaume Munch

Le 07/12/2016 à 13:57, Jan Niklas Hasse a écrit :

Hi,

try adding

#include 

to Encoding.cpp.



Yes



On Wed, 7 Dec 2016, at 00:36, Uwe Stöhr wrote:

With today's master I get this compilation error:

Encoding.cpp
D:\LyXGit\Master\src\Encoding.cpp(267): error C3861: 'back_inserter':
identifier not found [D:\LyXGit\Master\compile-2015\src\LyX.vcxproj]



Thanks for the report.





Re: LyX 2.2 slowness

2016-12-08 Thread Guillaume Munch

Le 08/12/2016 à 16:07, Jean-Marc Lasgouttes a écrit :


Also, it would be nice to know what are the callers of freettype that
consume the most time.


As a reminder here is where I got last time with kcachegrind:
https://www.mail-archive.com/lyx-devel@lists.lyx.org/msg194401.html

The caching in Qt5 seems much lower-level than what you have access to,
so it is not impossible that you can no longer get good performance with
Qt4. Let us hope that your patch works.





Re: [LyX/master] Cosmetic changes to the needauth dialogs

2016-12-08 Thread Guillaume Munch

Le 04/12/2016 à 19:59, Enrico Forestieri a écrit :

On Sun, Dec 04, 2016 at 06:38:42PM +0100, Guillaume Munch wrote:


+// FIXME: This dialog has issues with line breaking and size, in particular 
with
+// html. But it could easily be reimplemented as a QMessageBox using
+// QMessageBox::setCheckBox() available starting from Qt 5.2
 class GuiToggleWarningDialog : public QDialog, public Ui::ToggleWarningUi
 {
 public:


If this is true, it would be better to actually reimplement it and
conditionally compile for Qt >= 5.2, otherwise this risks staying so
for ever (or almost so).



I agree.



Re: Crash in stable

2016-12-04 Thread Guillaume Munch

Le 04/12/2016 à 18:48, Enrico Forestieri a écrit :

I am confused now. You make a convoluted patch justifying it by saying
that it was done in view of a proper fix. The simpler one-liner has the
same effect and actually would not be possible if the proper fix was in
place. Given that it is not backported to branch, the simpler fix is
preferable. But I am not going to argue further, as I don't seem to be
able to follow your logic.



And I am not going to argue further, because I gave reasons for why it
is preferable to have the one closest to master out of the two.




Re: #10481: Hardening LyX against potential misuse

2016-12-04 Thread Guillaume Munch

Le 04/12/2016 à 18:06, Tommaso Cucinotta a écrit :

On 28/11/2016 00:42, Tommaso Cucinotta wrote:

On 27/11/2016 13:52, Guillaume Munch wrote:

* Converters>Security is located below the converter configuration,
which leads to think that they are converter properties instead of
global settings. What about placing it above the converter list?


Same problem with the Converter Cache option already, isn't it?

Let me propose the attached fix for both at once.


just pushed as [f0f555b5/lyxgit], would be good if anyone could confirm
it shows up as intended ...



How is it supposed to show up? Can you send a screenshot? Here only the
font of "Converters Definition" changed, and I still find it unclear.



Re: #10481: Hardening LyX against potential misuse

2016-12-04 Thread Guillaume Munch

Le 28/11/2016 à 00:42, Tommaso Cucinotta a écrit :


eh eh, what about remembering 'needauth' (as well as cursor pos) only for
those files in the recent files list :-), and collapse the 3 lists into
a single one, and a single session section ?


Two problems I see with this idea is that migrating the user session is
going to require some work which can be avoided, and also that the three
lists may have different age/length requirements, for instance the
recent files is limited to what you see in the menu.




* Please see the attached patch for a few suggestions


looks great, HTML provides a much better graphics :-), feel free to push !



Done




* Converters>Security is located below the converter configuration,
which leads to think that they are converter properties instead of
global settings. What about placing it above the converter list?


Same problem with the Converter Cache option already, isn't it?

Let me propose the attached fix for both at once.


You made me realise that the converter cache option is indeed global.
So it's good to fix both, thanks.



browse to the Sweave converters, and you'll see
the needauth option in the extraflags, guess we can edit as well from
the dialog, can't we?


Ok, I did not realise.



1. graphics on-screen conversion & display, used with my (local) gnuplot
patch,
   so I insert a .gnuplot image file, and I see its produced output pic
on screen,
   ..., after a few warnings about running needauth converters :-)...


If there are n graphics, then are there n dialogs when opening the file
for the first time?


what else, what are further calls to converters?



Thanks for the explanations, I don't see other calls to converters.

Guillaume




Re: Crash in stable

2016-12-04 Thread Guillaume Munch

Le 04/12/2016 à 17:04, Enrico Forestieri a écrit :


Weak arguments, given that you say the patch was written with the
proper fix in mind (which is not in stable).


It is probably clear to Enrico (I hope) but maybe less to people who did
not follow the discussions and the mentioned commits too closely, that
"proper fix" refers to the pointer becoming invalid. This fix is
only meant to avoid bugs of the same kind in the future. Enrico's
"one-liner" does nothing about it. It is not in stable because nobody
is asking to backport it, because it would be useless there. Then this
fix for a problem B is supposed to be somehow related to the qualities
of a solution to problem A in stable, and if he is serious then I have
to admit that then his argument escapes me.


It would be better if you post your patches to stable for approval

> and comments before committing them.




In fact there was a private exchange with Richard on Friday when I saw
the report, apologies to the list for not being included in the
recipients but I was away without access to my usual git
configuration, and I wanted to spare people time wasted in a duplicate
fix. But from the discussion it was clear to me that Richard intended to
backport 79a947c9 in case it worked.




Re: Crash in stable

2016-12-04 Thread Guillaume Munch

Le 04/12/2016 à 12:25, Enrico Forestieri a écrit :

On Sun, Dec 04, 2016 at 11:57:29AM +0100, Guillaume Munch wrote:


Le 04/12/2016 à 00:57, Enrico Forestieri a écrit :

On Sat, Dec 03, 2016 at 11:59:33PM +0100, Guillaume Munch wrote:


This is the same as
https://www.mail-archive.com/lyx-devel@lists.lyx.org/msg196794.html. I
have now backported the fix.


Hmmm... a quite convoluted patch. There was a one-liner fixing this.



But then you are repeating the discussion above. What point are you
trying to make?


Given two equivalent patches, the simpler is preferable.



The patch which is preferable is the one that deviates the less from
master, because:
 * it is already tested,
 * it causes fewer merge conflicts with future backports.

In this case indeed, the backported code is still in master because it
was written with the proper fix at e0e765f6a98 in mind, as I explained
in the previous discussion.




Re: Crash in stable

2016-12-04 Thread Guillaume Munch

Le 04/12/2016 à 00:57, Enrico Forestieri a écrit :

On Sat, Dec 03, 2016 at 11:59:33PM +0100, Guillaume Munch wrote:


This is the same as
https://www.mail-archive.com/lyx-devel@lists.lyx.org/msg196794.html. I
have now backported the fix.


Hmmm... a quite convoluted patch. There was a one-liner fixing this.



But then you are repeating the discussion above. What point are you
trying to make?



Re: Crash-Reporting on Mac

2016-12-03 Thread Guillaume Munch

Le 30/11/2016 à 16:48, Jean-Marc Lasgouttes a écrit :


a useless backtrace




If you have the symbols at hand, then you can get them by hand. For
instance if you have compiled in debug mode, then you can run gdb as you
do usually, only after the fact, and get the symbols with "info symbols
0x12345".

Traces sent by users could become much more useful if we kept the
symbols from the releases (e.g. compile with symbols and then distribute
stripped binaries). I wonder if somebody else knows simpler ways to
exploit the traces.




Re: Crash in stable

2016-12-03 Thread Guillaume Munch

Le 02/12/2016 à 01:08, Richard Heck a écrit :

On 12/01/2016 06:44 PM, Enrico Forestieri wrote:

On Thu, Dec 01, 2016 at 07:50:19PM +0100, Enrico Forestieri wrote:


This only occurs with 2.2.3dev (not with 2.2.2, nor with 2.3.0dev):

1) Start lyx, make sure the source pane is closed
2) File->New
3) View->Source Pane
4) View->Source Pane
5) File->Close
6) File->New
7) View->Source Pane

Now lyx segfaults:

Bisect points at a36706c3.


Thanks for finding it. Here's the backtrace again:

#0  0x75f3f280 in std::string::empty() const ()
   from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#1  0x00511413 in lyx::BufferParams::getDefaultOutputFormat (this=0x8)
at ../../src/BufferParams.cpp:2524
#2  0x00510f7a in lyx::BufferParams::getOutputFlavor (this=0x8,
format="default") at ../../src/BufferParams.cpp:2474
#3  0x0097b121 in lyx::frontend::ViewSourceWidget::currentFormatName (
this=0x1b73930) at ../../../../src/frontends/qt4/GuiViewSource.cpp:316
#4  0x0097c38e in lyx::frontend::GuiViewSource::updateTitle (
this=0x1b5d880) at ../../../../src/frontends/qt4/GuiViewSource.cpp:456
#5  0x0097c34e in lyx::frontend::GuiViewSource::initialiseParams (
this=0x1b5d880) at ../../../../src/frontends/qt4/GuiViewSource.cpp:449

I suspect that the problem is that GuiViewSource::bv_ is invalid.



This is the same as
https://www.mail-archive.com/lyx-devel@lists.lyx.org/msg196794.html. I
have now backported the fix.




Re: Qt assertion when trying to view "Hangul Syllables" in special character insertion

2016-12-03 Thread Guillaume Munch

Le 02/12/2016 à 08:03, Scott Kostyshak a écrit :

On Sat, Nov 19, 2016 at 12:41:13AM +0100, Guillaume Munch wrote:


I cannot reproduce with the provided info (2.3dev & Qt5).


A bisect led to b3bed292. The attached patch fixes it for me, although I
don't understand my patch.


Hello Scott,

Nothing too complicated really. An update to lib/unicodesymbols had
introduced characters outside LyX's known blocks (specifically ꜜ and ꜛ
in "Modifier Tone Letters"). Then another bug in the char->block
assignment placed these characters in the next known block, "Hangul
Syllables", so GuiSymbols thought the encoding had Hangul Syllables. But
when you asked for Hangul Syllables, the block->chars assignment did its
job correctly and said there were actually none. So GuiSymbols tried to
initialize a 0-size model for Hangul Syllables. At this point, the
assumption 0 <= symbols.size()-1 is indeed wrong (though it does not
cause the bad access by itself). Your particular bug was fixed by
correcting the char->block assignment. However there were other means to
trigger symbols.size()==0, e.g. when the encoding changes by moving the
cursor, a third issue. There was also an unrelated bad access when
trying to show the tooltip after the end of the list. These bugs are now
fixed, except for the missing names and ranges of the new blocks.


What was the point of subtracting 1 from
symbols.size()?



This is according to the documentation:
https://doc.qt.io/qt-5/qabstractitemmodel.html#beginInsertRows
Inserting from 0 to n informs the view that n+1 lines are inserted. But,
its place is after endResetModel.

Guillaume



Re: #10481: Hardening LyX against potential misuse

2016-11-27 Thread Guillaume Munch

Hi Tommaso,


I have been following your work on this issue with interest. Thank you,
this is something that was much needed. Making AppArmor work would be
great too, but I suspect that it is going to be hard to have a
configuration which is both secure and without hassle to the user,
especially since with security there can be no half-measure, and
unrestricted read access can be an issue too. Still any experiment with
it is good. Further comments below.

Le 23/11/2016 à 22:27, Tommaso Cucinotta a écrit :

One note: one way to avoid the [auth session] section growing unbounded,
might be to have an expiry timestamp, so that e.g., authorizations would
expire in ~1y or so. This might be done with a section syntax like:

   

instead of the simple  we have now.


Could that be used to fix #10310 as well?
See https://www.mail-archive.com/lyx-devel@lists.lyx.org/msg195888.html



Any further comment welcome, thanks!



* Please see the attached patch for a few suggestions (more concise, a
few corrections, html formatting, no checkbox). (Using html much
improves readability, but then the html appears in the console output. I
have a solution for this if you choose this route.)

* Please try to avoid lines longer than 80 characters.

* "dangerous, including deleting files": it has been a while that
computer virus are created to do much worse than deleting files...

* Converters>Security is located below the converter configuration,
which leads to think that they are converter properties instead of
global settings. What about placing it above the converter list?

* It would be nice to be able to edit the needauth flag from the
converters settings.

* What happens if a needauth converter is called during instant preview?
Could that happen? And during graphics display?

Thanks again.

Guillaume
diff --git a/src/Converter.cpp b/src/Converter.cpp
index 14b18dd..e596b3d 100644
--- a/src/Converter.cpp
+++ b/src/Converter.cpp
@@ -283,29 +283,40 @@ bool Converters::checkAuth(Converter const & conv, string const & doc_fname)
 {
 	if (!conv.need_auth())
 		return true;
-	const docstring security_warning = bformat(_("Requested operation needs use of converter '%1$s' from %2$s to %3$s, "
-		"which is tagged with the 'needauth' option. This is an external program normally acting as a picture/format "
-		"converter, but which is known to be able to execute arbitrary actions on the system on behalf of the user, "
-		"including dangerous ones such as deleting files, if instructed to do so by a maliciously crafted .lyx document."),
-		from_utf8(conv.command()), from_utf8(conv.from()), from_utf8(conv.to()));
+	const docstring security_warning = bformat(
+	  _("The requested operation requires the use of a converter from "
+	"%2$s to %3$s:"
+	"%1$s"
+	"This external program can execute arbitrary commands on your "
+	"system, including dangerous ones, if instructed to do so by a "
+	"maliciously crafted .lyx document."),
+	  from_utf8(conv.command()), from_utf8(conv.from()),
+	  from_utf8(conv.to()));
 	if (lyxrc.use_converter_needauth_forbidden) {
-		frontend::Alert::warning(_("Launch of external converter is forbidden"), security_warning + _("\n\n"
-			"This is forbidden by default. In order to unlock execution of these converters, please, go to\n"
-			"Preferences->File Handling->Converters and uncheck Security->Forbid needauth converters."), true);
+		frontend::Alert::warning(
+		_("An external converter is disabled for security reasons"),
+		security_warning + _(
+		"Your current settings forbid its execution."
+		"(To change this setting, go to Preferences  File "
+		"Handling  Converters and uncheck Security  "
+		"Forbid needauth converters.)"), false);
 		return false;
 	}
 	if (!lyxrc.use_converter_needauth)
 		return true;
-	static const docstring security_title = _("Launch of external converter needs user authorization");
+	static const docstring security_title =
+		_("An external converter requires your authorization");
 	int choice;
-	const docstring security_warning2 = security_warning + _("\n\nWould you like to run the converter?\n\n"
-		"ANSWER RUN ONLY IF YOU TRUST THE ORIGIN/SENDER OF THE LYX DOCUMENT!");
+	const docstring security_warning2 = security_warning +
+		_("Would you like to run this converter?"
+		  "Only run if you trust the origin/sender of the LyX "
+		  "document!");
 	if (!doc_fname.empty()) {
 		LYXERR(Debug::FILES, "looking up: " << doc_fname);
 		std::set & auth_files = theSession().authFiles().authFiles();
 		if (auth_files.find(doc_fname) == auth_files.end()) {
 			choice = frontend::Alert::prompt(security_title, security_warning2,
-0, 0, _("Do  run"), _(""), _(" run for this document"));
+0, 0, _("Do  run"), _(""), _(" run for this document"));
 			if (choice == 2)
 auth_files.insert(doc_fname);
 		} else {
@@ -313,7 +324,7 @@ bool Converters::checkAuth(Converter const & conv, string const & doc_fname)
 		}

Re: #10481: Hardening LyX against potential misuse

2016-11-27 Thread Guillaume Munch

Le 25/11/2016 à 20:50, Scott Kostyshak a écrit :

On Fri, Nov 25, 2016 at 02:32:37PM -0500, Scott Kostyshak wrote:


I think the line-breaking in the warning dialog should be improved. The
horizontal width is larger than my 13 in. screen. See attached.


Note that the linebreaking on the *other* warning (the Run/ Do Not run)
is fine.


This dialog is custom (GuiToggleWarningDialog). All other messages
dialogs use QMessageBox. GuiToggleWarningDialog is essentially a
QMessageBox::warning with an added checkbox.

Starting from Qt5.2, QMessageBox supports adding a checkbox
(QMessageBox::setCheckBox). I suggest reimplementing
GuiToggleWarningDialog as a QMessageBox, for consistency and to fix the
issue. One will need an #if QT_VERSION >=... directive though.




Also regarding the "first" warning (the "Launch of external converter is
forbidden" warning, which is the one I'm referring to above), although I
have not closely followed the conversation I have the following comment:

I think it should be converted to an error and/or I don't think there
should be a checkbox "Do not show this warning again".


I agree. Note that this also fixes the the line breaking issue since 
QMessageBox is used instead of GuiToggleWarningDialog in that case. (But 
it would still be good to fix GuiToggleWarningDialog.)


Guillaume



Re: Remove templatization from bformat() ?

2016-11-26 Thread Guillaume Munch

Le 24/11/2016 à 00:33, Tommaso Cucinotta a écrit :

Hi,

while playing with strings, I've just run into a mysterious linking
error [1] which, after a bit of digging, turned out to be decrypted as
[2]. Basically, [2] is obtained by removing all templatization code for
bformat() in lstrings.h. All template implementations are completely
specialized on specific types, and do NOT make any actual use of the
template machinery (indeed, LyX compiles and links :-) ). Leaving aside
the specific error of the code snippet, as [2] is way more readable than
[1], I'd be totally in favor of pushing such a patch, which I'm
attaching. Any thoughts?


Hi Tommaso,

Looks good. Either that, or somebody with enough motivation implements
bformat in full generality which seems to have been the original
intention (see e.g. 6a55be95). This would have been another solution to 
your linking issue.




Re: [LyX/master] Citation dialog redesign

2016-11-25 Thread Guillaume Munch

Le 25/11/2016 à 21:04, Scott Kostyshak a écrit :

I think that "Search as you type" should be default because (1) this is
consistent with similar dialogs such as the cross-reference dialog and
the search in LyX preferences/document settings and (2) there is no
"search" button as there was in the previous dialog design.


Jürgen implemented it as default at 7d4292918. But this only affects new
installations. Anyone who had it disabled must enable it explicitly.



Re: [LyX/master] Improve fractions bar

2016-11-21 Thread Guillaume Munch

Le 22/11/2016 à 00:32, Guillaume Munch a écrit :

Le 21/11/2016 à 17:33, Jean-Marc Lasgouttes a écrit :


* did you try to look at what TeX does and see what can be picked from
there? The relevant rules are 15(a-e). It is not funny reading, but last
time I looked I conlcuded that part of that could be implemented.


Thanks for the reference. I used to have a TeXbook, but books made of
paper are not easy to move around, so it ended up being given away
before it could be of any use. Do you have another source for this
algorithm?



Never mind, I found it. Now, did you have a plan for implementing the
various \fontdimen in LyX ? (see the hack I had to resort to just for
determining the math axis...)




Re: [LyX/master] Improve fractions bar

2016-11-21 Thread Guillaume Munch

Le 21/11/2016 à 17:33, Jean-Marc Lasgouttes a écrit :


It is nice to give some love to \frac (it was somewhere on my to do
list). I have not tried it yet, but I have questions:

* why not use FontMetrics::lineWidth for the witdth of the line? You do
not want to use the same width for a fraction in textstyle or in
scriptstyle.


First because TeX itself always uses the same thin line whatever the
font size. Also, following your suggestion I tried, and you can get 1px
lines next to 2px lines, so the result is not very nice.



* did you try to look at what TeX does and see what can be picked from
there? The relevant rules are 15(a-e). It is not funny reading, but last
time I looked I conlcuded that part of that could be implemented.


Thanks for the reference. I used to have a TeXbook, but books made of
paper are not easy to move around, so it ended up being given away
before it could be of any use. Do you have another source for this
algorithm?



Re: [LyX/master] mathedSymbolDim only needs a MathBase

2016-11-21 Thread Guillaume Munch

Le 21/11/2016 à 17:35, Jean-Marc Lasgouttes a écrit :


Somewhere on my todo list I wanted to make MetricsInfo inherit from
MetricsBase, in order to avoid this stupid mi.base thing. Do you see a
reason not to do it?



Well, do you see a reason to do it? What is wrong with composition in
this case?




Re: [LyX/master] Let math mu skips scale with zoom

2016-11-21 Thread Guillaume Munch

Le 21/11/2016 à 17:28, Jean-Marc Lasgouttes a écrit :

Do you really see a difference with this patch? What makes you thing
that theFontMetrics(font).em() does not scale with zoom? Looking at the
code, the way zoom is taken in account on this code path in Length is
pretty ugly.

I find the previous code was more straightforward.


Sorry for overselling this commit. At first the goal was only to
remove duplicate code, and later I saw the (zoom * dpi)
business. I only realize now that it has no impact.



Re: [LyX/master] On-screen justification: stretch in proportion with the em, up to a limit

2016-11-21 Thread Guillaume Munch

Le 21/11/2016 à 11:56, Jean-Marc Lasgouttes a écrit :

Le 21/11/2016 à 11:49, Guillaume Munch a écrit :

Le 21/11/2016 à 11:45, Jean-Marc Lasgouttes a écrit :


I agree, but I could not figure it out.


Do you have a MWE?


That's the point, I don't. I can add a terminal error message to help
find one in the future if you think it is going to help.


Or an assertion maybe?



I do not think it is rare, only hard to reproduce.



Re: [LyX/master] On-screen justification: stretch in proportion with the em, up to a limit

2016-11-21 Thread Guillaume Munch

Le 21/11/2016 à 11:45, Jean-Marc Lasgouttes a écrit :


I agree, but I could not figure it out.


Do you have a MWE?


That's the point, I don't. I can add a terminal error message to help
find one in the future if you think it is going to help.




Re: [LyX/master] On-screen justification: stretch in proportion with the em, up to a limit

2016-11-21 Thread Guillaume Munch

Le 21/11/2016 à 09:06, Scott Kostyshak a écrit :

On Mon, Nov 21, 2016 at 01:04:10AM +0100, Guillaume Munch wrote:


But with your recipe I got the attached (unrelated) segfault when
closing LyX.


A bisect led me to fb264663.


Thank you for doing that.




Re: [LyX/master] On-screen justification: stretch in proportion with the em, up to a limit

2016-11-21 Thread Guillaume Munch

Le 21/11/2016 à 10:02, Jean-Marc Lasgouttes a écrit :


Can you expand on when w<0 happens? I think that fixing problems (other
than with an explicit workaround) is often a better idea in the long run.



I agree, but I could not figure it out.




Re: [LyX/master] On-screen justification: stretch in proportion with the em, up to a limit

2016-11-20 Thread Guillaume Munch

Le 21/11/2016 à 00:51, Guillaume Munch a écrit :

Le 20/11/2016 à 22:47, Scott Kostyshak a écrit :


After this commit I get an assertion.

To reproduce:

1. Open lib/doc/Math.lyx
2. ctrl+a to select all.
3. Insert > Branch > Insert New Branch. Call it "hello" and press OK.



Hi Scott,

Unfortunately I cannot reproduce.

But with your recipe I got the attached (unrelated) segfault when
closing LyX.


Sorry for the mix-up, only the last trace is relevant.
#0  0x75acc4f5 in malloc_consolidate (
av=av@entry=0x75e11b20 ) at malloc.c:4184
#1  0x75acf55e in _int_malloc (av=av@entry=0x75e11b20 , 
bytes=bytes@entry=62948305) at malloc.c:3451
#2  0x75ad15a4 in __GI___libc_malloc (bytes=62948305) at malloc.c:2914
#3  0x763c4e78 in operator new(unsigned long) ()
   from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#4  0x00486305 in __gnu_cxx::new_allocator::allocate (
__n=, this=)
at /usr/include/c++/5/ext/new_allocator.h:104
#5  std::allocator_traits<std::allocator >::allocate (
__n=, __a=...) at /usr/include/c++/5/bits/alloc_traits.h:491
#6  std::__cxx11::basic_string<char, std::char_traits, 
std::allocator >::_M_create (this=this@entry=0x7fffcfd0, 
__capacity=__capacity@entry=@0x7fffcf80: 62948304, 
__old_capacity=__old_capacity@entry=0)
at /usr/include/c++/5/bits/basic_string.tcc:157
#7  0x0048773c in std::__cxx11::basic_string<char, 
std::char_traits, std::allocator >::_M_construct<char*> 
(this=this@entry=0x7fffcfd0, 
__beg=0x3e30990 "`", __end=)
at /usr/include/c++/5/bits/basic_string.tcc:223
#8  0x00cf4d6a in std::__cxx11::basic_string<char, 
std::char_traits, std::allocator >::_M_construct_aux<char*> 
(__end=, 
__beg=, this=0x7fffcfd0)
at /usr/include/c++/5/bits/basic_string.h:195
#9  std::__cxx11::basic_string<char, std::char_traits, 
std::allocator >::_M_construct<char*> (__end=, 
__beg=, 
this=0x7fffcfd0) at /usr/include/c++/5/bits/basic_string.h:214
#10 std::__cxx11::basic_string<char, std::char_traits, 
std::allocator >::basic_string (__str=..., this=0x7fffcfd0)
at /usr/include/c++/5/bits/basic_string.h:400
#11 lyx::support::FileName::absFileName[abi:cxx11]() const (
this=this@entry=0x33b2a00) at ../../../src/support/FileName.cpp:181
#12 0x00cf5d73 in lyx::support::operator< (lhs=..., rhs=...)
at ../../../src/support/FileName.cpp:852
#13 0x00cab954 in std::less::operator() (
__y=..., __x=..., this=0x292e288)
at /usr/include/c++/5/bits/stl_function.h:387
#14 std::_Rb_tree<lyx::support::FileName, std::pair >, 
std::_Select1st<std::pair > >, 
std::less, 
std::allocator<std::pair > > >::_M_lower_bound (__k=..., 
__y=0x292e290, __x=0x33b29e0, this=0x292e288)
at /usr/include/c++/5/bits/stl_tree.h:1628
#15 std::_Rb_tree<lyx::support::FileName, std::pair >, 
std::_Select1st<std::pair > >, 
std::less, 
std::allocator<std::pair > > >::find 
(this=this@entry=0x292e288, 
__k=...) at /usr/include/c++/5/bits/stl_tree.h:2295
#16 0x00caaccc in std::__cxx1998::map<lyx::support::FileName, 
std::shared_ptr, std::less, 
std::allocator<std::pair > > >::find (__x=..., this=0x292e288) 
at /usr/include/c++/5/bits/stl_map.h:846
#17 std::__debug::map<lyx::support::FileName, 
std::shared_ptr, std::less, 
std::allocator<std::pair > > >::find (
__x=..., this=0x292e270) at /usr/include/c++/5/debug/map.h:413
#18 lyx::graphics::Cache::remove (
this=0x16ae0f8 <lyx::graphics::Cache::get()::singleton>, file=...)
at ../../src/graphics/GraphicsCache.cpp:119
#19 0x00cc6175 in lyx::graphics::Loader::Impl::resetFile (
this=this@entry=0x6f65b10, file=...)
at ../../src/graphics/GraphicsLoader.cpp:393
#20 0x00cc66b8 in lyx::graphics::Loader::Impl::~Impl (this=0x6f65b10, 
__in_chrg=) at ../../src/graphics/GraphicsLoader.cpp:370
#21 0x00cc6760 in lyx::graphics::Loader::~Loader (this=0x6f65a98, 
__in_chrg=) at ../../src/graphics/GraphicsLoader.cpp:243
#22 0x00a33eee in lyx::RenderGraphic::~RenderGraphic (this=0x6f65a80, 
__in_chrg=) at ../../src/insets/RenderGraphic.h:24
#23 lyx::RenderGraphic::~RenderGraphic (this=0x6f65a80, 
__in_chrg=) at ../../src/insets/RenderGraphic.h:24
#24 0x00946a2e in lyx::InsetGraphics::~InsetGraphics (this=0x6f65740, 
__in_chrg=) at ../../src/insets/InsetGraphics.cpp:196
#25 0x00946af7 in lyx::InsetGraphics::~InsetGraphics (this=0x6f65740, 
__in_chrg=) at ../../src/insets/InsetGraphics.cpp:197
#26 0x007d3786 in lyx::InsetList::~InsetList (this=0x6f65598, 
__in_chrg=) at ../../src/InsetList.cpp:74
#27 0x006c1736 in lyx::Paragraph::Private::~Private (this=0x6f65480, 
__in_chrg=) at ../../src/Paragraph.cpp:287
#28 lyx::Paragraph::~Paragraph (this

Re: [LyX/master] On-screen justification: stretch in proportion with the em, up to a limit

2016-11-20 Thread Guillaume Munch

Le 20/11/2016 à 22:47, Scott Kostyshak a écrit :


After this commit I get an assertion.

To reproduce:

1. Open lib/doc/Math.lyx
2. ctrl+a to select all.
3. Insert > Branch > Insert New Branch. Call it "hello" and press OK.



Hi Scott,

Unfortunately I cannot reproduce.

But with your recipe I got the attached (unrelated) segfault when
closing LyX.



I get:

support/lassert.cpp (51): ASSERTION x > -100 VIOLATED IN 
/home/scott/lyxbuilds/master/repo/src/Dimension.cpp:31

Backtrace is attached.


Thank you for the trace. Does the attached patch help?



Note that on some commits (I think the "good" commits) during the bisect I got 
the terminal messages below. I have no idea if this issue is related.

mathed/MathStream.cpp (718): Unable to find `\{' in the mathWordList.
mathed/MathStream.cpp (718): Unable to find `\}' in the mathWordList.



I have no clue about this.

Guillaume

diff --git a/src/Row.cpp b/src/Row.cpp
index 5f2bd2f..5e73ccc 100644
--- a/src/Row.cpp
+++ b/src/Row.cpp
@@ -329,8 +329,8 @@ int Row::countSeparators() const
 
 bool Row::setExtraWidth(int w)
 {
-	if (w < 0)
-		// this is not expected to happen (but it does)
+	if (w <= 0)
+		// w < 0 is not expected to happen (but it does)
 		return false;
 	// amount of expansion: number of expanders time the em value for each
 	// string element
#0  0x75a83418 in __GI_raise (sig=sig@entry=6)
at ../sysdeps/unix/sysv/linux/raise.c:54
#1  0x75a8501a in __GI_abort () at abort.c:89
#2  0x0063f3aa in lyx::lyx_exit (exit_code=exit_code@entry=1)
at ../../src/LyX.cpp:250
#3  0x0078612b in boost::assertion_failed (expr=0xd8e6b2 "false", 
function=0x12d00e0  
"void lyx::doAssertWithCallstack(bool)", 
file=0x12cff70 "../../../src/support/lassert.cpp", line=44)
at ../../src/boost.cpp:47
#4  0x00d05497 in lyx::doAssertWithCallstack (value=value@entry=false)
at ../../../src/support/lassert.cpp:44
#5  0x00d0556e in lyx::doAssert (
expr=expr@entry=0xd9a64b "lockCount_ >= 0", 
file=file@entry=0xd9a5a0 "../../src/mathed/MacroTable.cpp", 
line=line@entry=146) at ../../../src/support/lassert.cpp:53
#6  0x00835b05 in lyx::MacroData::unlock (this=0x2fcfb70)
at ../../src/mathed/MacroTable.cpp:146
#7  0x00867222 in lyx::ArgumentProxy::metrics (this=0x2fa6420, mi=..., 
dim=...) at ../../src/mathed/MathMacro.cpp:72
#8  0x0083ebdf in lyx::MathData::metrics (this=, mi=..., 
dim=...) at ../../src/mathed/MathData.cpp:268
#9  0x0089de12 in lyx::InsetMathDelim::metrics (this=0x3b21280, mi=..., 
dim=...) at ../../src/mathed/InsetMathDelim.cpp:108
#10 0x0083ebdf in lyx::MathData::metrics (this=, mi=..., 
dim=...) at ../../src/mathed/MathData.cpp:268
#11 0x008b36cd in lyx::InsetMathGrid::metrics (
this=this@entry=0x36a2430, mi=..., dim=...)
at ../../src/mathed/InsetMathGrid.cpp:400
#12 0x0080dc11 in lyx::InsetMathHull::metrics (this=0x36a2430, mi=..., 
dim=...) at ../../src/mathed/InsetMathHull.cpp:498
#13 0x00742b49 in lyx::TextMetrics::redoParagraph (
this=this@entry=0x427e498, pit=154) at ../../src/TextMetrics.cpp:432
#14 0x00791ba4 in lyx::BufferView::updateMetrics (
this=this@entry=0x3286a70) at ../../src/BufferView.cpp:2737
#15 0x0079547d in lyx::BufferView::processUpdateFlags (
this=this@entry=0x3286a70, flags=lyx::Update::Force)
at ../../src/BufferView.cpp:488
#16 0x007959e5 in lyx::BufferView::mouseEventDispatch (this=0x3286a70, 
cmd0=...) at ../../src/BufferView.cpp:2280
#17 0x00a94524 in lyx::frontend::GuiWorkArea::Private::dispatch (
this=0x39c8010, cmd=...)
at ../../../../src/frontends/qt4/GuiWorkArea.cpp:550
#18 0x00a947f8 in lyx::frontend::GuiWorkArea::mousePressEvent (
this=0x1f342a0, e=0x7fffced0)
at ../../../../src/frontends/qt4/GuiWorkArea.cpp:831
#19 0x772b0e7f in QWidget::event(QEvent*) ()
   from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#20 0x773abcae in QFrame::event(QEvent*) ()
   from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#21 0x76939192 in 
QCoreApplicationPrivate::sendThroughObjectEventFilters(QObject*, QEvent*) () 
from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#22 0x7726c1b5 in QApplicationPrivate::notify_helper(QObject*, QEvent*)
() from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#23 0x77271bc1 in QApplication::notify(QObject*, QEvent*) ()
   from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#24 0x00a3e79a in lyx::frontend::GuiApplication::notify (
this=0x171cce0, receiver=, event=)
at ../../../../src/frontends/qt4/GuiApplication.cpp:2699
#25 0x76939428 in QCoreApplication::notifyInternal2(QObject*, QEvent*)
() from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#26 0x77270995 in QApplicationPrivate::sendMouseEvent(QWidget*, 
QMouseEvent*, QWidget*, 

Re: Qt assertion when trying to view "Hangul Syllables" in special character insertion

2016-11-18 Thread Guillaume Munch

Le 19/11/2016 à 00:16, Richard Heck a écrit :

On 11/18/2016 04:01 PM, Scott Kostyshak wrote:

If I go to Insert > Special Character > Symbols...
and then go to "Hangul Syllables", I get an assertion.

On Qt 5 with custom-compiled Qt:
ASSERT: "last >= first" in file
/usr/src/qt/qt5/qtbase/src/corelib/itemmodels/qabstractitemmodel.cpp,
line 2604

With Qt 4:
ASSERT failure in QList::at: "index out of range", file
/usr/include/qt4/QtCore/qlist.h, line 469

Can anyone else reproduce?


No, not with Qt 4, anyway, though I have no symbols that appear in this
case.



The symbols that appear there depend on the output encoding. I see all 
symbols when setting Unicode (XeTeX). Strangely, they do not appear when 
checking Use non-TeX fonts.


I cannot reproduce with the provided info (2.3dev & Qt5).



Re: New warning on master

2016-11-09 Thread Guillaume Munch




I will gladly let you handle everything, then :)




Done.



Re: New warning on master

2016-11-09 Thread Guillaume Munch

Le 09/11/2016 à 20:46, Enrico Forestieri a écrit :

On Wed, Nov 09, 2016 at 11:46:33AM +0100, Jean-Marc Lasgouttes wrote:

When using dialogs (prefs for example) with master, I get some

QMetaObject::connectSlotsByName: No matching signal for
on_bufferViewChanged()
QMetaObject::connectSlotsByName: No matching signal for
on_bufferViewChanged()
QMetaObject::connectSlotsByName: No matching signal for
on_bufferViewChanged()

Does this ring a bell for somebody?


I think the explanation is here:
http://www.qtforum.org/post/76950/connectslotsbyname.html


The original intention was indeed to use auto-connect, and despite that
it did not do what I wanted I kept the name. According to the manual
(both qt4 and qt5) the syntax is on__ so it
was not supposed to be triggered. Then the explanation would be that qt5
just behaves closer to the documentation.



Qt interprets bufferViewChanged as an object name but doesn't find
any signal name. As we always manually connect signals and slots,
we should avoid having slots whose name begins with "on_".


I agree that it is better to avoid naming slots with "on_" unless an
auto-connection is intended. However, the LyX source do not always
connect manually: auto-connect is actually used (correctly) in some
places, in particular with objects defined in ui files.



The attached patch simply renames on_bufferViewChanged as
onBufferViewChanged (other than fixing an oversight in GuiView.cpp)
and gets rid of the warning. I don't know why this does not occur
on Qt5 or why it does not occur for all other on_XXX slots.



Patch looks good, feel free to commit. Except for:


QObject::connect(wa, SIGNAL(bufferViewChanged()),
-this, SIGNAL(bufferViewChanged()));
+this, SLOT(onBufferViewChanged()));


The original code is intended (see e.g.
https://stackoverflow.com/q/9798195), and the replacement does not
achieve the same. But since this misleads into thinking that there was a
typo, I will add a comment.


Guillaume



Re: New warning on master

2016-11-09 Thread Guillaume Munch

Le 09/11/2016 à 15:58, Jean-Marc Lasgouttes a écrit :

Le 09/11/2016 à 15:54, Scott Kostyshak a écrit :

On Wed, Nov 09, 2016 at 11:46:33AM +0100, Jean-Marc Lasgouttes wrote:

When using dialogs (prefs for example) with master, I get some



Does this ring a bell for somebody?


I don't see this with current master and Qt 5.6.2. To test, I just
started list and opened preferences and clicked around a few times.


It is indeed Qt4-only and does not appear on stable. Guillaume, an idea?



I'll have a look.




Re: [patch] On-screen justification: stretch in proportion with the em, up to a limit

2016-11-06 Thread Guillaume Munch

Le 06/11/2016 à 16:11, Jean-Marc Lasgouttes a écrit :

Something I meant to mention: shall we add some on screen cue that the
row has not been completely stretched ?

JMarc


Most of the times it is already clear from the other lines that the text 
is justified. In addition, on-screen justification cannot be much more 
than cosmetic anyway, since there is an option to disable it. The goal 
is to increase readability, and additional clues would only clutter, in 
my opinion.




Re: [patch] On-screen justification: stretch in proportion with the em, up to a limit

2016-11-06 Thread Guillaume Munch

Le 29/08/2016 à 14:16, Jean-Marc Lasgouttes a écrit :


The patch looks good.


It's in at b30f8d3c4b



Re: please revert: [LyX/2.2.x] Better title for ViewSource

2016-10-25 Thread Guillaume Munch

Le 24/10/2016 à 23:55, Uwe Stöhr a écrit :

Am 24.10.2016 um 00:38 schrieb Guillaume Munch:


You are not being rude at all, in fact you are quite polite.


OK. I felt I was rude. I thought about it and began to update the docs
when I realized what an impact these renaming have. Then I searched the
Wiki and googled around. The source code window is referenced all over
the place a lot. So changing this produces a lot of work for me, breaks
the idea that the docs for LyX 2.2.x are valid for all 2.2 releases
(except bugfixes of course) and finally I didn't see a benefit for the
renaming at all.


Sorry for all the work. It helps to see it as lifting a lot of small
confusions all over the place. I can also help with the change (maybe it
won't be needed if Jean-Pierre's magical solution works).



So if you think the changed name is an improvement and the others agree,
I am fine with this change in master but not for branch.


I can revert the menu name to "Source Pane" in 2.2.x for the
documentation reasons you gave.


Please do so.


Done.




I can also revert it in master if
the majority wants it. But I hope that my explanation reassured
you.


If your experience is that the new name helps users, it is OK, I mean
many things I do here are also driven from user feedback and are perhaps
sometimes hard to understand by developers. For me user feedback is very
important. So +1 from me.


Of course what I can only say is that the old name did not help.


I don't know if there was already a vote. if
not, I suggest to do one because this menu is used quite often and
better to decide now than later in the RC cycle.


Yes, if there are dissenting opinions, now is the time. And then if
rationally it cannot be decided, then it might be needed to resort to a
vote.

Guillaume



Re: [LyX/master] Correctly track ulem commands with change tracking

2016-10-23 Thread Guillaume Munch

Le 24/10/2016 à 01:00, Enrico Forestieri a écrit :

On Sun, Oct 23, 2016 at 11:52:55PM +0200, Guillaume Munch wrote:


The code does look fragile to me. I do not think that asking that
developers care about maintainability is being overzealous. Then, maybe
I am mistaken about the code and you got to something found maintainable
enough after thinking about it a lot, such that you did not feel the
need asking for advice. In that case, maybe all the misunderstandings
comes indeed from a different appreciation of what effort is asked.

But to know this we would need to speak about the code. Every time I
want to discuss your commits, I know that you are going to take things
personally, to the point that sometimes I just prefer to let it off
before even asking. I do not know another LyX developer who reacts like
you do.


It's not what you say but how you say it.


Before arguing that I am not the official maintainer, ask yourself who
was the only one who tested several of your big changes and spent a lot
of time writing detailed testcases (lots of them!)? And where would LyX
be if I had not?


I think you have an enormous ego and presumptuous manners.
I have nothing more to add to this discussion. Good bye.



Yeah, I see how this comes across, this is not what I meant, does not
correspond to what I think, and I regret writing this. The discussion
went nowhere I wanted and I regret starting it. I will no longer send
such messages to the list in the future.




Re: please revert: [LyX/2.2.x] Better title for ViewSource

2016-10-23 Thread Guillaume Munch

Le 23/10/2016 à 23:20, Uwe Stöhr a écrit :

commit a36706c3ffe9570588962a5ad3206d57e63ffcfd
Author: Guillaume Munch <g...@lyx.org>
Date:   Sun Aug 28 21:57:17 2016 +0100

Better title for ViewSource


Sorry for being quite rude here:
I am strictly opposed to these changes. LyX 2.2.x are bugfix releases.
If the developers agree also some new features can go in. But these
string changes are completely unnecessary in a bugfix release.

The documentation is for all 2.2.x releases, as their title says. Unless
bugs are fixed, there is no reason to change menu names. The consequence
is that the docs of 2.2.x would differ for every release in a way that
new users don't find the appropriate menu name.

I can also not find a discussion where we made a decision to rename menu
names without fixing a bug in a bugfix release.

Therefore please revert these renamings.

In general, what is the benefit of renaming
"Source Pane|S"
to
"Code Preview Pane|P" ?
I cannot see any advantage other than a lot of works to update the docs
and the Wiki pages.
Moreover, this changes the accelerator key. So you force users to learn
a new one one an unchanged feature.
Source is the Code.  So what?

thanks and regards



Hi Uwe,


You are not being rude at all, in fact you are quite polite. And it is
not too late to re-open an old discussion given that nothing has been
released yet.

First, surely you do not mean to revert the entire commit. The main
purpose of the patch is to change the title of GuiViewSource to "LaTeX
(pdflatex) preview", "LyXHTML preview", etc. The default title "Code
Preview" does not appear in practice. I think you are mainly referring
to the change in the menu name.

During the discussions, I saw some confusion about the meaning of
"source" and "code", and you even say that they mean the same to you.
This is not true, and this change is meant for people who make the
distinction. You say that it does not fix a bug, but I already explained
that the former name introduced the confusion that LyX is a LaTeX
editor. Maybe I was not clear enough, I am really concrete here: I saw
new users (coming from LaTeX) focusing on GuiViewSource, trying hard to
"edit" the ViewSourceWidget, thinking it was really the source, and
ending up very disappointed.

I have to admit that I was careless about changing the accelerator. I
understand your point. I think that the best solution instead is to have
"Code Preview Pane|S" as a reminiscence of the former name (just like
GuiViewSource is going to remain GuiViewSource). I am ready to have this
cosmetic discrepancy to fix such an issue.

I can revert the menu name to "Source Pane" in 2.2.x for the
documentation reasons you gave. I can also revert it in master if
the majority wants it. But I hope that my explanation reassured
you.


Guillaume



Re: [LyX/master] Correctly track ulem commands with change tracking

2016-10-23 Thread Guillaume Munch

Le 23/10/2016 à 22:53, Enrico Forestieri a écrit :

On Sun, Oct 23, 2016 at 07:02:31PM +0200, Guillaume Munch wrote:


Le 23/10/2016 à 18:38, Enrico Forestieri a écrit :

commit dea5ba16de1b98d93cf30ab65119bc2364a7ac2b
Author: Enrico Forestieri <for...@lyx.org>
Date:   Sun Oct 23 18:23:41 2016 +0200

Correctly track ulem commands with change tracking

LyX assumes that everything in \lyxdeleted is struck out by ulem
and increases the corresponding counter. However, deleted display
math material is struck out using tikz. As we also take into
account the deletion of underlined display math (in order to
properly position such material vertically), we have to take
care that the count is correct.



This code (this commit and previous related commits) looks fragile to
me. Did you not prefer to present a (full and tested) patch on the list
and ask other people about it before committing?


Sorry, but I think that your comments are unwarranted and overzealous.
Moreover, this is something that you should pretend from a novice.
I think I know when something should be submitted for review before
committing. And you are not the official maintainer. Please, try
to be less off-putting. Thank you.



The code does look fragile to me. I do not think that asking that
developers care about maintainability is being overzealous. Then, maybe
I am mistaken about the code and you got to something found maintainable
enough after thinking about it a lot, such that you did not feel the
need asking for advice. In that case, maybe all the misunderstandings
comes indeed from a different appreciation of what effort is asked.

But to know this we would need to speak about the code. Every time I
want to discuss your commits, I know that you are going to take things
personally, to the point that sometimes I just prefer to let it off
before even asking. I do not know another LyX developer who reacts like
you do.

Before arguing that I am not the official maintainer, ask yourself who
was the only one who tested several of your big changes and spent a lot
of time writing detailed testcases (lots of them!)? And where would LyX
be if I had not?

I wish we could speak about the code instead.


Guillaume



Re: [LyX/master] Correctly track ulem commands with change tracking

2016-10-23 Thread Guillaume Munch

Le 23/10/2016 à 19:55, Richard Heck a écrit :

On 10/23/2016 01:02 PM, Guillaume Munch wrote:

Le 23/10/2016 à 18:38, Enrico Forestieri a écrit :

commit dea5ba16de1b98d93cf30ab65119bc2364a7ac2b
Author: Enrico Forestieri <for...@lyx.org>
Date:   Sun Oct 23 18:23:41 2016 +0200

Correctly track ulem commands with change tracking

LyX assumes that everything in \lyxdeleted is struck out by ulem
and increases the corresponding counter. However, deleted display
math material is struck out using tikz. As we also take into
account the deletion of underlined display math (in order to
properly position such material vertically), we have to take
care that the count is correct.



This code (this commit and previous related commits) looks fragile to
me. Did you not prefer to present a (full and tested) patch on the list
and ask other people about it before committing?


Looked pretty straightforward to me. This is a very common use of an
OutputParams flag. In any event, I take it that this fixed a bug in some of
the previous work Enrico did on this. It's not usually our policy to
require
discussion of that kind of thing.

Where we do always want discussion is with significant changes of behavior,
and especially of UI-related changes that affect the user experience. If
I'm
remembering correctly, there has been some such discussion around how
deleted displayed math is handled.



Yes, each of the commits looks straightforward. Some of the flags
were already there indeed. And yes, the change in behaviour has been
discussed and is most welcome.

I meant something else. It seems that "common use of an OutputParams 
flag" results in having the logic of a feature scattered all around in 
tiny bits of code. I am worried that this is not going to be easy to

maintain.

When I make small or large changes to the code I always try to give back 
something more modular than what I start with. So I wonder whether this 
logic could be centralized in some way.



Guillaume



  1   2   3   4   5   6   7   8   9   10   >