Re: [LyX/master] Further amendment to 72a488d7

2017-03-26 Thread Guillaume MM

Le 21/03/2017 à 00:04, Enrico Forestieri a écrit :

commit 16d5c49b383841826d1bc563e2d392e12e497ed8
Author: Enrico Forestieri 
Date:   Mon Mar 20 23:59:16 2017 +0100

Further amendment to 72a488d7

Rephrase positively the check box for the output of en- and em-dashes
and disable it when using non-TeX fonts. The state of the check box
is remembered, so that toggling the non-TeX fonts check box does not
cause information loss.
---

...

+   connect(fontModule->dashesCB, SIGNAL(toggled(bool)),
+   this, SLOT(dashesToggled(bool)));

...


+void GuiDocument::dashesToggled(bool state)
+{
+   if (!fontModule->osFontsCB->isChecked())
+   fontModule->dashesCB->setChecked(state);
+}


Thanks. I am not sure to understand, is the above not redundant? When
osFontsCB is not checked, toggling dashesCB already changes its state.



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

2017-03-20 Thread Guillaume MM

Le 19/03/2017 à 22:12, Pavel Sanda a écrit :

Guillaume Munch wrote:

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


Hmm this is not what I see here (gcc 4.9.4 & qt 4.8.5).
I need to scroll or cause some movement so some painting is triggered,
activation of window isn't enough. But at least I know work around.


The first redraw triggers the modification check. It needs a second
redraw after being notified of the result. To me this looks like another
bug where a manual redraw is necessary for math previews. What is the
proper way of asking for a redraw inside the event loop?



So without touching this, do you see easy way how to trigger some signal
which would issue loading all images in the document, not just on the screen.
I could just call such lfun and let lyx work in backgrounds without managing
it for 5min; even that would be relief...


No, not really familiar with this. But it would be achieved by computing
the metrics of the whole document. Such an LFUN will also be useful
for making the scroll bar accurate, would it not?





Re: [LyX/master] Fix output of en- and em-dashes with TeX fonts

2017-03-20 Thread Guillaume MM

Le 19/03/2017 à 20:58, Enrico Forestieri a écrit :

+  Don't use ligatures for en- and em-dashes


Can I suggest to phrase it positively? "Output en- and em- dashes as
ligatures"



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

2017-03-20 Thread Guillaume MM

Le 19/03/2017 à 12:26, Christian Ridderström a écrit :


# Temporary fix: Apply JMarc's patch to allow 'make distcheck' to
complete
patch -p1 <
/builds/0001-Fix-distclean-for-recent-automake-versions.patch


Jean-Marc, perhaps that patch should no longer be applied?  It was
needed earlier to make the checks pass.
I've created another CI job that's not applying that patch and that's
currently running, so we'll see what happens.
/Christian




What is this patch that fixes make distcheck? Can it be applied in master?



Re: [LyX/master] Fix output of en- and em-dashes with TeX fonts

2017-03-20 Thread Guillaume MM

Le 20/03/2017 à 11:00, Enrico Forestieri a écrit :

On Mon, Mar 20, 2017 at 07:25:38AM +0100, Guillaume MM wrote:


Le 19/03/2017 à 20:58, Enrico Forestieri a écrit :

+  Don't use ligatures for en- and em-dashes


Can I suggest to phrase it positively? "Output en- and em- dashes as
ligatures"


I thought about it, but this is not possible. As it is phrased now, you
can be sure that, if it is checked, dash ligatures will not be used.
If you phrase it positively, then you have to always use dash ligatures,
also when using non-TeX fonts, in which case it will not work.



Even clearer, then, would be to phrase positively and grey out the box
on use-non-tex-fonts. Just a suggestion.



Re: Ticket #10455

2017-04-10 Thread Guillaume MM

Le 10/04/2017 à 21:05, racoon a écrit :

Hi!

Can someone check

http://www.lyx.org/trac/ticket/10455

and put it into master for testing?

Best,
Daniel




Hello Daniel and the list,

you also have
http://www.lyx.org/trac/ticket/10483
http://www.lyx.org/trac/ticket/10379
http://www.lyx.org/trac/ticket/10283

they are (and remain) on my to-do list for 2.3, but anybody else please
feel free to review the patches since it takes so much time for me to
get back to it.

Sincerely,
Guillaume



Re: [LyX/master] Add support to cross out characters

2017-04-12 Thread Guillaume MM

Le 11/04/2017 à 10:04, Jean-Marc Lasgouttes a écrit :

Le 11/04/2017 à 09:45, Guenter Milde a écrit :

The Python convention is to leave out "== true" while in C++ this may be
required. (Python auto-converts any value to a Boolean if required in an
"if" clause.)


I do not see any reason to keep == true in C++ either.



As Günter said this can be necessary if the class does not define
operator(bool). Before C++11, operator bool() could not be made explicit
and therefore defining operator bool() was discouraged. But there is no
longer any reason to define operator==(bool) instead of operator bool()
now that it can be made explicit.

But this issue will not arise with the LyX source since all occurrences
of "x == true" involve a boolean according to a quick check (this can
probably happen to anybody at the end of a long day...).

Amusingly, there are no occurrences of "== false".



Re: [LyX/master] MathAtom is a unique_ptr

2017-04-12 Thread Guillaume MM

Le 08/04/2017 à 15:10, Jean-Marc Lasgouttes a écrit :

Le 07/04/2017 à 23:55, Guillaume MM a écrit :

commit b382b246b62b66100733449b2bf75a01fd1d263f
Author: Guillaume MM <g...@lyx.org>
Date:   Sun Apr 2 21:04:06 2017 +0200

MathAtom is a unique_ptr

Fix coverity suggestion of defining a move constructor



-/// default constructor, object is useless, but we need it to put
it into
-/// std::containers
-MathAtom();
+MathAtom() = default;
+MathAtom(MathAtom &&) = default;
+MathAtom & operator=(MathAtom &&) = default;


Thanks for doing that. A question, though: do you advise to always add
the explicit default entries, even when we believe that the compiler
will do it by itself?


I would advise to declare all four (both copy operators and both move
operators, including if defaulted or deleted) as soon as either of the
five (including the destructor) is declared. This would make it clearer
for everybody.



Do you know of a warning level that allows us to know when the compiler
decides not to create a default constructor?


No, it does not make sense to have a warning about this since it makes
sense to have types that are non-copyable or non-movable. And even if
you write "= default" this could mean deleted. Also did I tell you
already about the nice feature where you write "move(x)" and it
makes a copy of x instead?

The only way I know to get some certainty about the use of move
operators is to declare the copy constructor explicit and the copy
assignment operator deleted. (For objects that you do not typically want
to copy around but you can move.) Then you know for sure when moves
happen and when copies happen. You get an error if you write move(x) and
the type of x prevents it from being moved. I have yet to see whether
this idiom scales well, in the case of the Inset hierarchy this seems to
work well.

Guillaume



Re: [coverity again] missing move constructors

2017-04-20 Thread Guillaume MM

Le 08/04/2017 à 23:05, Jean-Marc Lasgouttes a écrit :


Also, I do not understand this (I know it is the same as the old code)
+explicit Inset(Inset const &) : buffer_(0) {}

What is this good for? The semantics look quite broken to me. Is this
even used? (I guess we should make it private to find out).


Like in InsetMathHull and InsetMathNest, I imagine that this is meant to
implement some sort of cache that one invalidates on copy. It is used in
all the Inset*::clone functions for a start.

operator= on the other hand is used nowhere, so the difference does not 
matter. In the attached I even undefine it.



FileName:
This would be automatically copyable and movable if not for the use of
the pointer to implementation.


What is the problem with the pointer?


For motivations see for instance
<https://oliora.github.io/2015/12/29/pimpl-and-rule-of-zero.html>.


As long as the implementation does
not contain a backpointer, I would say that copying the pointer (and
setting to null in the original?


This is what I propose to do automatically.


Is this supposed to go through a
destructor at some time?)


Yes, one must be careful about custom destructors with move
operators. There is no magic ensuring that the destructor is not
called after a move.


should be good enough to won the contents.

There is a solution here:

<https://oliora.github.io/2015/12/29/pimpl-and-rule-of-zero.html>. If we
used spimpl.h from there, then all the custom destructor and copies
could be removed, and the moves would be generated automatically. In
fact this could be used to simplify other copyable classes with a
pointer to implementation. Do you think the Boost license allow its
inclusion in src/support?


I have to take a look.


Thanks.



InsetMathNest and InsetMathHull:
To do this one should define a template cached that
consists in X where the copy is overridden to reset the value (assuming
it is very important to do this).


I really do not know what this thing is.



I am thinking about something along the lines of the attached patches.
But to be clear, one should not expect any performance gain. Only some
review, clarification, and simplification of the code.

Speaking of review, I found that setMouseHover was never used, making
the variable useless. What do you think?

Guillaume
>From 9e0a2bc9ca8b496b5af488634eef905a9f837170 Mon Sep 17 00:00:00 2001
From: Guillaume MM <g...@lyx.org>
Date: Sun, 9 Apr 2017 21:43:33 +0200
Subject: [PATCH 1/3] InsetMathHull: let move operators be defined
 automatically

---
 src/CutAndPaste.cpp  |  11 +++--
 src/mathed/InsetMathHull.cpp | 101 ++-
 src/mathed/InsetMathHull.h   |  19 
 src/mathed/InsetMathNest.cpp |  33 +
 src/mathed/InsetMathNest.h   |  26 +-
 src/mathed/InsetMathRef.cpp  |   4 +-
 src/mathed/MathMacro.cpp |  11 -
 src/mathed/MathMacroTemplate.cpp |   4 +-
 src/support/unique_ptr.h |  29 +++
 9 files changed, 113 insertions(+), 125 deletions(-)

diff --git a/src/CutAndPaste.cpp b/src/CutAndPaste.cpp
index 17dbdc6..e625ebe 100644
--- a/src/CutAndPaste.cpp
+++ b/src/CutAndPaste.cpp
@@ -252,16 +252,17 @@ pasteSelectionHelper(DocIterator const & cur, ParagraphList const & parlist,
 		case MATH_HULL_CODE: {
 			// check for equation labels and resolve duplicates
 			InsetMathHull * ins = it->asInsetMath()->asHullInset();
-			std::vector labels = ins->getLabels();
+			std::vector<std::unique_ptr> const & labels =
+ins->getLabels();
 			for (size_t i = 0; i != labels.size(); ++i) {
 if (!labels[i])
 	continue;
-InsetLabel * lab = labels[i];
-docstring const oldname = lab->getParam("name");
-lab->updateLabel(oldname);
+InsetLabel & lab = *labels[i];
+docstring const oldname = lab.getParam("name");
+lab.updateLabel(oldname);
 // We need to update the buffer reference cache.
 need_update = true;
-docstring const newname = lab->getParam("name");
+docstring const newname = lab.getParam("name");
 if (oldname == newname)
 	continue;
 // adapt the references
diff --git a/src/mathed/InsetMathHull.cpp b/src/mathed/InsetMathHull.cpp
index 38cdd26..d5b9318 100644
--- a/src/mathed/InsetMathHull.cpp
+++ b/src/mathed/InsetMathHull.cpp
@@ -195,12 +195,10 @@ docstring hullName(HullType type)
 	return from_ascii("none");
 }
 
-static InsetLabel * dummy_pointer = 0;
-
 InsetMathHull::InsetMathHull(Buffer * buf)
 	: InsetMathGrid(buf, 1, 1), type_(hullNone), numbered_(1, NUMBER),
-	  numbers_(1, empty_docstring()), label_(1, dummy_pointer),
-	  preview_(new RenderPreview(this))
+	  numbers_(1, empty_docstring()), label_(1),
+	  preview_(make_unique(this))
 {
 	//lyxerr << "sizeof InsetMath: " << sizeof(InsetMath) << endl;
 	//lyxerr << "sizeof MetricsInfo:

Re: Qt terminal message "path is empty"

2017-04-20 Thread Guillaume MM

Le 18/04/2017 à 08:10, Scott Kostyshak a écrit :

I've been getting the following message on recent master:

  QFileSystemWatcher::removePath: path is empty

One way to trigger it is to have two different LyX instances open, write
"hello" in one of them and select it and copy it and paste it in the
other instance.

The attached patch fixes it for me, but it's probably wrong. I can
investigate into why I get an empty path at this point in the code, if
someone thinks it is worth the time.

Scott



I'll need to have a look; in a few days.

Guillaume



Re: beamer editing

2017-04-20 Thread Guillaume MM

Le 20/04/2017 à 23:10, Edwin Leuven a écrit :

On 20 Apr 2017, at 22:47, Guillaume MM <g...@lyx.org> wrote:


At the time I reviewed your patch and pointed out regressions of the new
behaviour: <https://marc.info/?l=lyx-devel=145031216812301=2>.


i don’t see any of those regressions


Strange, the patch is the same as at the time.



(unless i misunderstood you)


It was a long time ago and I do not remember the details unfortunately.




I also suggested to introduce instead a new layout that uses
flex insets for frames, to solve this exact issue. It is now available
at <https://github.com/gadmm/beamer-flex>. I had positive feedback about
it on the general list. Would you like to give it a try?


it’s editing wise definitely an improvement!

(and aesthetically a minor regression ;-)



Thanks for your test

Guillaume



best, ed.






Re: SIGSEGV on luatex export #2

2017-04-20 Thread Guillaume MM

Le 20/04/2017 à 19:02, Scott Kostyshak a écrit :

On Thu, Apr 20, 2017 at 04:21:49AM -0400, Scott Kostyshak wrote:

On Thu, Apr 20, 2017 at 09:10:54AM +0200, Tommaso Cucinotta wrote:

A 2nd SIGSEGV, involving similar actions, but the crash happens earlier, so
I guess it's a different bug:
1. clear your ~/.lyx-trunk/cache/*
2. start LyX, new doc
3. type "info-insert icon whatevernonsense"

=> LyX SIGNAL CAUGHT dialog with std::bad_alloc.


I can reproduce with dbcbf305 and after but not 15fd7920. I can do a git
bisect tomorrow if no one beats me to it:

  git bisect start
  git bisect good 15fd7920
  git bisect bad dbcbf305


My bisect lead to 244de5d2 but I'm not confident in the bisect (for some
revisions I could reproduce once and then not a separate time. I always
cleared the cache each time but still had mixed results).

When git bisect narrowed in on the commits around there, I thought it
would be 6c920757 since that has to do with icons. But that's not where
bisect took me.

So don't take my results too seriously. Hopefully someone else can take
a look.

Scott



The attached fixes it for me.

You can get a trace for when an exception is thrown in gdb with "catch 
throw".


Guillaume
diff --git a/src/graphics/GraphicsCacheItem.cpp b/src/graphics/GraphicsCacheItem.cpp
index c698168..33d4747 100644
--- a/src/graphics/GraphicsCacheItem.cpp
+++ b/src/graphics/GraphicsCacheItem.cpp
@@ -95,7 +95,7 @@ public:
 	/// The filename we refer too.
 	FileName const filename_;
 	/// The document filename this graphic item belongs to
-	FileName const & doc_file_;
+	FileName doc_file_;
 	///
 	FileMonitor const monitor_;
 
diff --git a/src/graphics/GraphicsConverter.cpp b/src/graphics/GraphicsConverter.cpp
index 55ae246..9e6cbd7 100644
--- a/src/graphics/GraphicsConverter.cpp
+++ b/src/graphics/GraphicsConverter.cpp
@@ -63,7 +63,7 @@ public:
 	SignalType finishedConversion;
 
 	///
-	FileName const & doc_fname_;
+	FileName doc_fname_;
 	///
 	string script_command_;
 	///


Re: beamer editing

2017-04-20 Thread Guillaume MM

Le 20/04/2017 à 09:40, Edwin Leuven a écrit :

hello,

quite some time ago i complained about editing frame content in lyx

basic frame content is indented and has a standard environment

a new line  however removes the indent and resets the standard 
environment to frame

to continue editing one has to re-ident and reset the environment, which is imo 
disruptive and unnecessary

i think the desired/expected behavior is that  preserves the identing 
and environment

and that  only resets things if the line is empty

(just like in itemize)

at the time i send in the attached little patch



Hello Edwin,

At the time I reviewed your patch and pointed out regressions of the new
behaviour: .

I also suggested to introduce instead a new layout that uses
flex insets for frames, to solve this exact issue. It is now available
at . I had positive feedback about
it on the general list. Would you like to give it a try?

Guillaume




i was wondering what you think about the suggested behavior, and whether you 
have some comments on the patch

it would be nice to have this fixed for 2.3

regards, edwin








New warnings in master

2017-04-20 Thread Guillaume MM
../../src/insets/InsetSpecialChar.cpp: In member function ‘virtual 
lyx::docstring lyx::InsetSpecialChar::toolTip(const lyx::BufferView&, 
int, int) const’:
../../src/insets/InsetSpecialChar.cpp:92:9: warning: enumeration value 
‘LDOTS’ not handled in switch [-Wswitch]

  switch (kind_) {
 ^
../../src/insets/InsetSpecialChar.cpp:92:9: warning: enumeration value 
‘MENU_SEPARATOR’ not handled in switch [-Wswitch]
../../src/insets/InsetSpecialChar.cpp:92:9: warning: enumeration value 
‘PHRASE_LYX’ not handled in switch [-Wswitch]
../../src/insets/InsetSpecialChar.cpp:92:9: warning: enumeration value 
‘PHRASE_TEX’ not handled in switch [-Wswitch]
../../src/insets/InsetSpecialChar.cpp:92:9: warning: enumeration value 
‘PHRASE_LATEX2E’ not handled in switch [-Wswitch]
../../src/insets/InsetSpecialChar.cpp:92:9: warning: enumeration value 
‘PHRASE_LATEX’ not handled in switch [-Wswitch]


Guillaume



Re: [LyX/master] MathsUi.ui: adjust dimensions as requested

2017-04-07 Thread Guillaume MM

Le 07/04/2017 à 02:14, Uwe Stöhr a écrit :

commit 74bcd5d26c558e5d540b2e3370d869a3f0469aff
Author: Uwe Stöhr 
Date:   Fri Apr 7 02:14:34 2017 +0200

MathsUi.ui: adjust dimensions as requested
---
-432
+500
- 411
- 351
+ 480
+ 350


One should need to rely on the indicative pixel size which does not take 
into account the platform, dpi and language. This probably indicates 
that the ui file is broken.




Re: [LyX/master] Document new behavior of "delete" LFUNs

2017-04-07 Thread Guillaume MM

Le 06/04/2017 à 06:41, Scott Kostyshak a écrit :

commit 22c4a24a360b4607551ff5275cf24ac5f5e5eb4a
Author: Scott Kostyshak 
Date:   Thu Apr 6 00:34:51 2017 -0400

Document new behavior of "delete" LFUNs

- Describe the change in RELEASE-NOTES.
- Update the example for inset-forall.

This commit follows 71623b88.



Thanks.




Re: [coverity again] missing move constructors

2017-04-07 Thread Guillaume MM

Le 28/03/2017 à 12:22, Jean-Marc Lasgouttes a écrit :

This one is probably for Guillaume.

Coverity complains about missing move constructors for Inset, MathAtom,
Mover, SpecializedMover, FileName, InsetMathNest, InsetMathHull.

The help text says: "This class does not have a user-written move
assignment operator and its copy assignment operator is found to be
applied to rvalue(s), which can be moved to possibly enhance program
performance had the move assignment operator been in place."

At least I would guess that FileName would benefit from it, but what
about the others?



Hi Jean-Marc,

The solution is of course not to make the code more complicated (and
error-prone) with custom move operators. Instead one should rely on
compiler-generated move and copy constructors and assignment operators
in a more systematic manner, sometimes making the implementation simpler
even. See <https://rmf.io/cxx11/rule-of-zero>.

It is good to have in mind that the move operators are not defined
implicitly if there are custom copy operators or a custom destructor.

Inset:
I do not understand why the copy constructor does one thing but the copy
assignment something else. I doubt that users of Inset and its
descendents are all aware of the difference. If not important, I suggest
removing the custom copy constructor. This lets default copy and move
being generated automatically. If important, I suggest marking the
constructor explicit so that it is always called on purpose (then the
others need to be defined as defaulted). See attached patch.

MathAtom:
This is essentially an owning pointer with copy semantics. It can be
simplified by deriving from unique_ptr. See b382b246. To get the most
out of the move operators one should also make sure that MathData can
also be moved. To further audit unnecessary copies you can mark the copy
constructor explicit (as well as that of MathData).

Mover, SpecialisedMover:
This is certainly unimportant. But, they can be added very simply by
declaring them as defaulted (once you do that you have to declare the
copies as defaulted too). See attached.

FileName:
This would be automatically copyable and movable if not for the use of
the pointer to implementation. There is a solution here:
<https://oliora.github.io/2015/12/29/pimpl-and-rule-of-zero.html>. If we
used spimpl.h from there, then all the custom destructor and copies
could be removed, and the moves would be generated automatically. In
fact this could be used to simplify other copyable classes with a
pointer to implementation. Do you think the Boost license allow its
inclusion in src/support?

InsetMathNest and InsetMathHull:
Remove custom copies and destructors using the same ideas as for
MathAtom. To do this one should define a template cached that
consists in X where the copy is overridden to reset the value (assuming
it is very important to do this). Note that defining an empty destructor
explicitly is harmful since it prevents implicit definition of the move
operators.

Of course one should audit and simplify all the classes with these
principles in mind.

Guillaume
>From f69fd0aa1f58284103cb4689b90d02b1648713fa Mon Sep 17 00:00:00 2001
From: Guillaume MM <g...@lyx.org>
Date: Sun, 2 Apr 2017 19:58:44 +0200
Subject: [PATCH 1/2] Specify move constructors

Fix coverity's suggestion to define a move constructor
---
 src/insets/Inset.h | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/insets/Inset.h b/src/insets/Inset.h
index 3d93904..b1494f9 100644
--- a/src/insets/Inset.h
+++ b/src/insets/Inset.h
@@ -589,8 +589,11 @@ public:
 
 protected:
 	/// Constructors
-	Inset(Buffer * buf) : buffer_(buf) {}
-	Inset(Inset const &) : buffer_(0) {}
+	explicit Inset(Buffer * buf) : buffer_(buf) {}
+	explicit Inset(Inset const &) : buffer_(0) {}
+	Inset(Inset &&) = default;
+	Inset & operator=(Inset const &) = default;
+	Inset & operator=(Inset &&) = default;
 
 	/// replicate ourselves
 	friend class InsetList;
-- 
2.7.4

>From 0bce11f97ead15e7cacfaa4ecdcdb2a5a4b69e36 Mon Sep 17 00:00:00 2001
From: Guillaume MM <g...@lyx.org>
Date: Mon, 3 Apr 2017 00:31:37 +0200
Subject: [PATCH 2/2] define move constructors

---
 src/Mover.h | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/Mover.h b/src/Mover.h
index 4d1c1c7..3769d1c 100644
--- a/src/Mover.h
+++ b/src/Mover.h
@@ -27,7 +27,12 @@ namespace support { class FileName; }
 class Mover
 {
 public:
+	Mover() = default;
 	virtual ~Mover() {}
+	Mover(Mover &&) = default;
+	Mover & operator=(Mover &&) = default;
+	Mover(Mover const &) = default;
+	Mover & operator=(Mover const &) = default;
 
 	/** Copy file @c from to @c to.
 	 *  This version should be used to copy files from the original
@@ -109,9 +114,14 @@ protected:
 class SpecialisedMover : public Mover
 {
 public:
-	SpecialisedMover() {}
+	SpecialisedMover() = default;
 

Re: Options for resolving the minted + shell-escape issue

2017-07-31 Thread Guillaume MM
t, has looked at the code (the patch is not so long) 



I spotted various areas of improvement:
* It is better to move the definition of a function to the cpp file than
moving an #include from the cpp file to the header (GuiView).
* Addition of shell-escape is hard-coded not once but twice: two
different bits of code are responsible for the dialog and for what is run.
* There are const_casts which could be avoided.
* The new buffer param and the boolean argument in the session file are
superfluous and make the code much more complicated than needed.
* After clicking "always for this document" the emblem only appears
after one clicks again on the document.

For the latter three one can have a look at my patches for better
solutions. The last issue is fixed (in my case) by the third patch.
>From 28b242803687ef57b0d4212d9d0bf00f64e83ccb Mon Sep 17 00:00:00 2001
From: Guillaume MM <g...@lyx.org>
Date: Sat, 29 Jul 2017 23:23:50 +0200
Subject: [PATCH 1/3] Add visibility and revocability for needauth

Based on a patch by Enrico Forestieri
---
 lib/images/emblem-auth.svgz | Bin 0 -> 1419 bytes
 src/Buffer.cpp  |  17 
 src/Buffer.h|   7 +
 src/Converter.cpp   |  42 ++---
 src/Session.cpp |  26 +++---
 src/Session.h   |  11 +---
 src/frontends/qt4/GuiDocument.cpp   |  13 +
 src/frontends/qt4/GuiDocument.h |   1 +
 src/frontends/qt4/GuiView.cpp   |  45 +++-
 src/frontends/qt4/GuiView.h |  18 -
 src/frontends/qt4/GuiWorkArea.cpp   |   5 +++-
 src/frontends/qt4/GuiWorkArea_Private.h |   2 ++
 src/frontends/qt4/ui/OutputUi.ui|  16 +---
 13 files changed, 174 insertions(+), 29 deletions(-)
 create mode 100644 lib/images/emblem-auth.svgz

diff --git a/lib/images/emblem-auth.svgz b/lib/images/emblem-auth.svgz
new file mode 100644
index ..67a8313c2354f0c6630abf7c54bdfa4a5ccc7a92
GIT binary patch
literal 1419
zcmV;61$6o!iwFP!00L@ohZ`(E$e$TJql$RE>DBi@2YNrn^3iP$>uxC}GZMCvw
zNOGLy*Y8kwxel74#Wov(#qWINxgRR)w-1LL-P@|p%3|YDOgv<Zq)fA7xAFe`{K3~A
zs++h-<Gd{F#w$wi?cL4#H{VC^DjPR8Mdz&9qmRY6I*E@Kz1la;F$#k7`HZt3#AUS$
zUL)VXySZ7{_q!W}5a=!HC`~qAzu|bQ@|Kt;fz9o~7EK*cOapJCClft!+OvC`l!rrE
z)a}lqzMYY)bUU)0HD}(Es4*rWEMUwBhF?DxP5j_5w}Oq=wlYFU0J4esGFDVWK#%Y@
z(gy(7<*7>S7WP?O*d}=Y`TZ#L2~L}IcFYjZh1Nxoi}+ycW1QGJ81Q!gBEt?iXKAzF
zctW?Uz0G#}W?W|J#sdeaCLMV`!x3X#nZo=DBY>#Xx=P_Vp^GQ60gCVTvjmaz(v6
zAgvF!iPN}=-G)xpfKb%KE40Ytzexb(O+fttzW{&5#lf932gT6MtGej5rpy(
zH+R_~-dUH0-ywACU=l3I#&)Y!c3Dx<~9X?G+Ca1bV@uFTpXG;xxH(3
zc8`M>LGMQIWH9q`9SlKeS7X3kwA1^{p1U)Spf4%+xFvif-hy2+G-HV_OlR`
zP!wP}6hIG`U%uL;Y0BJIapAnBq?ha}sEup*DNF4&!I*Yu-l*uRwBE;QdER*JQh3e^
z5cqvrQfZ!J45dm5;oek3W1wi*qxHT#J6ASdQ=RPPX-{Q)#-;iW8j00SlFnleql
z6M}0@JY(F$6}NlQwTSehL;R2(vL~Bfp(LlOf=TG-@uRJ#uIx)_q{2B6Rh+@g5!>
z<_(~Wg9ErUvai^6@O_!Jcxt!x6j`_8!<J**>@O`WivlXCtbDNbK5kBxo&2bq#~hY@
zQAG637TPAdO%j-qq+R$+nO~i98x>{o1U37nZmRN|jc%oG`Q5TBk_hGs5t=Hk20YKe
zkoXu?<*7*L;MX!MmMC<wt#XKZ6Nv#!W2nxmiXWYNJ4egywzf?)8ZeqXD?k?+D#O8X
zH`b5{vDQ>*wqhI`Be^jsMvOwy87al8e+m!RTScQf#T^+Jg1;Yctb&{^GRQ
zEi=qqq~(8;4bI0Q(q%(n78=4;n+*~Y9lC4~OreNGxayy{8Ph>9<Dt|>xpX|sThPXQ
z>?UJ|%6*X<?kWO{;-<<TUQsN#W=26a;XAE-O0YDV>-LF<Mk(=nX#6SLy>n0_
zOe-qPiZZN);sPZIULYt_MhP&?cUB4sS};{Y7%@jUctaG*5xgEqp(LClunmOYzW!VrWG-!x$_ES#03#E161L-jBpA7go`@>
zxd?D7^_6oLM~+?&*7&`b;T-7=oO$Ul-SkI4{n1gkFBDwKF<x|r?UB!LsEMMqi#H)A
z>|6tL+h6E@_sdM!9!p?;hZFL+P)a-8j7q{oUZ@S(T5GW?#{a5_L
zCjC)6{&*k_FHl#iT4JFkr+md3mXc{HUCUxlxDYT^yN6lFVo9Xdt`eRl%{a^8JME0f
z?}C4MdHr-67-mXp=;;4`R=zYj>tOdYx6Bwgdl@+BDazeY4*y96>AzUN<8=a6v$#$;
z3=~2+b=L`jIi=be0<&5JmBPG`n1qbFF(a^p;P&^a#!|s}*6qNQTx;|WQHo&%Y6Ulw
z##DzPVNj-&!@H?kagH^#td<kBQ0NaD71CH40%ihcu!LGxhNJ#bU!eV_c9ixEx#|yK
Ze1^?7b(h=P{abZ+^EbutIKev*002BXz0Uvu

literal 0
HcmV?d1

diff --git a/src/Buffer.cpp b/src/Buffer.cpp
index a9cc619..d17f142 100644
--- a/src/Buffer.cpp
+++ b/src/Buffer.cpp
@@ -55,6 +55,7 @@
 #include "ParagraphParameters.h"
 #include "ParIterator.h"
 #include "PDFOptions.h"
+#include "Session.h"
 #include "SpellChecker.h"
 #include "sgml.h"
 #include "texstream.h"
@@ -875,6 +876,22 @@ void Buffer::setReadonly(bool const flag)
 }
 
 
+bool Buffer::auth() const
+{
+	return theSession().authFiles().find(absFileName());
+}
+
+
+void Buffer::setAuth(bool authorize) const
+{
+	if (authorize)
+		theSession().authFiles().insert(absFileName());
+	else
+		theSession().authFiles().remove(absFileName());
+	workAreaManager().updateTitles();
+}
+
+
 void Buffer::setFileName(FileName const & fname)
 {
 	bool const changed = fname != d->filename;
diff --git a/src/

Re: Going into dangerous mode (Was: Can shell-escape take advantage of needauth framework?)

2017-07-31 Thread Guillaume MM

Le 29/07/2017 à 23:54, Scott Kostyshak a écrit :

On Thu, Jul 27, 2017 at 04:09:56PM +0200, Guillaume MM wrote:


* Having to use -shell-escape for running Pygments.


Yes, and if we go the way of the patch, I don't think any other
improvements (e.g. post-beta1) will be made to address this, until
perhaps 2.4.0 if the Github issues is addressed.


I take note.




I would also be more comfortable if somebody takes responsibility for
any patch that is to be committed, given that the author has said that
they do not endorse it.


Fair point. My goal with the vote was to collectively take
responsibility, since this is an important patch and involves security.
But I feel that most people are just tired of the debate and are hoping
too much to move forward that they have not taken a deep look.


I meant it in this sense. If a vote only means "I did not have a look at
the patch but I am fed-up so let us go ahead" then it is not taking
responsibilities.



Re: Options for resolving the minted + shell-escape issue

2017-07-31 Thread Guillaume MM

Le 31/07/2017 à 10:57, Guillaume MM a écrit :


I spotted various areas of improvement:

...

* The new buffer param and the boolean argument in the session file are
superfluous and make the code much more complicated than needed.

...

I now understand better that this is by design, but it (still) does not
seem very natural to me. There are three levels, because there is
1. a switch to enable shell-escape (after which we are still given a
further dialog at compilation before actually running with shell-escape).
2. a button on this dialog to further "always allow".

The red icon on the bottom right is given when 1. is active even though
one is still safe as long as 2. has not been pressed. With e.g. Sweave
the first switch is automatic and innocuous (there are Sweave insets, so
ask for running sweave). Then with my patch the icon informs when one
has authorized to never ask anymore, so one is in an actual unsafe state.

With Enrico's patch, somebody using minted will always check the switch
1. and from this point on he always sees the red icon, so the icon
provides zero information. The contextual menu reverts the choice of
converters, which is pointless because it just makes the
document uncompilable. There is no option to see just revoke 2. or to
see the status of 2.

As a conclusion, as it is, the status bar icon in this patch is not an
implementation of visibility nor revocability. For it to be so, the icon
should be shown when 2. is active, and the context menu should switch 2.
instead of 1.

If furthermore a validation mechanism was provided for minted requiring
shell-escape, then one would just need 2 levels as with needauth.



Re: [LyX/master] prefs/needauth: added warning if user tries to disable authorization for needauth converters.

2017-07-31 Thread Guillaume MM

Le 27/07/2017 à 00:06, Tommaso Cucinotta a écrit :

commit 8a4fcd3d95ca4aeed1c46152cecadf29ed21e774
Author: Tommaso Cucinotta 
Date:   Thu Jul 27 00:01:51 2017 +0200

 prefs/needauth: added warning if user tries to disable authorization for 
needauth converters.
---
+   _("SECURITY WARNING!"), _("Unchecking this option has the effect 
that potentially harmful converters would be run without asking your permission first. This is 
UNSAFE and NOT recommended, unless you know what you are doing. Are you sure you would like to 
proceed ? The recommended and safe answer is NO!"),


Thanks. HTML maybe?



Re: alternatives for dashes (please vote)

2017-07-30 Thread Guillaume MM

Le 27/07/2017 à 23:27, Guenter Milde a écrit :

Dear LyX developers,

following Scotts suggestion, I prepared an overview of alternatives to
handle the problem with 2.2 breaking formatting of em- and en-dashes.
Comments and votes welcome.

Unfortunately, the topic is emotionally loaded, as we did a mistake and
there is no solution that everyone will agree with. OTOH, it is complex, but
not very important.

The Problem
===

Up to version 2.2, LyX used 2 representations for em- and en-dashes:

"literal" dashes:
literal Unicode or \textemdash rsp. \textendash macro,
"ligature" dashes:
"---" and "--" converted by the TeX font ligature mechanism.

"Ligature" and "literal" dashes can coexist in <2.2-documents,
e.g., when parts were copy-pasted from different sources.


LyX 2.2 uses only one representation and converts "ligature dashes" to
"literal dashes".

This turned out to break formatting in some documents, because

"literal" dashes suppress line breaks after the dash
   and allow hyphenation in adjacent words while
"ligature dashes" allow line breaks after the dash
   and suppress hyphenation in adjacent words.

There are use cases for both, dashes with and without optional line break
**switching the representation either way can lead to broken output**
(overfull lines or unwanted line breaks).

For details and examples see http://www.lyx.org/trac/ticket/10543#comment:3
and http://www.lyx.org/trac/attachment/ticket/10543/emdash-line-breaks.lyx


Alternatives for 2.3


a) convert ligature dashes to literal dash + allowbreak

+ keeps formatting in most cases
  - allows (possibly undesired) hyphenation in adjacent words
  - a redefinition of \text*dash would also affect former
"ligature" dashes.
+ simple
+ configurable/editable (special char can easily be deleted)
± verbose (the "allowbreak" special-char is displayed as a small blue mark)

b) convert ligature dashes to "special character" insets

+ no change to LaTeX output.
- requires "special character" inset for ordinary printable character
  - does not scale well
  + precedence cases: curly quotes, text ellipsis

c) keep 2.2 behaviour (convert ligature dashes to literal dashes)

+ simple
+ no change for 2.2 documents (the damage is already done)
- breaks formatting for <2.2 documents using ligature dashes

d) convert "ligature dashes" to ERT

+ no change to LaTeX output
+ simple
- ERT for formerly supported feature

e) re-define \textemdash

+ simple (except for LuaTeX)
+ already done by XeTeX
  
(http://tex.stackexchange.com/questions/62800/lualatex-and-line-breaks-after-em-dashes)
+ configurable
- hidden in the user preamble

f) keep the current 2.3alpha behaviour

+ configurable
- breaks formatting for <2.2 documents using literal dashes
  (these were not affected by the change in 2.2)
- new document setting for LaTeX export of Unicode characters:
  - does not scale well,
  - unprecedented
- Data loss (ZWSP following dashes are removed by lyx2lyx)
- different line breaks of the same document with LuaTeX and non-TeX fonts

g) revert to 2.1 behaviour

- regression on bugs  #3647 and #8561
- no WYSIWYM: "lyx --help" in the GUI becomes "lyx –help" in the output.


Alternatives b there are two pairs of dashes 

), c), d) and e) could also be applied to 2.2.x,

a) and f) require a fileformat change.



Open question:
   which representation to use with the -- and --- input conversion?

* The simplest way is to keep 2.2 behaviour (insert literal dashes).
   
* Maybe we can use a lyxfun with options that will be bound to the '-'-key?


* Sensible defaults would be
   allow linebreak after em-dash
   prevent linebreak after en-dash.
   
   Rationale:

 en-dash around an ellipsis is used with spaces, no line break
 allowed in ranges.


Günter





So for each dash there are two variants, one with a line break
opportunity and one without, and all have a use. Do I understand
correctly? Then, one should first decide how/whether to (ideally)
present the various combinations to the user in 2.3. The answer to the
question of converting 2.2 to 2.3 will follow from there. Do you agree
that the real question is what dashes one wants to propose to users in
2.3 and how?

About e): redefining \textemdash is not nice, instead one could define a
new \lyxemdash. What are the caveats?



Re: Going into dangerous mode (Was: Can shell-escape take advantage of needauth framework?)

2017-07-31 Thread Guillaume MM

Le 31/07/2017 à 13:31, Jürgen Spitzmüller a écrit :

I meant it in this sense. If a vote only means "I did not have a
look at

the patch but I am fed-up so let us go ahead" then it is not taking
responsibilities.


A vote is a vote. If the given voting will be Rates differently, this 
will be have been the last voting I have participated on this list.




I am not sure I got your last sentence right :) but nobody proposed to
discard any vote.




Re: Options for resolving the minted + shell-escape issue

2017-07-31 Thread Guillaume MM

Le 31/07/2017 à 13:00, Enrico Forestieri a écrit :

On Mon, Jul 31, 2017 at 12:01:39PM +0200, Enrico Forestieri wrote:


On Mon, Jul 31, 2017 at 10:57:01AM +0200, Guillaume MM wrote:


A key point to me is that, according to your other message, going with
3. locks 2.3 into implementing Pygments using minted. For this reason
and that it does not solve fundamental issues, there are more reasonable
solutions than the patch at [2] even understood as an intermediary
solution to get beta out.


I now count 4 votes for the patch at [2] and only one against it
(if I correctly interpret this post as such). I think this means that a
decision can be taken without entering in other exhausting and unfruitful
discussions that most probably would go on forever.


Just to be clear. It shouldn't and cannot work this way. There were a vote
called on some options. One cannot come up with another option when he sees
that this is not what he would like. Otherwise, another one is entitled
to come up with yet another option and so on. We have to make a decision
(which is now possible, I think) and then move from there.



I am sure that Scott meant to include in some way the option that I have
been advocating constantly from the beginning, which I understand is
probably 1. (Otherwise, I do not see what the option 1. refers to nor
who proposed it, and I would opt for not taking part in the vote.) So to
be clear, my vote is 1., and the patch gives it substance as part of a
larger proposition providing the most secure option for beta and for
release so far (because after minted there remains to fix needauth). It
just incidentally happens that it does not lock one into removing minted
by providing a better basis for what people voting 3. are trying to
achieve. (In particular Scott, even if the route of 3. is voted for
beta, this patch minus disabling of minted remains an option for 2.3.)

As for your counting of votes, calling the end of the poll, and deciding
what fits or does not fit proposed options, last time I have checked you
were not release manager :)



Re: [LyX/master] prefs/needauth: added warning if user tries to disable authorization for needauth converters.

2017-08-09 Thread Guillaume MM

Le 04/08/2017 à 18:51, Tommaso Cucinotta a écrit :

As per the HTML, if this is the standard for LyX then let's go for it.

I don't think it is standard, but needauth messages are unusually long 
and require attention, and this is for them that that LyX dialogs were 
adapted to accept HTML formatting. I am not sure if I should wait the 
update of the string.




Re: Silent/automatic execution of converter and needauth, concrete questions to clarify my understanding

2017-07-17 Thread Guillaume MM

Le 17/07/2017 à 23:53, Christian Ridderström a écrit :

Hi,

I've gotten lots of information from Enrico and Guillaume related to the 
security "gap", but I'd like to boil it down to simpler questions to 
make the situation clear to me.


Assume that I've gotten a LyX document by e-mail. It was not created by 
me, but let's say that the sender of the e-mail appears to be from a 
colleague whom I trust, asking me to do him a favour and generate a PDF 
because his computer is acting up. It's urgent of course...


A) In LyX 2.2.x, if I open the document, no "converters" are executed. 
But when I attempt to generate the PDF, the document could via e.g. 'R' 
execute arbitrary code on my computer, as if it were my user account. 
And this would happen silently, with no warning etc.

Correct?

But what would happen if I used LyX 2.3.0alphaX and tried to build the 
document?

B) Would LyX still allow the document to run arbitrary code on my computer?


Depends on your needauth settings.
* Never (default for a new install): no, and an error message tells you
to change the needauth settings before you can proceed.
* Enable and ask: first you get a message asking to authorise the
converter (every time or only the first time depending on whether you
chose "allow" or "always allow for the document").
* Disabled: like in 2.2.

Note that currently all this and the below appears to hold as well for
gnuplot previews, so one does not need to compile to PDF, just to open a
file (this was not the case in 2.2).



C) Would the execution still happen "silently"?


In two cases:
* Enable and ask: if you previously clicked "always for the document".
* Disabled: it always happens silently.



D) Can the above happen with a document completely created by someone else?


In one case:
* Disabled

(Another one is if the path is ~/Download/new1.lyx and you happen to
have given permanent permissions for a file with the same path three
years earlier, deleted and forgotten about since...)


Guillaume




Re: Can shell-escape take advantage of needauth framework?

2017-07-18 Thread Guillaume MM

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

On the contrary, if preview never uses needauth converters, is it as
useful in cases like gnuplot?


By "it" do you mean the external template?


Yes


Once it is in, then it
has to be supported forever, I believe there is an agreement about this.


I wouldn't say this in absolute terms, but I would agree that there's
lots of hesitation before removing a feature, and that hesitation only
increases with time. But not that we have removed features. For example,
we removed support for printing, even though there were still some using
the feature.


I agree, but note that for printing this did not invalidate existing
documents.




Re: Options for resolving the minted + shell-escape issue

2017-07-18 Thread Guillaume MM

Le 17/07/2017 à 16:25, Richard Heck a écrit :


If I read JMarc's messages properly, then he also agrees that the
security issues are essentially the same. That also seems right to me.


Hi Richard,


I did not reply to Jean-Marc, so I'll say here that I too agree with
what he wrote at
. I
think we are on the same page that -shell-escape and R are similar in
terms of security and should both be treated using needauth, and
needauth be improved, you can also find suggestions along those lines in
earlier messages of mine.



It's true that we've always tried to be cautious about security.


I saw that you have been a proponent of the safe approach in
the past and thank you for this.


But
there is only so much we can do. Warning the user that they are about to
do something that is potentially dangerous, and making it as simple as
possible for the user to manage those privileges, is the best we can do.
I don't see the difference either between R-code and minted in this
respect. So I'm inclined to go with some version of Enrico's patch.



I disagree with the "there is only so much we can do" argument.
Minted.sty is only a small interface to Pygments.

* One could implement one of the several other interfaces to Pygments
(trading a few features in exchange of security).

* One could interface Pygments directly with LyX without relying on a
LaTeX package.

* One could ask the author of minted.sty whether he would like to
provide an alternative to requiring -shell-escape.

Either of these would require less time and less imagination than has
been spent so far into personal attacks. The limitations of
needauth-like mechanisms have been raised early, and ignored.

One cannot do much currently to increase the security of users of
-shell-escape, R or gnuplot. Needauth only helps when these are not
needed. But one can try to avoid encouraging LyX users becoming
-shell-escape users. That would be a real service made to LyX users.

I see arbitrary code execution at the other end of balance and think
that "there is only so much we can do" means a complete turnaround in
this case.

Guillaume


(About the personal attacks: I mean to write about it at a later point
in time. If I have not been replying to Enrico, this does not mean that
I do not see his messages.)



Re: Can shell-escape take advantage of needauth framework?

2017-07-16 Thread Guillaume MM

Hi Scott,

Sorry for the delay. I was very busy over the past
two weeks.

Le 05/07/2017 à 06:54, Scott Kostyshak a écrit :

On Wed, Jun 28, 2017 at 02:36:49PM +0200, Guillaume MM wrote:

Le 27/06/2017 à 23:45, Tommaso Cucinotta a écrit :


needauth was a urgently needed mitigation of the security issues behind
running
arbitrary external tools when compiling LyX documents; a more engineered
remedy
AFAICR was actually the use of sandboxing machineries, which was
prototyped on
Ubuntu/Linux using AppArmor.


This is also what I remember. The now secured converters were sweave and
knitr, introduced in 2011 and 2012.


+1


I see that you have also introduced a gnuplot converter with an example.

+ Proportionality: unsafety is actually a main feature of gnuplot from
what I understand from http://www.yqcomputer.com/320_2475_1.htm
+ Specificity: only gnuplot is given elevated privileges, which is what
the user wants.
- UI problem 1: When I open the example, I immediately get the needauth
dialog for showing the preview. I thought we only wanted unsafe
execution when compiling the document.


I forget what we decided on this. If we don't give the dialog, then we
should just disable the preview?


But then if I enable gnuplot for compilation, does it mean that preview
becomes enabled? Then will this be remembered on next opening without
asking? What if I change my mind later on / do not remember? etc.

On the contrary, if preview never uses needauth converters, is it as
useful in cases like gnuplot?

etc.




It seems to me that needauth, as it is, is not ready for the addition of
gnuplot. What do you think?


I'm not sure. Is it less secure than Sweave/knitr?


At the moment I believe it is less secure, because UI issues discussed
above are more acute currently, but mainly:


Or is your argument
that those were already there so needauth makes them safer, but we
should not add any other converter that needs needauth?


Yes, this is what I believe is the safest route (see answers to your
more specific points in the other message). I did not dare suggesting
to remove features that were there since 2011. Once it is in, then it
has to be supported forever, I believe there is an agreement about this.
This is also why I have been suggesting that the most careful choices
are made from the start.

Guillaume



Re: Options for resolving the minted + shell-escape issue

2017-07-16 Thread Guillaume MM

Le 17/07/2017 à 00:49, Christian Ridderström a écrit :
On 5 July 2017 at 06:59, Scott Kostyshak > wrote:


Dear all,

This is an important topic since it involves security. I'd appreciate it
if you spent some time on understanding the issue.

I see three options for what to do about the minted + shell-escape
issue:

1. Revert the recently added minted support.

2. Keep the current state of master.

3. Apply the patch at [1] (also attached for convenience).


Hi Scott,

I think you're doing an excellent job as release manager and also when 
summarising the issue. I therefore have to apologise that even though I 
have read various postings/threads on this topic, I'm still confused.

Perhaps I can expand on that:

- What do we lose by doing 1) above?
- Is 1) necessary for users in order to use minted, or is it a 
convenience feature?
- How many users do we think need / want to use minted, i.e. how many 
users are affected?


Opinions seem to vary regarding what's secure and I cannot say anything 
to that. My general inclination is to prefer that the user to go through 
a bunch of manual steps in order to use dangerous features. If they 
remain in place, so be it. But perhaps we could warn the user when he's 
configured converters to use shell-escape, or if LyX was started with 
some global option enabling shell-escape. Perhaps by making the LyX 
window "look dangerous" like private browser windows, or more 
realistically flash a warning before each build.


As an aside, I've used org-mode documents in Emacs to invoke MATLAB on 
snippets from within the document and it's very nice, except the really 
annoying part where you for each snippet have to approve that it's run. 
Each time. A big bug in org-mode is that it's not properly showing the 
snippet that's about to be executed before you approve it to be run... 
Perhaps I'm a masochist...




Hi Christian,


The current needauth interface is not so much better in this respect. It
only shows the unsafe command line about to be run. Various degrees of
compromise between security and usability are proposed: always allow
needauth / always for this document / allow once / never allow. The
default is never allow, and after going to the preferences, the user is
suggested to always be asked.

Various UI improvements that you and others have suggested are subsumed
by some articles I have sent to the list. I am convinced that if one
thinks enough about it, one can get to a better secure UI. This is
to me the best hope to get to a more reasonable balance between security
and usability in the future, because the AppArmor stuff will be harder
to put into place.

Enrico argued that there are other (equally) dangerous converters 
already in LyX. Then that's something to address. Does it have to be for 
this release?


In 2.2 and before, a document could contain R code which could cause
arbitrary code execution with user privilege during compilation. The
user was not warned that a document could contain such code. (R needed
to be installed, though.)

I think I did not really measure the extent of the problem before
Tommaso's work to patch things up. This is because LyX has always been
quite strict with security, refusing for instance to directly open links
from documents (although a secure implementation of this would be quite
simple: one would just need a confirmation dialog with the URL clearly
displayed).

Starting with 2.3, Tommaso's needauth feature asks for a confirmation
before running converters labelled as needauth, and R code is now
labelled as such. Users not expecting to see R code are now safe, but
ones who expect to run R code are still unsafe. According to Tommaso,
this was meant as a quick measure before a more elaborate containment is
in place (e.g. using AppArmor in Linux–this seems experimental for now
and it will be difficult to integrate it since a proper solution
requires one for each platform).

Support for gnuplot has been rejected for a long time because of
security, but now taking advantage of needauth it has been introduced.
There are specific issues with gnuplot being also run for previews.
There is currently a separate discussion about gnuplot.

Then there is the new minted implementation. The issue with minted is
that it cannot compile without -shell-escape. There seemed to be an
agreement that this would encourage users to configure -shell-escape in
whatever ways, and that this seemed bad enough to fast-track (between
feature-freeze and beta release) the inclusion of a needauth-like
interface for -shell-escape. Now it seems that there is no
longer an agreement that there is an issue at all and that the current
partial support of minted that fails to compile on the default
configuration is acceptable. The issue, IMO, is that proposing something
in the interface is both an encouragement to use (including for users
who would not have cared or known until then) and a 

Re: Can shell-escape take advantage of needauth framework?

2017-07-16 Thread Guillaume MM

Le 05/07/2017 à 06:54, Scott Kostyshak a écrit :

On Wed, Jun 28, 2017 at 02:37:41PM +0200, Guillaume MM wrote:

Le 27/06/2017 à 21:00, Scott Kostyshak a écrit :


Where I
think there is disagreement is on whether we take a paternalistic
approach of "are you sure you know what you're doing? Think very hard
about this before you do it" or a lax approach of allowing users to
shoot themselves in the foot. Should we treat LyX users like teenagers
or adults? I really don't know the answer.


I am in favour of treating lyx users like adults. To me, this means not
reinventing the wheel and follow established guidelines. See e.g.

https://developer.apple.com/library/content/documentation/Security/Conceptual/SecureCodingGuide/Articles/AppInterfaces.html

and the paper they mention which has a lot of examples.


That's a great article. Thanks for the link. I've thought about a lot of
the principles mentioned in the article in the context of each of what I
view as the three options for moving forward:

1. Revert the recently added minted support.

2. Keep the current state of master.

3. Apply the patch from [1].

Below, I will refer to these three options by their numbers above, e.g.
(1), (2), and (3).

Of course, the quotes I choose and my analysis of whether each option
"passes" or "fails" a criterion are completely subjective.

Starting at the section <>

"In many cases, a simpler interface is more secure, because the user is
less likely to ignore security features and less likely to make
mistakes."

(1) fails: implementing minted through custom preamble code and
ERT is not simple

(2) at least the user only needs to do the "shell-escape dance", which
is what I refer to going to
Tools > Preferences > File Handling > Converters, adding "-shell-escape"
(remembering whether "shell-escape" is preceded by one dash or two
dashes), clicking "Modify", and then finally clicking "Apply" or "Save"
(which I believe is a separate source of confusion for the users).

(3) passes: it is simple to read the dialog and click on the
button.

Moving on to the section <>

"The user must be made aware of when they are granting authorization to
some entity to act on their behalf or to gain access to their files or
data."

(1) fails: when the user does the "shell-escape dance" they are not
given any dialog making them aware of the potential dangers. You might
think "well they are adding it themselves so they should already know",
but I would say that most users are probably following some unofficial
guide they saw on the internet and just copy/pasting stuff in without
thinking.

(2) same as (1)

(3) passes: there is a dialog with an explanation.

"In this case, sharing should be off by default."

(1) - (3) all pass.

"If the user turns it on, the interface should make clear the extent to which 
remote users can
read from and write to files on the local system."

(1) fails: there is no dialog.

(2) fails: no dialog

(3) passes

"In addition, as long as sharing is on, there should be some clear
indication that it is on, lest users forget that their files are
accessible by others." (this was also brought up by Guillaume at [2])

(1) - (3) all fail.

I think that (3) has the most potential for implementing a visual
indication, unless we want to parse the converter command for
"shell-escape".

"Authorization should be revocable"

(1) passes

(2) passes

(3) fails: if the user puts "Always allow for this document", it is not
easy to revoke. The user must know something about the sessions file,
which they should not be expected to know. We could put instructions for
how to remove access. I'm not sure if I agree with adding even more text
to the dialog though. Tommaso mentioned some ideas regarding revocation
in the context of needauth at [3].

"Whenever possible, your program should not only make this possible, it
should make it easy to do."

(1) - (2) fail because of the "shell-escape dance" and (3) fails because
of the sessions file.

"You should also make it clear that revoking authorization cannot
reverse damage already done"

I don't understand this. In our case would this mean that if the user
removes "shell-escape" we give a dialog saying "the previous
compilations you did with this compiler could have run malicious code" ?
If so, I think that might confuse users more than benefit them.

Moving on to the section <>

Not much to talk about here. (3) does work as I expected when I tested
on the command line.

Moving on to the section <>

I think that (1) - (3) all fail this. Preferences and sessions files are
stored in plain text that can be easily edited. I think we could improve
this by storing them somewhere else where the user doesn't have
permissions, but I suppos

Re: Cleanup before 2.3.0?

2017-07-16 Thread Guillaume MM

Le 15/07/2017 à 19:06, Jean-Marc Lasgouttes a écrit :

Le 15/07/2017 à 18:55, Christian Ridderström a écrit :
In my opinion, if we don't reach consensus easily on formatting 
issues, we should at least for now refrain from using a .clang-format.


This is probably what is going to take too much time...  I am not sure 
that we should have such a debate now.


Also regarding formatting, the available versions of clang-format 
might [*] Having done a few diffs when testing with clang-format I do 
find the use of TAB for indentation annoying. The reason is that 
sometimes the diff output in my terminal isn't aligned properly, i.e. 
what in reality is nicely aligned isn't aligned in my terminal. This 
was even after I configured Git to invoke 'less' with a tab size of 
width 4. So for this reason I'd actually prefer spaces to be used for 
indentation, but as tabs are what's currently used, that's a strong 
reason to stay with tabs.


The trick is to avoid using tabs for space alignment, like emacs' 
smart-tabs-mode does:

https://www.emacswiki.org/emacs/SmartTabs

I have to admit that I have not done it yes, and therefore I do that by 
hand currently.


Having taken the trouble of configuring this, I would recommend it. So I
am in favour of some automated code cleanup but only of the lightweight
kind. It should not touch indentation nor overlong lines which is
sometimes done by hand. There could be two levels of configuration: a
lightweight configuration that is applied to the code base and a more
complete one that people can use if they desire to. Does that make sense?

Guillaume



Re: [LyX/master] Fix uninitialized members outfd and infd

2017-07-16 Thread Guillaume MM

Le 06/07/2017 à 14:55, Jean-Marc Lasgouttes a écrit :

Le 06/07/2017 à 14:52, Jean-Marc Lasgouttes a écrit :

commit 02c9d2e67c16400588dfb103df284121520912a0
Author: Jean-Marc Lasgouttes 
Date:   Thu Jul 6 14:46:03 2017 +0200

 Fix uninitialized members outfd and infd
 Also move initialization of some variables to initializers list.


Question for someone who groks C++11: instead of using initializer 
lists, should we use default initialization in the the class definition?


Yes, I thought it was a good idea at some point...



E.g. replace ready_(false) in constructor(s) by
   bool ready = false;
in the class definition. C++11 allows it AFAIU.


gcc 4.6 :)

Guillaume



Re: Living with shell-escape: Using two LyX instances - critique invited

2017-07-18 Thread Guillaume MM

Le 18/07/2017 à 21:29, Christian Ridderström a écrit :

Hi,

If I had to use a converter that requires e.g. shell-escape perhaps the 
approach below would be useful. What problems do you see with it?


1) Use two lyx user directories, one standard and one "dangerous", with 
converters using shell-escape only in the dangerous lyx.


2) Create a tiny shell script or a desktop icon etc to launch 
"dangerous" lyx using the "dangerous" user dir.


3) Configure dangerous lyx to have a reddish background colour.

4) By default work in the regular lyx and only use the dangerous lyx
 for documents where converters are needed.

Justification:
The 1) allows me to have converters permanently configured in a 
dangerous mode, and through 4) I select in which mode I work. The 4) 
also reduces risk exposure as I should not open documents from strangers 
in the dangerous lyx.


The 2) makes it easy for me to launch the dangerous lyx. I've actually 
used this approach for a requirements tool, to open it in a special mode.


The 3) makes it clear to me in which LyX I'm working.

By working as per above I'm reducing exposure and don't have to worry 
about shell-escape in normal documents.


What would the drawbacks be with this approach?
/Christian



I find that it would be more cumbersome and error-prone than a good
needauth implementation. If I understand, what you want is visibility
and revokability, which people already seem to agree are desirable
improvements to make to needauth (a red status bar thingy).

Guillaume



Re: Can shell-escape take advantage of needauth framework?

2017-07-18 Thread Guillaume MM

Le 18/07/2017 à 23:27, Jean-Marc Lasgouttes a écrit :

Le 18/07/2017 à 23:24, Christian Ridderström a écrit :
The threat model is one important aspect, but it's difficult for us to 
know who uses LyX and in which industries. Or how many users there are 
at all. And how many of them that use converters.  If we can achieve 
good security we don't need to care about user / usage statistics though.


If we talk principles, I think we should aim for really good security 
and then discuss compromises for what's not doable. But I do think we 
could do a whole lot better than the current 'needauth'.


+1



As I wrote privately, we could have a page describing how to make LyX 
secure. Or even provide a compilation flag that removes dangerous features:

- disable needauth files
- somewhat limit what is read from the user directory to the bare minimum.

JMarc



Meep! Principle 1: "Things don’t become unsafe all by themselves
(Explicit authority)". This means in particular: the secure settings are
the default.

More generally, about the discussion becoming "crazy": academics have
come up with principles that everybody can apply for designing secure
UIs (we do trust them on this list, do we? ;). I see several virtues to
starting from such principles: this keeps the discussion focused, we do
not reinvent the wheel, we do not need to do guesswork on who are the
users and their threat vectors (whether they work in a philosophy
department or a P4 lab :).

Even considering this, I agree that this looks like a heavy discussion,
and I did not imagine it taking place between feature-freeze and beta
release. But if Scott decides to take the time to get to a good
implementation of needauth for 2.3, he will deserve credit for this.

(To add to the list: the "shoot yourself in the foot" option should be
removed IMO.)



Re: Any descriptions of the security aspects (related to needauth and shell-escape)?

2017-07-20 Thread Guillaume MM

Le 19/07/2017 à 16:59, Richard Heck a écrit :

On 07/19/2017 02:22 AM, Christian Ridderström wrote:

Hi,

When having tried to contribute to the discussion on needauth and
shell-escape I've felt that it's quite difficult to get a good picture
of things like:
- Goals of design, what are we trying to achieve
- Principle of design and system
- Assumed threat models, and perhaps list threat scenarios we _don't_
try to protect against

The e-mail threads are ... long, sometimes confusing and I suspect
contains at least a few misunderstandings.  So I would like to ask
(not being optimistic), if there's some design description anywhere?


No, as usual, there is not. The needauth mechanism was developed by
Tommaso in response
to security worries about certain sorts of converters, e.g., the ones
for R and related worries
about the use of gnuplot. (It may have been the latter that got him
interested.) Once that was on
board, Enrico decided to employ at least a somewhat similar mechanism to
support minted.sty,
and for whatever reason, that set off alarm bells which, in retrospect,
should have gone off
earlier. So we find ourselves in the middle of things.

Richard




Yes Richard, (smaller) alarm bells could have gone off a month earlier
if I had paid attention to the gnuplot discussion. They went off when
Scott explicitly asked about extending the use of needauth, and it did
not seem to have changed the course of things.

For 2.3 Scott chose to ask "what can we do for LyX to be the safest?"
rather than the obvious solution to get beta out. I find it reasonable
and a worthwhile time investment.

Guillaume



Re: Can shell-escape take advantage of needauth framework?

2017-07-20 Thread Guillaume MM

Le 19/07/2017 à 16:47, Richard Heck a écrit :

On 07/19/2017 05:06 AM, Pavel Sanda wrote:

Christian Ridderström wrote:

I just did a test with gnuplot. In the LyX settings I had unchecked 'Forbid
of use of needauth converters' and unchecked 'Use needauth option'. Then I
opened a LyX doc with a gnuplot script. Result: LyX tried to run the script
due to the preview, without asking or alerting me.

In my opinion this demonstrates a case where the security is _not_ good
enough, as I don't think it'd very difficult to trick someone into
unchecking these boxes.

I think at the end it boils down to the question whether we rather want
LyX for unaware users who can't handle any responsibility or we want
to allow advanced features for more hackish crowd of people.

I obviously stay in the hackish campground ground but understand your
fear for the poor.

I would offer two quick options here:
1. Rename 'Forbid of use of needauth converters' to something scary
so users have red flag.
2. Let the machinery alive, but move the flags from UI to RC files,
and forcing people to edit them, so they have time to think
what they are doing instead of randomly clicking.


I've suggested this, too. Just to be clear, you just have to remove the
UI for this setting. It
can stay in the same file, which can just be edited.



Not sure if what is being discussed is for 2.3 or for an ideal 
implementation, but ideally what about:


1. No "needauth" preferences (do not allow needauth from being disabled).
2. The dialog has a checkbox "I have read the above and I understand the
consequences", unchecked by default, which one has to check before
clicking "allow" or "always allow for the document". This checkbox is
remembered per-user (this replaces the "forbid use of needauth" option).
3. For command-line only (without GUI), have a command-line options
--needauth=[never(default)|always|ask].

Assuming other principles are implemented (visibility, revocability,
etc.), then IMO 2. is all we need as a secure GUI, and 3. all that is
needed for a secure command line (for a needauth implementation that
satisfies other principles).



Re: Can shell-escape take advantage of needauth framework?

2017-07-20 Thread Guillaume MM

Le 19/07/2017 à 11:47, Pavel Sanda a écrit :

Guillaume MM wrote:

Le 18/07/2017 ?? 23:27, Jean-Marc Lasgouttes a écrit :

Le 18/07/2017 ?? 23:24, Christian Ridderström a écrit :

The threat model is one important aspect, but it's difficult for us to
know who uses LyX and in which industries. Or how many users there are at
all. And how many of them that use converters.  If we can achieve good
security we don't need to care about user / usage statistics though.

If we talk principles, I think we should aim for really good security and
then discuss compromises for what's not doable. But I do think we could
do a whole lot better than the current 'needauth'.


+1


You start driving me to the wall.


To clarify, this +1 is about the ideal approach to securing the UI.

For 2.3, like Jürgen, I vote for getting back to productivity quickly,
and I do not need to repeat what was to me the realistic solution for
meeting the beta deadline.


Where you have been when there was huge discussion about details of these
mechanisms with Tommasso?


The discussion was not about designing a secure UI for extending the
realm of unsafe converters. Needauth was a much needed improvement, and
I actually try not to be a PITA in general (really). Moreover, Helge had
said what had to be said about its limitations in terms of security.

And between us, I try to steer away from discussions on the list unless
really necessary, to the point that I have been avoiding making
developments which I knew would involve discussions :( I mean, look at
the thread I found back in the archives exactly during the time frame of
the needauth discussion:
<https://www.mail-archive.com/lyx-devel@lists.lyx.org/msg197930.html>.
Do you think that I am incited me to contribute to discussions more than
is really necessary when they tend to become this embarrassing and
everybody acts as if this is normal?




Meep! Principle 1: "Things don???t become unsafe all by themselves
(Explicit authority)". This means in particular: the secure settings are
the default.


Yep. Isn't it the case with needauth right now?


Indeed, this is how needauth increased security.

Thank you for your involvement in the thread.

Guillaume



Re: [PATCH][RFC] Make "devel mode" configurable at run time

2017-07-24 Thread Guillaume MM

Le 24/07/2017 à 16:23, Jean-Marc Lasgouttes a écrit :

Hi,

Anyone against putting this in master?

JMarc


0001-Make-devel-mode-configurable-at-run-time.patch


 From 5488f8f06eae75bf2265e88a344be246130b9ada Mon Sep 17 00:00:00 2001
From: Jean-Marc Lasgouttes
Date: Mon, 24 Jul 2017 00:21:43 +0200
Subject: [PATCH] Make "devel mode" configurable at run time

Traditionally LyX behaves differently when the directive DEVEL_VERSION
is defined at compile time. This covers
* more detailed description of current position in status bar
* the help files are open in read/write mode
* more detailed debug output in the View Source panel

This patch introduces the new function devel-mode-toggle that allows
to use devel mode in stable releases, and vice versa.


Sounds good.



The information is saved in the session file. Only the default value
depends on the compile-time #define.


It makes sense to me to entirely remove the compile-time switch.

Guillaume



Re: Living with shell-escape: Using two LyX instances - critique invited

2017-07-27 Thread Guillaume MM

Le 23/07/2017 à 20:45, Christian Ridderström a écrit :

Bah, I again e-mailed only Guillaume and not the list.

On 19 July 2017 at 00:00, Guillaume MM <g...@lyx.org> wrote:



I find that it would be more cumbersome and error-prone than a good
needauth implementation.



cumbersome:
Do you refer to using two user dirs, or perhaps having to (once?)
modify the parameter of the converter settings?
Or do you mean having two LyX windows open at the same time?


All of that. Also that, when running in unsafe mode, you have to assume
that everything is unsafe. So this unsafe mode it is still less safe
than a good needauth implementation.



error-prone:
Do you mean that you'll open the doc^H^H^Hprogram [*] in normal mode
and when you try to build it fails?
Or do you think it'd be difficult to set this thing up?


That one.



If I understand, what you want is visibility
and revokability, which people already seem to agree are desirable
improvements to make to needauth (a red status bar thingy).



Yes, visibility that I'm operating in an unusual/non-default and
potentially dangerous mode.

Revokability I'd rather say isolation/separation.


By isolation/separation, what do you mean? Is it different from 
visibility (being able to tell whether things are safe)?




Besides wanting to allow the use of shell-escape only for a limited
set of documents (i.e. documents), I would likely also only want to
allow shell-escape when I want to run the program.


It makes sense that an option in the command line could disable needauth
entirely. It is becoming hard to track all the suggestions so if you are
interested in this I suggest filing enhancement reports / the wiki page.



Anyway, I think we should strive to allow a design where shell-escape
is not needed. And this topic is about a fallback when shell-escape is
necessary.


+1



Re: Question for Guillaume about b30f8d3c and GuiFontMetrics::countExpanders

2017-07-27 Thread Guillaume MM

Le 25/07/2017 à 14:36, Jean-Marc Lasgouttes a écrit :


Hi Guillaume,

The commit log for b30f8d3c says: " CountExpanders() is moved to 
FontMetrics to fix a discrepancy with the duplicate implementation from 
598f7e4a."


I interpret that to mean that countExpanders() should be available in 
GuiPainter too, right?


OTOH, countExpanders is basically a plain function and it seems weird to 
require a FontMetrics object to compute it.


Is there a reason why it is not a free standing function somewhere in 
support/ ?


Hi Jean-Marc,

The reasoning was that how many expanders there are in a string is
toolkit-specific. This implementation was by testing and reading the
docs and source code of Qt, and does not correspond to my knowledge to
any Unicode standard (say). So, using the frontends/ virtual interface
is more appropriate, and virtual functions cannot be non-member nor
static. Concretely, the FontMetrics object is used to know which
frontend one refers to.

In contrast, it seems that support/ contains non-UI related Qt tools
that could still work with other frontends (e.g. command-line
interface). (Or maybe I am being too optimistic.)

I you say that for countExpanders this looks like an overzealous
application of the frontend separation I will agree. But if the question
was just out of curiosity and does not limit you then I would say leave
it as is.

Any clarification on Qt in support/ vs. Qt in frontends/ is welcome.

Guillaume



Re: Going into dangerous mode (Was: Can shell-escape take advantage of needauth framework?)

2017-07-27 Thread Guillaume MM

Le 22/07/2017 à 00:47, Guenter Milde a écrit :

On 2017-07-19, Richard Heck wrote:

On 07/19/2017 01:48 AM, Christian Ridderström wrote:

On 18 July 2017 at 23:49, Jean-Marc Lasgouttes > wrote:
 Le 18/07/2017 à 23:42, Christian Ridderström a écrit :



 I think the default should be secure, and that the user should
 have to do something actively to go into a dangerous mode.




 Well, since you consider that turning off two options is not
 active enough, I am not sure what to propose :)




The problem I see with only unchecking two check boxes are e.g.:
- Users uncheck settings all the time, it doesn't seem very "scary"
- In the settings dialog, the real implications of unchecking these
options
   did not seem sufficiently clear to me.
   So calling it "Allow yourself to be shot in the foot by converters"
would help;-)
- The setting is persistent, and easily forgotten



This, I believe, was part of what was addressed by Enrico's patch. Or
the idea behind it.


Enrico's patch did not touch "needauth" but has some nice features for
"shell-escape":


+1


it addressed the "set and forget" issue by

a) adding a red icon to the status bar if a document has the "allow
shell-escape" flag.


+1

   
b) revoking the permission, if the document is moved/copied to another

location.


I like the principle, but I wonder whether this will cause annoyances
for files on removable filesystems.

   
I like the approach, especially b) seems a good compromise between user

comfort and security.

   
 From a user perspective, a common interface to "needauth" and "allow

shell escape" seems the best. "needauth" could actually take advantage of
Enrico's patch.


+1



Some ideas:

* Add "unsafe pdflatex" (== pdflatex --shell-escape) and "unsafe xelatex"
   as new converters requiring "needauth".

* Allow per-converter permission settings (instead of one generic: "I
   trust/don't trust all unsafe converters").


+1



* clicking the red icon should take you to the dialogue allowing to
   revoke the unsafe permission.


+1

   
* Give users the possibility to check scripts before allowing to run them

   with shell-escape or at least list all parts of the document that will be
   allowed to run in unsafe mode
   (e.g. all gnuplot scripts for "gnuplot allowed", all ERT, preamble,
   document classes and packages for latex with shell escape).



I like the idea, though for shell-escape this becomes more complicated.


* I also like the error dialog when -shell-escape has been configured
without needauth, for legacy configurations. (The specific wording can
be discussed later on.)

* Like Jean-Marc, I would prefer if the -shell-escape option was not
hardcoded, but integrated with needauth and the full command-line
visible in the converters dialog in some way. For instance a new token
$$unsafe together with a per-converter checkbox to allow its replacement
by whatever unsafe option.

* One has to decide which suggestions are needed for 2.3 and which ones
can be implemented later.


On the negative side, the patch does not address the original issues:
* The limitations of needauth in the context of adding new converters
such as gnuplot (the patch is only about -shell-escape),
* Having to use -shell-escape for running Pygments.


I would also be more comfortable if somebody takes responsibility for
any patch that is to be committed, given that the author has said that
they do not endorse it.


Guillaume



Re: Can shell-escape take advantage of needauth framework?

2017-06-27 Thread Guillaume MM

Hi Scott,

Le 25/06/2017 à 22:41, Scott Kostyshak a écrit :


Judging by the comments of gpoore, we do not want to wait for this for
2.3.0. But this does affect the discussion of what to do for 2.3.0,
since we might not want to introduce a workflow in 2.3.0 that we will
change soon after.


I agree.



But regardless of what we decide to do about minted specifically, there
is still the open question of what to do with other .lyx files that
require -shell-escape. I don't think we ship any besides the newly added
minted ones, but it might be relevant to whether we make it easy to
temporarily add the -shell-escape or whether we want to make it hard (to
discourage it), with the consequence that the user might forget to
remove it. Once we answer this question in general, then we can decide
what to do with minted.


Looking at the problem from the -shell-escape perspective looks like a
false simplification of the problem to me and is likely to limit your
perspectives.

It is clear that any implementation of -shell-escape will require a
compromise between security and usability, but it is not clear to me
that the compromise should be the same independently of the feature
being implemented (I am abstract because it is not clear what else is
being discussed apart from minted.sty).

For instance, one could decide that there is no fundamental reason that
an implementation of Pygments in lyx should require -shell-escape. This
means requiring users to think about whether they want to enable
arbitrary code execution from a document for the sole purpose of having
latex instead of lyx call Pygments (which might be convenient to latex
users but pointless to lyx users). The user, given the opportunity to
think about it, will conclude that it is absurd to have to compromise
security (at least I do).




If the answer to the general question is "yes, let's make it easy so
that the user is not encouraged to permanently change a converter that
they might forget about", then from what I understand, Enrico has
proposed a patch that does that so it is straight-forward to move on: we
can use that approach for minted for 2.3.0, and when the github issues
is fixed, then we can transition to a safer approach (but I suppose it
will depend on what version of minted the user has?).
> If the answer to the general question is "no, let's make it hard so that
the user is discouarged from adding -shell-escape without thinking about
it", then from what I understand, we do not make any changes to the
current state of master (i.e. we do not apply the patch proposed by
Enrico), but we still ship minted support as it is currently
implemented.


I have not seen anyone suggesting to ship minted support as currently
implemented.



I'm sure I got something wrong in my attempt to summarize the situation
and figure out what we must decide on, so can someone correct me and add
more details? Please do so without adding your opinion on what we
*should* do. I just want to know the potential options out there.




A possible course of action. For 2.3:

* Revert the work on minted for now (without reintroducing the
external template). The work done so far is likely not lost and can be
reintroduced if minted is made into a 3-step process in the future.

* Without minted.sty support in lyx, there is no need to hurry for an
implementation of -shell-escape between feature freeze and beta release.

* Let third parties currently encouraging the manual addition of
-shell-escape do so using the needauth mechanism. This is already an
improvement.

* Optionally improve the current needauth mechanism with various ideas
that have been explored for -shell-escape.


In the future:

* Do not add new unsafe default converters in lyx until the needauth
mechanism satisfies standard guidelines referred to in the other message.

* Encourage safe alternatives instead whenever possible.


Does everyone agree that the general question (of "make it easy or hard
for user to add -shell-escape") is important and must be addressed
before 2.3.0beta1, or did I miss something?



I find that the enhancement request came in a bit late in the 2.3
release process for such a sensitive issue, and that 2.3 already
improves the situation with the needauth mechanism. So, if we conclude
that an implementation of Pygments should not have to request
-shell-escape, then I do not agree that this question is important and
must be addressed before 2.3.0beta1 (besides, for me it is not
well-framed either).

Good luck.

Guillaume



Re: Can shell-escape take advantage of needauth framework?

2017-06-28 Thread Guillaume MM

Le 27/06/2017 à 21:00, Scott Kostyshak a écrit :


Where I
think there is disagreement is on whether we take a paternalistic
approach of "are you sure you know what you're doing? Think very hard
about this before you do it" or a lax approach of allowing users to
shoot themselves in the foot. Should we treat LyX users like teenagers
or adults? I really don't know the answer.


I am in favour of treating lyx users like adults. To me, this means not 
reinventing the wheel and follow established guidelines. See e.g.


https://developer.apple.com/library/content/documentation/Security/Conceptual/SecureCodingGuide/Articles/AppInterfaces.html

and the paper they mention which has a lot of examples.



I agree that it is late in the process, and indeed that does make
stronger the proposal of "let's just revert". But this issue is not the
only one holding up beta1. When we make progress on the other issues, if
this one is still hanging in the air and we cannot agree on what to do,
then we might need to move on and revert. My opinion is that we're not
there yet.


What is your schedule in either case for implementing and testing the
changes?



Re: Can shell-escape take advantage of needauth framework?

2017-06-28 Thread Guillaume MM

Le 27/06/2017 à 23:45, Tommaso Cucinotta a écrit :


needauth was a urgently needed mitigation of the security issues behind 
running
arbitrary external tools when compiling LyX documents; a more engineered 
remedy
AFAICR was actually the use of sandboxing machineries, which was 
prototyped on

Ubuntu/Linux using AppArmor.


This is also what I remember. The now secured converters were sweave and
knitr, introduced in 2011 and 2012.

I see that you have also introduced a gnuplot converter with an example.

+ Proportionality: unsafety is actually a main feature of gnuplot from
what I understand from http://www.yqcomputer.com/320_2475_1.htm
+ Specificity: only gnuplot is given elevated privileges, which is what
the user wants.
- UI problem 1: When I open the example, I immediately get the needauth
dialog for showing the preview. I thought we only wanted unsafe
execution when compiling the document.
- UI problem 2: If I have N scripts in the document, I am asked N times
and must press no N times. It misses a "Never execute" button.

This is in addition to other needauth shortcomings in its current state
already mentioned.

It seems to me that needauth, as it is, is not ready for the addition of
gnuplot. What do you think?

Guillaume



Re: Options for resolving the minted + shell-escape issue

2017-08-02 Thread Guillaume MM

Le 01/08/2017 à 10:50, Scott Kostyshak a écrit :

On Mon, Jul 31, 2017 at 09:07:11PM +0200, Guillaume MM wrote:


I am sure that Scott meant to include in some way the option that I have
been advocating constantly from the beginning, which I understand is
probably 1. (Otherwise, I do not see what the option 1. refers to nor
who proposed it, and I would opt for not taking part in the vote.)


Yes my intention was to represent your opinion in a simplified way. I'm
sorry to have failed that. You will have an opportunity to write your
own option for the next vote (see separate email).



Hi Scott,

That's alright, I think my opinion was represented well enough. Patch
0002 was disabling minted as per 1., and patches 0001 and 0003 at
<https://www.mail-archive.com/lyx-devel@lists.lyx.org/msg201383.html>
answer your more general question "how can we make LyX the most secure
for 2.3?". One can see it like that. 0001 takes various comments that
have been made about Enrico's patch by various people into account and
0003 fixes a small display issue. It just misses for now the enabling of
shell-escape via the improved needauth mechanism but this looks simple
enough to do, and after that it can be proposed as an improvement to
Enrico's mechanism.

The first thing we will remember about your handling of this discussion
is your commitment of rationality. Please do not let Enrico's new
attacks affect you.

Guillaume




Re: Blank buttons

2017-05-14 Thread Guillaume MM

Le 14/05/2017 à 23:25, Andrew Parsloe a écrit :

I was testing compilation of files with apostrophes in the filename.
When I had finished I deleted the files on disk forgetting they were
still open in LyX. The attached screenshot shows the choices LyX
presented me with (a difficult choice).

(alpha1-1, windows 7)




Hi Andrew, thanks for the report, I'll try to have it fixed for beta so
that you can test again.

For now, know that the first button is "Reload" and the second one "Ignore".

Guillaume



Re: #10400: keyboard shortcut for document settings?

2017-05-14 Thread Guillaume MM

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

On Fri, Apr 21, 2017 at 03:53:08PM -0400, Scott Kostyshak wrote:

On Fri, Apr 21, 2017 at 10:48:05AM +0200, Jean-Marc Lasgouttes wrote:

Le 21/04/2017 à 05:50, Scott Kostyshak a écrit :

Should we have a keyboard shortcut for document settings, as proposed at

  http://www.lyx.org/trac/ticket/10400

?

There is a proposal to use ctrl + p, which is now free because we got
rid of print.

I agree a shortcut would be nice. I have mixed feelings about the
particular shortcut ctrl + p, but I don't have another suggestion.


Ctrl+P is assigned to Print in most applications. I would rather re-use it
for View (default), which would become some print preview. However, I would
not say that Cttrl+R is a good shortcut for document settings, but Ctrl+P is
not either (unless we thin of settings as properties).


We should figure out which reasonable shortcuts are free.

As for using Ctrl + P for View (default), what is the smoothest way we
should make that transition?

idea 1.
unbind Ctrl + R and bind Ctrl + P and expect user complaints of broken
LyX.

idea 2.
For 2.3.0, bind both Ctrl + R and ctrl + P to View (default) and put a note 
that Ctrl +
R will be deprecated in favor of Ctrl + P in 2.4.0?
We could put the note in the release notes and the messages pane, each
time a user tries to do Ctrl + R. Of course, few will read the release
notes and few will have the messages pane open, but at least in 2.4.0 we
can say "we warned you this would happen".

idea 3.
For 2.3.x we can bind Ctrl + R to a warning that pops up that says the new key 
binding for View (default) is Ctrl + P.

Am I thinking too much about this?




I do not see an opportunity to change the existing shortcut Ctrl+R.

The denotation of an accelerator has to be accidental. In French it is 
"Paramètres", which fits with Ctrl+P.





Re: [LyX/master] Add acmart template

2017-05-14 Thread Guillaume MM

Le 14/05/2017 à 11:28, Scott Kostyshak a écrit :

On Sun, May 14, 2017 at 10:48:08AM +0200, Kornel Benko wrote:


Same as for you (e.g. failing)
If previewing I get pdf-luatex-output if requested (show nevertheless). The 
latex log exists and shows many errors.
So it is not only your machine.





Thanks for the tests. Can you please send me the logs? From today's 
version just in case.


Guillaume



Re: [LyX/master] rename buffer parameter math_number_before to math_numbering_side

2017-05-15 Thread Guillaume MM

Le 13/05/2017 à 20:39, Uwe Stöhr a écrit :

commit 0dd3311dd42996f9aede7d8c58b27937ebb48b54
Author: Uwe Stöhr 
Date:   Sat May 13 20:39:45 2017 +0200

rename buffer parameter math_number_before to math_numbering_side

this is a fileformat change

also try to fix an UI issue that JMarc gets
---
+def convert_mathnumberingname(document):
+" rename the \\math_number_before tag to \\math_numbering_side "
+document.warning("Malformed LyX document: Missing '\\end_inset' of Float 
inset.")


It looks like this warning shows up every time.



Re: [LyX/master] Fix compilation failure

2017-05-15 Thread Guillaume MM

Le 15/05/2017 à 10:31, Jean-Marc Lasgouttes a écrit :

commit 676ce147da9f5873b4177a0cc3dd706005dd0690
Author: Jean-Marc Lasgouttes 
Date:   Mon May 15 10:29:09 2017 +0200

Fix compilation failure
---
 src/frontends/qt4/qt_helpers.h |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)



Thanks Jean-Marc




Re: 2.3.0alpha1-1: file save

2017-05-10 Thread Guillaume MM

Le 10/05/2017 à 22:57, Stephan Witt a écrit :

Am 09.05.2017 um 21:19 schrieb john kennan :


Start a new file
type something
save
type some more
save

I get "the file ... changed on disk." with two buttons: "Reload" and "Ignore"

hitting Ignore leaves the file unsaved
hitting Reload works -- I get a warning:

Any changes will be lost. Are you sure you want to load the version on disk of 
the document .../Papers/LyX/test1.lyx?

with buttons for Revert and Cancel
Hitting Revert saves the file, as desired, and the changes are not lost.


Dear John, thanks for the report. This looks like LyX incorrectly
recording the modification it has made itself as an external
modification. This means that in fact the file is already correctly
saved when the message shows up, and the two buttons do nothing except
annoy. I am looking into it.

Does it happen only on the second save? Does it happen on the third save
if you carry on? Does it not happen on the first save? If not, is it
because the first save is a "Save As"?



Confirmed.


Dear Stephan, thanks for the test. This looks specific to OSX. I would
ask the same questions as above.



Guillaume, it’s the FileMonitor which is detecting the file save as external 
modification.
How can I provide further info to correct this annoying behavior?
My attempt to diagnose the culprit failed - the code is too weird for me :(

I tried to stop in Buffer::Impl::refreshFileMonitor() at the first line.
The debugger didn’t stop - but it stopped in 
Buffer::Impl::fileExternallyModified()
and the call stack claims it comes from the last line in refreshFileMonitor() 
???


As QFileSystemWatcher is intrinsically asynchronous, gdb does not really
help with debugging here. What you see on the trace is
QFileSystemWatcher calling the function that was passed to connect at
the end of refreshFileMonitor (via a qt signal that is converted into a
boost signal).

What should happen is inside Buffer::save there is a FileMonitorBlocker
that should block the signal. Now, since QFileSystemWatcher is
asynchronous, the signal is not received before the next iteration of
the even loop. At this point the FileMonitorBlocker has been destroyed a
long time ago. This is why FileMonitorBlocker unblocks the signal in an
asynchronous way too, using a QTimer. On Linux, a delay of 0 (meaning
wait until the next event loop) is enough to actually block the signal.

The delay is defined in Buffer::Impl::blockFileMonitor (Buffer.cpp:388)
currently at 10ms. Can you try to increase this value and see if that
helps? It will likely help, but this is not a nice solution.

Ideally I would like to have everything work with a delay of 0ms, to be
sure that everyone experiences the same. Can you help me and see if it
is possible to flush the file operations in FileName::copyTo and
FileName::moveTo and if it makes a difference ? There is QFile::flush
but I do not really see how to use it in this context and I cannot test
the situation.



Frankly said: I’m lost now.


For this bug and the other similar bug on the bugtracker [down currently
again] I thought about detecting false positives by comparing the file
hashes before setting the flag.

Before this drastic measure I would like to see if I can understand why
the signal comes too late for the blocker, and if it's just missing
something simple.


Guillaume





Re: [LyX/master] Pass thru overlay argument content.

2017-05-15 Thread Guillaume MM

Le 15/05/2017 à 17:38, Jürgen Spitzmüller a écrit :

Am Samstag, den 13.05.2017, 15:15 +0200 schrieb Guillaume MM:

Hi Jürgen, what do you think about changing the font of these
arguments
to monospace to better reflect the new behaviour? (and for changes
at 7f753440 too)


A good idea.

Jürgen


That would involve a lot of copy-paste into the layouts, but
alternatively one can make PassThru change the font automatically,
which
the user can always override later on.




Would you want to give it a go? If I were to do it, I would implement 
the second option above (and maybe not in time for 2.3).


Guillaume




Re: [LyX/master] Add acmart template

2017-05-15 Thread Guillaume MM

Le 14/05/2017 à 10:31, Scott Kostyshak a écrit :

On Sun, May 14, 2017 at 01:23:05AM +0200, Guillaume MM wrote:


There are more general issues with listsof. I asked the maintainer about
this.


Good to know this is a known issue -> inverted the test at ec27acb5.


I am curious to know which Tocs work out of the box in acmart.lyx
on recent texlive. Can anybody please try?

Guillaume



Re: updatelayouts.py broken?

2017-05-13 Thread Guillaume MM

Le 12/05/2017 à 16:55, José Abílio Matos a écrit :

On Monday, 24 April 2017 06.46.21 WEST Scott Kostyshak wrote:

I use

  development/tools/updatelayouts.py

to easily update the format of our layout files. The script no longer
works for me. I wonder if the script needs to be updated after 77511ea1
(I think the call to layout2layout is now incorrect). I tried to update
it myself, but my Python skills aren't even hackish enough to fix it
with trial and error.

This issue is not urgent at all. I can update the layout files with a
find -exec and calling layout2layout directly.

An alternative to the script being broken is that I just don't know how
to call it correctly. Jürgen updated all layouts at a3315733 (which is
more recent than 77511ea1), but perhaps he used a find -exec also?

Scott


Hi Scott,
do you still see this problem?

Regards,



Hi José,

I experienced updatelayouts.py not working as I expected just two days 
ago and now Scott's message that you quote makes me realize that it 
might indeed have an issue.


Regards,
Guillaume



Re: 2.3.0alpha1-1: file save

2017-05-13 Thread Guillaume MM

Le 11/05/2017 à 07:46, Stephan Witt a écrit :


On the first save LyX asks for a file name because it’s a new file.
This in fact is a Save-As operation. The save succeeds normal.
After that every change+save is accompanied by the message popup.
NB: I’m accepting the file name proposal - so it’s the same file name
before and after the save-as.


Thanks.




Guillaume, it’s the FileMonitor which is detecting the file save as external 
modification.
How can I provide further info to correct this annoying behavior?
My attempt to diagnose the culprit failed - the code is too weird for me :(

I tried to stop in Buffer::Impl::refreshFileMonitor() at the first line.
The debugger didn’t stop - but it stopped in 
Buffer::Impl::fileExternallyModified()
and the call stack claims it comes from the last line in refreshFileMonitor() 
???


As QFileSystemWatcher is intrinsically asynchronous, gdb does not really
help with debugging here. What you see on the trace is
QFileSystemWatcher calling the function that was passed to connect at
the end of refreshFileMonitor (via a qt signal that is converted into a
boost signal).


That makes sense. BTW, what’s the semantic of a void C++ function when
it returns a value (2nd line after the if())? I’m surprised it’s allowed.


I am not surprised that it is allowed (void is a type and here it
type-checks), but indeed this is not how I write usually. This
looks like something that used to return a bool at some point. I'll
correct it.

Guillaume



Re: 2.3.0alpha1-1: file save

2017-05-13 Thread Guillaume MM

Le 11/05/2017 à 09:59, Stephan Witt a écrit :


The delay is defined in Buffer::Impl::blockFileMonitor (Buffer.cpp:388)
currently at 10ms. Can you try to increase this value and see if that
helps? It will likely help, but this is not a nice solution.


I tried it with a delay of 100ms. The effect is now LyX presents the external
modification message box every 2nd or 3rd save. So it is not the solution.


Thanks for the test, this helps. BTW is there a delay before the message
appears and if so how long? Do you see the following error message with
the files debug flag:
  LYXERR(Debug::FILES, "Could not add path to QFileSystemWatcher: " <<
filename_);
?





Ideally I would like to have everything work with a delay of 0ms, to be
sure that everyone experiences the same. Can you help me and see if it
is possible to flush the file operations in FileName::copyTo and
FileName::moveTo and if it makes a difference ? There is QFile::flush
but I do not really see how to use it in this context and I cannot test
the situation.


I cannot see how to do it here.

The save operation in this scenario is done with one create (temp), one
rename (backup) and another rename (final name).

Are you sure the file monitor is able to follow these steps?


What is expected to happen is:

In Buffer::save:

1. Disconnect from QFileSystemWatcher
2. Perform various file operations
3. Queue the reconnection to QFileSystemWatcher

Later:

4. QFileSystemWatcher notifies of the deletion
5. The new file is moved
6. Reconnect to QFileSystemWatcher and refresh (watch the new file)

Given the behaviour that you report above, it seems that the
reconnection happens too early. The number and the nature of the
operations is not important.

I notice that the QSaveFile class which is provided by Qt (but not used
in LyX) calls a function fileEngine->syncToDisk() not accessible from
the API, and below there are the comments:
// atomically replace old file with new file
// Can't use QFile::rename for that, must use the file engine directly

I suspect that using QSaveFile instead of the current implementation
would solve the bug and make file saving safer (even).

Can you please try the attached patch? Before deciding which solution is
the best I will still need to know the answer to the questions at the
beginning, if you please can help with that.

Thank you
Guillaume

>From 17f644c2cc1c21020aa71215762941930689f951 Mon Sep 17 00:00:00 2001
From: Guillaume MM <g...@lyx.org>
Date: Sat, 13 May 2017 01:00:30 +0200
Subject: [PATCH] Prevent false positives in external modifications

When the Buffer is notified to be externally modified, check that the
file contents have changed using the checksum.
---
 src/Buffer.cpp | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/Buffer.cpp b/src/Buffer.cpp
index 239bacd..60619dc 100644
--- a/src/Buffer.cpp
+++ b/src/Buffer.cpp
@@ -386,7 +386,7 @@ public:
 	void fileExternallyModified(bool modified) const;
 
 	/// Block notifications of external modifications
-	FileMonitorBlocker blockFileMonitor() { return file_monitor_->block(10); }
+	FileMonitorBlocker blockFileMonitor() { return file_monitor_->block(); }
 
 private:
 	/// So we can force access via the accessors.
@@ -5340,8 +5340,11 @@ void Buffer::Impl::refreshFileMonitor()
 
 void Buffer::Impl::fileExternallyModified(bool modified) const
 {
-	if (modified)
+	if (modified) {
+		if (filename.exists() && checksum_ == filename.checksum())
+			return;
 		lyx_clean = bak_clean = false;
+	}
 	externally_modified_ = modified;
 	if (wa_)
 		wa_->updateTitles();
-- 
2.7.4



Re: [LyX/master] Pass thru overlay argument content.

2017-05-13 Thread Guillaume MM

Le 18/02/2017 à 08:48, Juergen Spitzmueller a écrit :

commit 5b03606482bfd153dc97442163de19216e7c5ad0
Author: Juergen Spitzmueller 
Date:   Sat Feb 18 08:48:15 2017 +0100

Pass thru overlay argument content.
---
 lib/layouts/beamer.layout |   38 ++
 1 files changed, 38 insertions(+), 0 deletions(-)




Hi Jürgen, what do you think about changing the font of these arguments
to monospace to better reflect the new behaviour? (and for changes
at 7f753440 too)

That would involve a lot of copy-paste into the layouts, but
alternatively one can make PassThru change the font automatically, which
the user can always override later on.

Guillaume



Re: Regression in child document referencing (alpha1-1)

2017-05-13 Thread Guillaume MM

Le 14/05/2017 à 00:59, Andrew Parsloe a écrit :

I've attached a mwe consisting of a master and two child documents.
References in Child-1 to Child-2 are labelled BROKEN. References in the
other direction are OK. Reversing the order in which Child-1 and Child-2
are inserted in Master shows refs in Child-2 to Child-1 are now BROKEN
while the Child-1 refs to Child-2 are OK.

Summarising: refs from one child doc to a *later* child doc are BROKEN.

I created a similar trio of documents in 2.2.2 and the problem is not
present, so this is a regression (at least with my installation on
windows 7).

Andrew




Thanks, I'll have a look.

Guillaume




Re: [LyX/master] Add acmart template

2017-05-13 Thread Guillaume MM

Le 13/05/2017 à 18:11, Scott Kostyshak a écrit :

On Sat, May 13, 2017 at 04:19:13PM +0200, Guillaume MM wrote:

commit 5608f6fdb67b86b4cf3d9215d24d7734239ad05a
Author: Guillaume MM <g...@lyx.org>
Date:   Sat May 13 16:00:57 2017 +0200

Add acmart template

Move obsolete templates to templates/obsolete


Thanks for your work on this, Guillaume!


Thanks for the report Scott.



Several of the ctests are failing. I haven't updated TeX Live for a few
weeks so perhaps that explains things. The following tests fail for me:

export/templates/acmart_lyx16 (Failed)
export/templates/acmart_lyx21 (Failed)
export/templates/acmart_lyx22 (Failed)
check_load/templates/acmart (Failed)
export/templates/acmart_dvi3_systemF (Failed)
export/templates/acmart_pdf (Failed)
export/templates/acmart_pdf4_systemF (Failed)
export/templates/acmart_pdf5_systemF (Failed)

For check_load, this is because when opening the file in LyX, I see the
terminal message:

TextClass.cpp (1385): The layout does not provide a list command for
the float `sidebar'. LyX will not be able to produce a float list.


There are more general issues with listsof. I asked the maintainer about
this.



For pdf4_systemF (XeTeX with system fonts), I get:


!
! LaTeX error: "xparse/command-already-defined"
!
! Command '\liningnums' already defined!
!
! See the LaTeX3 documentation for further information.
!
! For immediate help type H .
!...



Strange, all the outputs you listed above work here.


We don't need for all of the tests to pass. But it's good to know why
they fail before we ignore/invert them. Also, we might want to put
explanations in the template, e.g. "this template is not compatible with
XeTeX and system fonts because ..."

Scott






Re: 2.3.0alpha1-1: file save

2017-05-15 Thread Guillaume MM

Le 14/05/2017 à 19:48, Stephan Witt a écrit :

Am 13.05.2017 um 08:19 schrieb Guillaume MM <g...@lyx.org>:


Le 11/05/2017 à 09:59, Stephan Witt a écrit :


The delay is defined in Buffer::Impl::blockFileMonitor (Buffer.cpp:388)
currently at 10ms. Can you try to increase this value and see if that
helps? It will likely help, but this is not a nice solution.


I tried it with a delay of 100ms. The effect is now LyX presents the external
modification message box every 2nd or 3rd save. So it is not the solution.


Thanks for the test, this helps. BTW is there a delay before the message
appears and if so how long? Do you see the following error message with
the files debug flag:
 LYXERR(Debug::FILES, "Could not add path to QFileSystemWatcher: " <<
filename_);
?


This is the output of the un-patched LyX on save with false-positive message:

support/FileName.cpp (605): Checksumming 
"/Users/stephan/Documents/newfile12.lyx" 3874605062 lasted 1 ms.
support/TempFile.cpp (35): Temporary file in 
/Users/stephan/Documents/newfile12-XX.lyx
support/TempFile.cpp (38): Temporary file 
`/Users/stephan/Documents/newfile12-P37773.lyx' created.
Buffer.cpp (1435): Saving to /Users/stephan/Documents/newfile12-P37773.lyx
BufferParams.cpp (321): Checking whether document is in a system dir... no
support/FileName.cpp (605): Checksumming 
"/Users/stephan/Documents/newfile12.lyx" 3874605062 lasted 1 ms.
Buffer.cpp (1461): Backing up original file to 
/Users/stephan/Documents/newfile12.lyx~
support/FileName.cpp (267): Moving /Users/stephan/Documents/newfile12.lyx~ to 
/Users/stephan/Documents/newfile12.lyx
support/FileName.cpp (267): Moving /Users/stephan/Documents/newfile12.lyx to 
/Users/stephan/Documents/newfile12-P37773.lyx
support/FileName.cpp (605): Checksumming 
"/Users/stephan/Documents/newfile12.lyx" 2474078819 lasted 1 ms.


Thanks, this is as expected. Can I ask again, is there a delay between
saving and the time when the notification appears, and if so how long?


Can you please try the attached patch?


This is the output of the patched one - no false-positive message:
support/FileName.cpp (605): Checksumming 
"/Users/stephan/Documents/newfile13.lyx" 2101337338 lasted 0 ms.
support/TempFile.cpp (35): Temporary file in 
/Users/stephan/Documents/newfile13-XX.lyx
support/TempFile.cpp (38): Temporary file 
`/Users/stephan/Documents/newfile13-B38676.lyx' created.
Buffer.cpp (1435): Saving to /Users/stephan/Documents/newfile13-B38676.lyx
BufferParams.cpp (321): Checking whether document is in a system dir... no
support/FileName.cpp (605): Checksumming 
"/Users/stephan/Documents/newfile13.lyx" 2101337338 lasted 1 ms.
Buffer.cpp (1461): Backing up original file to 
/Users/stephan/Documents/newfile13.lyx~
support/FileName.cpp (267): Moving /Users/stephan/Documents/newfile13.lyx~ to 
/Users/stephan/Documents/newfile13.lyx
support/FileName.cpp (267): Moving /Users/stephan/Documents/newfile13.lyx to 
/Users/stephan/Documents/newfile13-B38676.lyx
support/FileName.cpp (605): Checksumming 
"/Users/stephan/Documents/newfile13.lyx" 834715601 lasted 2 ms.
support/FileName.cpp (605): Checksumming 
"/Users/stephan/Documents/newfile13.lyx" 834715601 lasted 0 ms.



Thanks for testing. Again the trace is as expected. Good to know that it
works.



And this is for a big document (on a SSD based machine):

support/FileName.cpp (605): Checksumming 
"/Users/stephan/git/lyx/lib/doc/de/UserGuide.lyx" 1327983642 lasted 35 ms.
support/TempFile.cpp (35): Temporary file in 
/Users/stephan/git/lyx/lib/doc/de/UserGuide-XX.lyx
support/TempFile.cpp (38): Temporary file 
`/Users/stephan/git/lyx/lib/doc/de/UserGuide-X38676.lyx' created.
Buffer.cpp (1435): Saving to 
/Users/stephan/git/lyx/lib/doc/de/UserGuide-X38676.lyx
BufferParams.cpp (315): Checking whether document is in a system dir... yes
support/FileName.cpp (605): Checksumming 
"/Users/stephan/git/lyx/lib/doc/de/UserGuide.lyx" 1327983642 lasted 37 ms.
Buffer.cpp (1461): Backing up original file to 
/Users/stephan/git/lyx/lib/doc/de/UserGuide-lyxformat-541.lyx~
support/FileName.cpp (267): Moving 
/Users/stephan/git/lyx/lib/doc/de/UserGuide-lyxformat-541.lyx~ to 
/Users/stephan/git/lyx/lib/doc/de/UserGuide.lyx
support/FileName.cpp (267): Moving 
/Users/stephan/git/lyx/lib/doc/de/UserGuide.lyx to 
/Users/stephan/git/lyx/lib/doc/de/UserGuide-X38676.lyx
support/FileName.cpp (605): Checksumming 
"/Users/stephan/git/lyx/lib/doc/de/UserGuide.lyx" 582248824 lasted 35 ms.
support/FileName.cpp (605): Checksumming 
"/Users/stephan/git/lyx/lib/doc/de/UserGuide.lyx" 582248824 lasted 37 ms.



Good to know that the time to check the sum is reasonable for a big 
document. For slow file systems one can expect the OS cache to help in 
this precise situation.


The best solution would be to reimplement the saving with QSaveFile
+ Safer save mechanism
- Not sure it fix

Re: [LyX/master] Add acmart template

2017-05-15 Thread Guillaume MM

Le 14/05/2017 à 14:41, Kornel Benko a écrit :

Am Sonntag, 14. Mai 2017 um 12:17:09, schrieb Guillaume MM <g...@lyx.org>

Le 14/05/2017 à 11:28, Scott Kostyshak a écrit :

On Sun, May 14, 2017 at 10:48:08AM +0200, Kornel Benko wrote:


Same as for you (e.g. failing)
If previewing I get pdf-luatex-output if requested (show nevertheless). The 
latex log exists and shows many errors.
So it is not only your machine.





Thanks for the tests. Can you please send me the logs? From today's
version just in case.

Guillaume


I am sending privately. BTW, the errors remained.



Thanks.

It seems that many errors come from the algorithm2e package being out of
date. Then does it works with pdflatex already?

For other errors it would be useful to try the latest acmart.cls version.

I do not understand the rest of the errors and cannot test, so I'll just
suggest to invert the tests for now. Lets hope it fixes itself with time.

Guillaume



Re: Regression in child document referencing (alpha1-1)

2017-05-16 Thread Guillaume MM

Le 14/05/2017 à 00:59, Andrew Parsloe a écrit :

I've attached a mwe consisting of a master and two child documents.
References in Child-1 to Child-2 are labelled BROKEN. References in the
other direction are OK. Reversing the order in which Child-1 and Child-2
are inserted in Master shows refs in Child-2 to Child-1 are now BROKEN
while the Child-1 refs to Child-2 are OK.

Summarising: refs from one child doc to a *later* child doc are BROKEN.

I created a similar trio of documents in 2.2.2 and the problem is not
present, so this is a regression (at least with my installation on
windows 7).



This is fixed now.





Re: Blank buttons

2017-05-16 Thread Guillaume MM

Le 14/05/2017 à 23:25, Andrew Parsloe a écrit :

I was testing compilation of files with apostrophes in the filename.
When I had finished I deleted the files on disk forgetting they were
still open in LyX. The attached screenshot shows the choices LyX
presented me with (a difficult choice).

(alpha1-1, windows 7)

Andrew




This should be fixed now. Testing appreciated.





Re: Can shell-escape take advantage of needauth framework?

2017-06-19 Thread Guillaume MM

Le 19/06/2017 à 15:39, Enrico Forestieri a écrit :

On Mon, Jun 19, 2017 at 06:39:22AM +0200, Jürgen Spitzmüller wrote:


Am Sonntag, den 18.06.2017, 19:56 +0200 schrieb Enrico Forestieri:

I think we need to provide an option to add -shell-escape only to
specific documents and only on the given machine. This prevents
sending
documents with -shell-escape (main problem of a document setting).


This is contradictory. We avoid sending documents with -shell-escape
but then add it to specific documents. So, it is the same thing.


No, it isn't. I didn't propose a document property, but a per-document
session setting. This is a completely different thing.


Sorry, it was not clear to me what you meant. Here is a patch following
this strategy.

- We never store in the document the need for -shell-escape.
- When the user checks the toolbar button and then runs a latex backend,
   he is alerted that the backend will be allowed to run external programs.
- At this point, he can decide to let the backend run (and be asked again
   next time), or to always allow execution with -shell-escape for this doc.
- If the user chooses to always allow -shell-escape for the current document,
   the document path is stored in the session file, so that next time it is
   loaded on the current machine, the toolbar button will be automatically
   toggled and no question will be asked.
- If the user manually toggles the toolbar button so that to disallow the
   -shell-escape option for an authorized document, the document is
   automatically removed from the list of authorized documents.

This patch does not introduce a format change, because nothing is recorded
in the document (the document status is only recorded in the session file).




If I understand correctly, this is the latest proposal for hard-wiring
the "-shell-escape" option when running the child latex processes, so I
will comment on this one. But I could write almost the same for all the
other proposals I have read so far.

Let me summarize.

Pygments: a python software that does some valuable processing on
certain text inputs.

minted.sty: a LaTeX interface for Pygments. For some convenience reason,
minted.sty calls Pygments using the \write18 command. \write18 lets one
execute arbitrary commands on the console so it is disabled by default.
The -shell-escape option is necessary for using minted, it enables
\write18 but then \write18 can be used anywhere in the document.

Enrico's patch: add the -shell-escape option when some conditions are
met, overriding the values shown to the user in the converter
preferences. The idea is that the user has the ability to "trust" a
document (roughly a different name for needauth, with small differences
in the details such as the ability to revoke the trust given, which is
an improvement compared to needauth).

First I note that minted.sty executes an external tool contrary to the
LaTeX tradition. External tools are used all the time, but usually the
user is responsible for calling the external tools themselves before
calling LaTeX again. And the small convenience gain of doing it the
minted way is irrelevant to LyX users given that LyX is capable of
automating the process of calling Pygments. Here I tell you nothing new
about LaTeX and LyX. And in fact, there already exists an interface to
Pygments that works the LaTeX way: pygmentex.sty. (There also were
difficulties in guessing whether minted.sty will be able to find
Pygments, this is also no longer an issue if LyX is in charge of calling
Pygments.)

Second, the design seems to be based on elaborate assumptions about the
user's usage and their behaviour. I would like to see the arguments
being are in principles of secure usability, which is a topic of
academic research with available articles and textbooks. See e.g.:

https://dl.acm.org/citation.cfm?id=687663
http://shop.oreilly.com/product/9780596008277.do

At a cursory glance, the proposed mechanism violates several principles:

* Prompting the user to give up security before anything happens. This
is equivalent to having no dialog at all the second or third time it
appears if the user depends on it, because they will only think about it
the first time if at all (think of "security exceptions" in your browser),
* Running child processes with more privileges than necessary,
* Forcing all-or-nothing choices (e.g. one needs to trust the whole
document instead of just minted),
* What is trusted can change over time.

I am further less convinced because the relationship between usability
issues and security has already been pointed out by Helge and I would
have like to see his points being taken into account in the discussion.

Until these needauth mechanisms (or whatever they are called) are
designed with the needs of users in mind following established
principles, their purpose will be for the developer to shift blame to
the user in case something bad happens. (For developers they also make
it safer to open random lyx files from 

Re: [LyX/master] Fix bug #9101

2017-06-19 Thread Guillaume MM

Le 08/06/2017 à 04:28, Enrico Forestieri a écrit :


Actually it was simpler than expected.



Thanks.



Re: [LyX/master] Use otexstringstream for the captions of InsetCaptionables

2017-06-19 Thread Guillaume MM

Le 11/06/2017 à 01:08, Enrico Forestieri a écrit :

On Mon, Oct 17, 2016 at 00:16:21AM +0200, Guillaume Munch wrote:


commit 676a0639c505d52336e3228c44c2515ccbbaf34a
Author: Guillaume Munch 
Date:   Sat Sep 24 00:49:00 2016 +0200

 Use otexstringstream for the captions of InsetCaptionables
 
 * Enable TexRow for InsetListings caption.
 
 * Move getCaption* from InsetText to InsetCaptionable.
 
 * Clean-up caption generation for InsetFloat.

[...]

@@ -420,7 +417,11 @@ docstring InsetListings::getCaption(OutputParams const & 
runparams) const
// NOTE that } is not allowed in blah2.
regex const reg("(.*)label\\{(.*?)\\}(.*)");
string const new_cap("$1$3},label={$2");
-   return from_utf8(regex_replace(to_utf8(cap), reg, new_cap));
+   // TexString validity: the substitution preserves the number of 
newlines.
+   // Moreover we assume that $2 does not contain newlines, so that the 
texrow
+   // information remains accurate.
+   cap.str = from_utf8(regex_replace(to_utf8(cap.str), reg, new_cap));
+   return cap;
  }
  
  


This commit broke the caption handling in InsetListings. For example,
the document attached at #9382:
https://www.lyx.org/trac/raw-attachment/ticket/9382/problem.20_nopreamble.lyx
now fails to compile. It can be compiled again with the attached patch,
which is not the right one, of course.




Dear Enrico,


I had a look at this issue 8 days ago since you pointed at my commit. I
realised then that my commit did nothing to worsen or improve this
issue (the commit is meant to leave the output unchanged, and the bug
appears in LyX 2.2.2 where this line of commits has never been). Sorry
for the wait: I did not have much time on my hands and it was not a
regression for me so I decided to postpone my reply until I had more time.

I now see that you are proposing a patch at
. I have several questions about
your patch.

Given that you had located a regression (you even point to a specific
part of the code), have you tried to see what was causing the regression
in the commit while coming up with a patch? I think that trying to do so
would show you that the commit you are pointing to actually leaves the
output unchanged.

In your bug report you notice that the bug is present in stable and you
say to Scott that the patch is valid for stable. It does not apply. Why
did you let Scott think that you have tested your patch on stable? I
think that if you had, then you could have noticed that the commit was
never backported.

As you know, you are writing to a developers list where people are
willing to put in their valuable time to help you, and expect accurate
technical informations. It happens to make a mistake, but it does not
make sense to me that with enough care you did not find an occasion to
correct yourself.

I want to be able to trust technical claims that are made on the list in
the future, and so do other developers probably. I try to always
double-check technical claims I make and sometimes it helps me catch
errors at the last moment. I would like to be sure that we agree on
everybody putting in the same care in their contributions.


Guillaume



Re: [LyX/2.2.x] Fix bugs #9598 and #10650

2017-06-19 Thread Guillaume MM

Le 08/06/2017 à 02:07, Enrico Forestieri a écrit :

On Thu, Jun 08, 2017 at 12:50:19AM +0200, Guillaume MM wrote:


Le 05/06/2017 à 23:15, Enrico Forestieri a écrit :

commit 59c22bd7b604a3ba9e0e78f7c51cb601f08d0192
Author: Enrico Forestieri<for...@lyx.org>
Date:   Mon Jun 5 23:14:48 2017 +0200

  Fix bugs #9598 and #10650
---



+// gcc < 4.8.0 and msvc < 2015 do not support C++11 thread_local
+#if defined(__GNUC__) && ((__GNUC__ == 4 && __GNUC_MINOR__ < 8) || __cplusplus 
< 201103L)
+#define THREAD_LOCAL_STATIC static __thread
+#elif defined(_MSC_VER) && ((_MSC_VER < 1900) || __cplusplus < 201103L)
+#define THREAD_LOCAL_STATIC static __declspec(thread)
+#else
+#define THREAD_LOCAL_STATIC thread_local static
+#endif
+



According to Stephan in this discussion:

https://www.mail-archive.com/lyx-devel@lists.lyx.org/msg196176.html

it is unfortunately not possible to use thread_local on Mac before some
time, it seems. I would actually be happy to hear the contrary.


Note that I also added the check for __cplusplus < 201103L, which should
assure full compliance with C++11.


Only in a perfect world. Note also that for clang you are always taking
the first branch so the value of __cplusplus is irrelevant. If
intentional, I suggest that you make it clear in the comment.


If thread_local is not supported, the
compiler should not set __cplusplus to such value or higher and thus
should use the fallback definition.


Sorry, this does not make sense to me. First, all of your definitions
use some form of thread-local storage. The problem referred to above is
that there is no implementation of it at all, even called __thread or
something else. You do not acknowledge that there is an issue, so it
is not clear to me that you have read the discussion mentioned above
carefully. To the best of our knowledge, in the best case your current
patch requires a discussion about dropping the support of Xcode < 8.





(Also, it had been decided that LyX requires msvc ≥ 2015 so the second
branch would not be necessary.)


I don't think that LyX really cannot be compiled with older versions.
Are there reports in this regard?



There was a discussion at
<https://www.mail-archive.com/lyx-devel@lists.lyx.org/msg194628.html>
and parent messages, and maybe someplace else as well. Since my patch on
Unicode string literals alluded to in that message is not going to be
committed before 2.4 at least, it could make sense to support MSVC 2013
in 2.3 if it currently works. What worries me most is lack of support
for <http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2008/n2660.htm>
("magic statics") for which your criteria “does it compile?” is not
going to be enough.



Re: Can shell-escape take advantage of needauth framework?

2017-06-20 Thread Guillaume MM

Le 20/06/2017 à 09:54, Jürgen Spitzmüller a écrit :

2017-06-20 2:43 GMT+02:00 Guillaume MM <g...@lyx.org <mailto:g...@lyx.org>>:

...

An alternative is provided by the possibility to add 
pygmentize to the list of "trusted commands", but this is something 
users need to do themselves.


This is interesting. However after looking at minted.sty I am convinced
that it cannot work unfortunately because it calls many different
commands, including ones such as rm.

Note that the trust only applies to the current document as stored in 
the current path, since we store absolute path names in sessions. So 
trusting one document foo.lyx does not give trust to all files named 
foo.lyx.


Yes this was clear enough.

The "LaTeX tradition" (up to TL 2002) was to allow all sorts of commands 
via \write18. Only then, restricted shell access was introduced (which 
is, of course, a good thing):

https://tex.stackexchange.com/questions/76105/what-does-restricted-write18-enabled-mean-and-why-does-texlive-keep-reporting

Just to set the records straight.


Ok. In addition, there are many external tool processing some input into
some LaTeX-readable output and this is the first time the question of
implementing -shell-escape in LyX for such a tool arises.



I cannot comment on the quality of pygmentex vs. minted, but here's an 
opinion on that:

https://tex.stackexchange.com/questions/102596/minted-vs-texments-vs-verbments


While I believe that the question of providing the package most popular
at a certain point in time vs. a good enough implementation is secondary
to security implications, I also inquired at
<https://github.com/gpoore/minted/issues/166> whether it would be hard
to implement 3-step compilation in minted.sty.



At a cursory glance, the proposed mechanism violates several principles:

* Prompting the user to give up security before anything happens. This
is equivalent to having no dialog at all the second or third time it
appears if the user depends on it, because they will only think about it
the first time if at all (think of "security exceptions" in your
browser),


I am not convinced by this argument.

* Running child processes with more privileges than necessary,


If we do not grant shell-escape, we run with less privileges than necessary.


I agree that the issue is rather for users who need Pygments and are
thereby forced to grant -shell-escape.



* Forcing all-or-nothing choices (e.g. one needs to trust the whole
document instead of just minted),


I would rather trust a document (I have written myself) than a program 
(I haven't).


But you would be trusting both.



* What is trusted can change over time.


Sure.

...

But the point is that we currently encourage users to enable an
unsure option _globally_ just for the sake of on document that needs
a specific feature.

Apart from the new minted.sty implementation, where else does LyX
encourages to enable an unsafe option globally?



I do not know the differences in features between minted.sty and
pygmentex.sty nor how long it would take to design and implement an
interface to Pygments directly in LyX, but this is irrelevant next to
the security implications of relying on minted.sty. One must look at the
big picture and see that adding an authorization mechanism for arbitrary
execution of commands is absurd when its sole purpose is to call an
external tool from within LaTeX. My conclusion is that the current
support for Pygments must be set aside, and (after 2.3) another solution
devised which does not relies on minted.sty.


As the discussion currently stands, this (removing minted support for 
the time being) will probably be the only option. With the result, 
though, that users who need it will have to use ERT and extensively care 
about whether to enable or disable the -shell-escape flag or nor. Which 
brings us to the original problem that we look away and let users run 
into trouble.


I now prefer to distinguish the two issues.

The case of the current Pygments implementation in LyX is specific since
there is no useful reason that implementing Pygments should require
-shell-escape. A decision to support a particular package is forever,
therefore the good choice has to be made from the start. Implementing it
while avoiding the security hazards of minted.sty would be a real
service made to the user.

The lack of a safe interface to -shell-escape for cases where this is
needed seems an old issue, and cannot be done in a hurry. In addition,
from Scott's original post I get that third-parties currently
encouraging the addition of -shell-escape could now explain how to make
it use needauth, or am I missing something? In that case the situation
is already improved in 2.3.



Lastly, I find interesting the idea of a "secure" icon providing visual
feedback and the ability to revoke the permissions, and 

Re: dash patch for stable

2017-06-19 Thread Guillaume MM

Le 03/06/2017 à 01:08, Enrico Forestieri a écrit :

On Fri, Jun 02, 2017 at 07:30:34PM +, Guenter Milde wrote:


Note that the "ERT patch" is only for *stable*, i.e. 2.2.x.
For 2.2, ERT is the only way to ensure full backwards compatibility.


A solution should have been thought and done before 2.2.0 was released.
Now it's too late.



Enrico, I also have to criticise this sort of comment of yours. I am
sorry to say that to me it looks disconnected with the understanding of
the problem at the time and gratuitous.

Guillaume



Re: [LyX/master] Fixup 522516d9 : editable() is unusable in mathed

2017-06-20 Thread Guillaume MM

Hi Jean-Marc,

There is the case of InsetMathRef which has nargs()!=0 but whose cell is 
not active. See e.g. the fix at 7337c968. Is the logic still valid in 
this case? Just in case.


Guillaume



Le 20/06/2017 à 09:44, Jean-Marc Lasgouttes a écrit :

commit d0acc3e570447b293169b8bdd5ac67aaade189e0
Author: Jean-Marc Lasgouttes 
Date:   Tue Jun 20 09:41:48 2017 +0200

 Fixup 522516d9 : editable() is unusable in mathed
 
 This is a relic from IU (Inset Unification): editable() is for text

 insets and isActive() for mathed. This needs to be cleaned up.
 
 Part of bug #10667.

---
  src/DocIterator.cpp |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/DocIterator.cpp b/src/DocIterator.cpp
index 3d9bd68..fe1250f 100644
--- a/src/DocIterator.cpp
+++ b/src/DocIterator.cpp
@@ -562,7 +562,7 @@ bool DocIterator::fixIfBroken()
size_t n = slices_.size();
for (; i != n; ++i) {
CursorSlice & cs = slices_[i];
-   if (() != inset || !cs.inset().editable()) {
+   if (() != inset || cs.nargs() == 0) {
// the whole slice is wrong, chop off this as well
--i;
LYXERR(Debug::DEBUG, "fixIfBroken(): inset changed");






Re: [LyX/master] Fixup 522516d9 : editable() is unusable in mathed

2017-06-25 Thread Guillaume MM

Le 23/06/2017 à 20:38, Jean-Marc Lasgouttes a écrit :

Le 20/06/2017 à 23:31, Guillaume MM a écrit :

Hi Jean-Marc,

There is the case of InsetMathRef which has nargs()!=0 but whose cell 
is not active. See e.g. the fix at 7337c968. Is the logic still valid 
in this case? Just in case.


Hi Guillaume,

What do you think of this one instead? I don't want to commit it and fix 
it up a third time :)


I think that it is consistent with your comments :) sorry I do not have
anything further useful to say about it. I agree that it would be good
to see if isActive and editable could be merged.

Guillaume



Re: [LyX/2.2.x] Fix bugs #9598 and #10650

2017-06-25 Thread Guillaume MM

Le 08/06/2017 à 26:72, Enrico Forestieri a écrit :
> On Thu, Jun 08, 2017 at 12:50:19AM +0200, Guillaume MM wrote:
>
>> Le 05/06/2017 à 23:15, Enrico Forestieri a écrit :
>>> commit 59c22bd7b604a3ba9e0e78f7c51cb601f08d0192
>>> Author: Enrico Forestieri<for...@lyx.org>
>>> Date:   Mon Jun 5 23:14:48 2017 +0200
>>>
>>>   Fix bugs #9598 and #10650
>>> ---
>>
>>> +// gcc < 4.8.0 and msvc < 2015 do not support C++11 thread_local
>>> +#if defined(__GNUC__) && ((__GNUC__ == 4 && __GNUC_MINOR__ < 8) || 
__cplusplus < 201103L)

>>> +#define THREAD_LOCAL_STATIC static __thread
>>> +#elif defined(_MSC_VER) && ((_MSC_VER < 1900) || __cplusplus < 
201103L)

>>> +#define THREAD_LOCAL_STATIC static __declspec(thread)
>>> +#else
>>> +#define THREAD_LOCAL_STATIC thread_local static
>>> +#endif
>>> +
>>
>>
>> According to Stephan in this discussion:
>>
>> https://www.mail-archive.com/lyx-devel@lists.lyx.org/msg196176.html
>>
>> it is unfortunately not possible to use thread_local on Mac before some
>> time, it seems. I would actually be happy to hear the contrary.
>
> Note that __thread appears to be supported since OSX 10.7, and
> clang always takes the first branch because for historical reasons it
> currently defines __GNUC__ = 4 and __GNUC_MINOR__ = 2, so it
> should work on 10.7 and above. But I agree that this is confusing
> and fragile, so I will clarify the code so as not to lay a trap
> for the developers who are going to come after me.
>

Ok, good to know, thanks.

Guillaume



Re: [LyX/2.2.x] Fix bugs #9598 and #10650

2017-06-25 Thread Guillaume MM
so as not to lay a trap for the developers who are going to come 
after me.



"to maintain the code after me" (avoiding any unintentional double
meaning due to my uncolloquial English).



Re: Can shell-escape take advantage of needauth framework?

2017-06-25 Thread Guillaume MM

Le 21/06/2017 à 07:15, Guillaume MM a écrit :


While I believe that the question of providing the package most popular
at a certain point in time vs. a good enough implementation is secondary
to security implications, I also inquired at
<https://github.com/gpoore/minted/issues/166> whether it would be hard
to implement 3-step compilation in minted.sty.



A quick update: there is now a reply on the ticket above. In addition I 
received the following reply from the author of minted.sty.




I agree that -shell-escape is problematic, especially in a case like
LyX where the raw LaTeX code can be less visible. I will be happy to
add a 3-step compile option; the only issue will be finding time to
do it. I think the Python side of doing this would be trivial. The
main work would need to be in LaTeX code in minted.sty. I've provided
some additional technical details (if you want them) in the GitHub
issue.




Re: [coverity again] missing move constructors

2017-05-24 Thread Guillaume MM

Le 24/05/2017 à 04:59, Scott Kostyshak a écrit :

On Tue, May 09, 2017 at 04:25:15PM +0200, Jean-Marc Lasgouttes wrote:

Le 21/04/2017 à 00:11, Guillaume MM a écrit :

Le 08/04/2017 à 23:05, Jean-Marc Lasgouttes a écrit :



FileName:
This would be automatically copyable and movable if not for the use of
the pointer to implementation.


What is the problem with the pointer?


For motivations see for instance
<https://oliora.github.io/2015/12/29/pimpl-and-rule-of-zero.html>.


The spimpl template declared there looks good to me. There is no problem
with distributing Boost licenced stuff with LyX, we do it already.

Concerning the patches, thery are in general way above my head, and I trust
your judgment. My remarks are
* it is not nice to have to use unique_ptr allover the place, I
do not really care about implementation details. Is it possible to have the
vector carry InsetLabel objects instead?
* the MouseHover.* files seems to be missing fro your third patch.


Speaking of review, I found that setMouseHover was never used, making
the variable useless. What do you think?


I'm afraid I don't understand what you mean here.


Guillaume, do you feel confident enough to push these patches for 2.3.0?
Or since there is no performance gain, do you think it's best to not
have them in 2.3.0 at this point? It's your call.



Hi Scott, indeed there is no need for this in 2.3 and I meant to reply
to Jean-Marc later. In addition gcc 4.6 might get in the way and I am
hoping that 2.4 is the good time to unsupport it. So it's best not to
have this in 2.3.



Re: No toolbar menu and no toolbar icon-size in menu

2017-05-24 Thread Guillaume MM

Le 24/05/2017 à 08:30, racoon a écrit :

On 24.05.2017 08:21, racoon wrote:

On 22.05.2017 18:55, racoon wrote:
With LyX 2.2.3 on Win7 I have no toolbar menu (just "No Action 
Defined!") and no toolbar icon-size in menu. Can someone else verify?


On alpha1-1 the toolbar menu works as expected but in the normal menu 
the icon size is missing.


Same in master. I think the patch in question is coming from here 
http://www.lyx.org/trac/ticket/10428.





It works fine here. Uwe, Andrew, can you reproduce on Windows?



Re: lyx-2.3.0alpha1-1 crash

2017-05-26 Thread Guillaume MM

Le 26/05/2017 à 23:15, PhilipPirrip a écrit :

On 05/26/2017 05:04 PM, PhilipPirrip wrote:


I think I caught this one too. I was working on a document with a few 
float figures exported from inkscape as pdf, LyX 2.3.0alpha1 crashed 
after updating (saving as pdf) one of the figures.
I was running J. Matos' version from 
https://copr.fedorainfracloud.org/coprs/jamatos/lyx-next/

Will try to reproduce.





This is how I crashed LyX now:
I had my 3000 word document open, four figure floats in it: Fig1.pdf to 
Fig4.pdf.

"/bin/cp Fig4.pdf Fig3.pdf" was enough to crash it.





Thanks, I'll have a look.


Guillaume



Re: lyx-2.3.0alpha1-1 crash

2017-05-26 Thread Guillaume MM

Le 26/05/2017 à 23:29, Guillaume MM a écrit :

Le 26/05/2017 à 23:15, PhilipPirrip a écrit :

On 05/26/2017 05:04 PM, PhilipPirrip wrote:


I think I caught this one too. I was working on a document with a few 
float figures exported from inkscape as pdf, LyX 2.3.0alpha1 crashed 
after updating (saving as pdf) one of the figures.
I was running J. Matos' version from 
https://copr.fedorainfracloud.org/coprs/jamatos/lyx-next/

Will try to reproduce.





This is how I crashed LyX now:
I had my 3000 word document open, four figure floats in it: Fig1.pdf 
to Fig4.pdf.

"/bin/cp Fig4.pdf Fig3.pdf" was enough to crash it.





Thanks, I'll have a look.




Here is a trace.


Thread 1 "lyx" received signal SIGSEGV, Segmentation fault.
0x768612ef in QFileInfo::absoluteFilePath() const ()
   from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5

#0  0x768612ef in QFileInfo::absoluteFilePath() const ()
   from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#1  0x00d3a681 in lyx::support::FileName::removeFile (
this=this@entry=0x2fd0d68) at ../../../src/support/FileName.cpp:612
#2  0x00cfbcbe in lyx::graphics::Converter::Impl::converted (
this=0x2fd0d10, retval=0) at 
../../src/graphics/GraphicsConverter.cpp:205
#3  0x00cfc28f in std::_Mem_fn_base(lyx::graphics::Converter::Impl::*)(int, int), true>::operator()<int, 
int, void>(lyx::graphics::Converter::Impl*, int&&, int&&) const 
(__object=, this=)

at /usr/include/c++/5/functional:600
#4  std::_Bind<std::_Mem_fn(lyx::graphics::Converter::Impl::*)(int, int)> 
(lyx::graphics::Converter::Impl*, std::_Placeholder<1>, 
std::_Placeholder<2>)>::__call<void, int&&, int&&, 0ul, 1ul, 
2ul>(std::tuple<int&&, int&&>&&, std::_Index_tuple<0ul, 1ul, 2ul>) 
(__args=, this=)

at /usr/include/c++/5/functional:1074
#5  std::_Bind<std::_Mem_fn(lyx::graphics::Converter::Impl::*)(int, int)> 
(lyx::graphics::Converter::Impl*, std::_Placeholder<1>, 
std::_Placeholder<2>)>::operator()<int, int, void>(int&&, int&&) 
(this=)

at /usr/include/c++/5/functional:1133
#6 
boost::detail::function::void_function_obj_invoker2<std::_Bind<std::_Mem_fn(lyx::graphics::Converter::Impl::*)(int, int)> 
(lyx::graphics::Converter::Impl*, std::_Placeholder<1>, 
std::_Placeholder<2>)>, void, int, 
int>::invoke(boost::detail::function::function_buffer&, int, int) 
(function_obj_ptr=...,

a0=, a1=)
at ../../3rdparty/boost/boost/function/function_template.hpp:159
#7  0x00d535ed in boost::function2<void, int, int>::operator() (
a1=, a0=, this=)
at ../../../3rdparty/boost/boost/function/function_template.hpp:771
#8 
boost::signals2::detail::call_with_tuple_args::m_invoke<boost::function(int, int)>, 0u, 1u, int&, int&>(boost::function&, 
boost::signals2::detail::unsigned_meta_array<0u, 1u>, std::tuple<int&, 
int&> const&, boost::enable_if<boost::is_void<boost::functionint)>::result_type>, void>::type*) const (

args=std::tuple containing = {...}, func=..., this=)
at 
../../../3rdparty/boost/boost/signals2/detail/variadic_slot_invoker.hpp:105

[...]
#18 lyx::support::ForkedProcess::emitSignal (this=this@entry=0x2db1bf0)
at ../../../src/support/ForkedCalls.cpp:116
#19 0x00d4f28e in 
lyx::support::ForkedCallsController::handleCompletedProcesses () at 
../../../src/support/ForkedCalls.cpp:662
#20 0x00a72ba9 in 
lyx::frontend::GuiApplication::handleRegularEvents (

this=)
at ../../../../src/frontends/qt4/GuiApplication.cpp:2680





Re: lyx-2.3.0alpha1-1 crash

2017-05-27 Thread Guillaume MM

Le 27/05/2017 à 00:56, Guillaume MM a écrit :



Thread 1 "lyx" received signal SIGSEGV, Segmentation fault.
0x768612ef in QFileInfo::absoluteFilePath() const ()
from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5

#0  0x768612ef in QFileInfo::absoluteFilePath() const ()
from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#1  0x00d3a681 in lyx::support::FileName::removeFile (
 this=this@entry=0x2fd0d68) at ../../../src/support/FileName.cpp:612
#2  0x00cfbcbe in lyx::graphics::Converter::Impl::converted (
 this=0x2fd0d10, retval=0) at 
../../src/graphics/GraphicsConverter.cpp:205
#3  0x00cfc28f in std::_Mem_fn_base(lyx::graphics::Converter::Impl::*)(int, int), true>::operator()<int, 
int, void>(lyx::graphics::Converter::Impl*, int&&, int&&) const 
(__object=, this=)

 at /usr/include/c++/5/functional:600



This is boost::signals2 calling a member function of a destroyed object.

Starting at 61b2bd5e, boost::bind was progressively replaced with
std::bind. They are not interchangeable though. boost::bind implements
the tracking of boost::signals{,2}::trackable objects. Now that
std::bind has completely replaced boost::bind, tracking never occurs.
With the attached patch it does not crash. (Only meant as a demo.)

The issue is sensitive to timing which is why the new FileMonitor
exacerbated the issue and why people who have tried to debug might have
noticed that it is harder to trigger with valgrind. But it is possible
that the issue has already occurred with other forms.

Fixing is easy but I prefer to review the complete code tree for the
problem.

Guillaume
diff --git a/src/support/bind.h b/src/support/bind.h
index 5a734ff..60f1304 100644
--- a/src/support/bind.h
+++ b/src/support/bind.h
@@ -13,13 +13,14 @@
 #define LYX_BIND_H
 
 #include "support/functional.h"
+#include "boost/bind.hpp"
 
 namespace lyx
 {
-	using std::placeholders::_1;
-	using std::placeholders::_2;
-	using std::bind;
-	using std::ref;
+	using boost::placeholders::_1;
+	using boost::placeholders::_2;
+	using boost::bind;
+	using boost::ref;
 }
 
 


Re: lyx-2.3.0alpha1-1 crash

2017-05-28 Thread Guillaume MM

Le 26/05/2017 à 23:04, PhilipPirrip a écrit :


I think I caught this one too. I was working on a document with a few 
float figures exported from inkscape as pdf, LyX 2.3.0alpha1 crashed 
after updating (saving as pdf) one of the figures.
I was running J. Matos' version from 
https://copr.fedorainfracloud.org/coprs/jamatos/lyx-next/

Will try to reproduce.




Here is a patch, reviews are welcome.

Guillaume

>From a3b7ae3c752327768952115b5990de455bd6cc48 Mon Sep 17 00:00:00 2001
From: Guillaume MM <g...@lyx.org>
Date: Sun, 28 May 2017 13:25:53 +0200
Subject: [PATCH] Properly track the lifetime of signals2::slots

Starting at 61b2bd5e, boost::bind was progressively replaced with
std::bind. They are not interchangeable though. boost::bind implements
the tracking of boost::signals{,2}::trackable objects. Now that
std::bind has completely replaced boost::bind, tracking never occurred.

This commit replaces boost::signals2::trackable with the new preferred
boost::signals2 methods: scoped_connections or slot::track_foreign. The
support::Trackable class introduced is less safe but easier for transitioning
old code.

Fixes the crash at
https://www.mail-archive.com/lyx-users@lists.lyx.org/msg105230.html and possibly
other crashes.
---
 src/Converter.cpp  | 22 +
 src/LaTeX.h|  5 ++--
 src/Server.cpp | 11 +
 src/Server.h   |  9 ---
 src/frontends/qt4/GuiView.cpp  |  3 ++-
 src/frontends/qt4/GuiWorkArea.cpp  |  7 +++---
 src/graphics/GraphicsCacheItem.cpp | 25 +---
 src/graphics/GraphicsCacheItem.h   |  6 ++---
 src/graphics/GraphicsConverter.cpp | 26 -
 src/graphics/GraphicsConverter.h   |  8 ---
 src/graphics/GraphicsLoader.cpp| 22 +
 src/graphics/GraphicsLoader.h  | 10 
 src/graphics/PreviewImage.cpp  | 12 --
 src/graphics/PreviewLoader.cpp | 21 ++---
 src/graphics/PreviewLoader.h   |  9 +++
 src/insets/InsetExternal.cpp   |  3 +--
 src/insets/InsetExternal.h |  4 +---
 src/insets/RenderPreview.cpp   | 23 ++
 src/insets/RenderPreview.h | 17 +++---
 src/support/FileMonitor.cpp|  3 +--
 src/support/FileMonitor.h  |  8 +++
 src/support/ForkedCalls.cpp| 19 ---
 src/support/ForkedCalls.h  | 20 +---
 src/support/Makefile.am|  1 +
 src/support/Timeout.h  |  4 ++--
 src/support/signals.h  | 48 ++
 26 files changed, 194 insertions(+), 152 deletions(-)
 create mode 100644 src/support/signals.h

diff --git a/src/Converter.cpp b/src/Converter.cpp
index 104ad0a..6e10b18 100644
--- a/src/Converter.cpp
+++ b/src/Converter.cpp
@@ -693,20 +693,6 @@ bool Converters::scanLog(Buffer const & buffer, string const & /*command*/,
 }
 
 
-namespace {
-
-class ShowMessage
-	: public boost::signals2::trackable {
-public:
-	ShowMessage(Buffer const & b) : buffer_(b) {}
-	void operator()(docstring const & msg) const { buffer_.message(msg); }
-private:
-	Buffer const & buffer_;
-};
-
-}
-
-
 bool Converters::runLaTeX(Buffer const & buffer, string const & command,
 			  OutputParams const & runparams, ErrorList & errorList)
 {
@@ -719,8 +705,12 @@ bool Converters::runLaTeX(Buffer const & buffer, string const & command,
 	buffer.filePath(), buffer.layoutPos(),
 	buffer.lastPreviewError());
 	TeXErrors terr;
-	ShowMessage show(buffer);
-	latex.message.connect(show);
+	// The connection closes itself at the end of the scope when latex is
+	// destroyed. One cannot close (and destroy) buffer while the converter is
+	// running.
+	latex.message.connect([](docstring const & msg){
+			buffer.message(msg);
+		});
 	int const result = latex.run(terr);
 
 	if (result & LaTeX::ERRORS)
diff --git a/src/LaTeX.h b/src/LaTeX.h
index f5d66a5..44920c3 100644
--- a/src/LaTeX.h
+++ b/src/LaTeX.h
@@ -18,8 +18,7 @@
 
 #include "support/docstring.h"
 #include "support/FileName.h"
-
-#include 
+#include "support/signals.h"
 
 #include 
 #include 
@@ -148,7 +147,7 @@ public:
 	};
 
 	/// This signal emits an informative message
-	boost::signals2::signal<void(docstring)> message;
+	signals2::signal<void(docstring)> message;
 
 
 	/**
diff --git a/src/Server.cpp b/src/Server.cpp
index b89e834..98e1d66 100644
--- a/src/Server.cpp
+++ b/src/Server.cpp
@@ -55,8 +55,7 @@
 #include "support/lassert.h"
 #include "support/lstrings.h"
 #include "support/os.h"
-
-#include "support/bind.h"
+#include "support/signals.h"
 
 #include 
 
@@ -859,8 +858,12 @@ int LyXComm::startPipe(string const & file, bool write)
 	}
 
 	if (!write) {
-		theApp()->registerSocketCallback(fd,
-		

Re: [LyX/master] Fix bug #10263

2017-05-25 Thread Guillaume MM

Le 25/05/2017 à 17:34, Enrico Forestieri a écrit :


No, this is not related to language nesting but is due to the fact
that \textbf is not a robust command and the footnote contains
multiple paragraphs. Three possible solutions come to mind:
1) Use \cprotect if the footnote contains a \par token
2) Use \robustify, in the same case
3) Revert this commit

See attached examples for the first two options.



Another solution here by calling separately \footnotemark and \footnotetext:
http://www.lyx.org/trac/ticket/10263#comment:19

For people wondering why doing this when there is no visible effect, one
should switch to KOMA-Script to see the effect on the footnote mark.



Re: [LyX/master] Fix bugs #10650 and #9598

2017-06-03 Thread Guillaume MM

Le 03/06/2017 à 17:26, Enrico Forestieri a écrit :

commit 55bbd67cde18184082b074f669a6b81cc48257b6
Author: Enrico Forestieri 
Date:   Sat Jun 3 17:26:05 2017 +0200

 Fix bugs #10650 and #9598


Hi Enrico,

It seems that the above commit changes the output of the attached file.

Before:

\documentclass{scrartcl}
\begin{document}
\begin{abstract}
Lorem
\begin{itemize}
\item Ipsum{\small \par}
\item Dolor{\small \par}
\end{itemize}
\end{abstract}
\end{document}

After:

\documentclass{scrartcl}
\begin{document}
\begin{abstract}
Lorem
\begin{itemize}
\item Ipsum
\item Dolor
\end{itemize}
\end{abstract}
\end{document}

Note that the font size of the abstract is small.

I am wondering since this results in a different vertical spacing. Is
this change intended? Which one is the desired output (I am not certain
about the first one)?



small-par.lyx
Description: application/lyx


Re: Can shell-escape take advantage of needauth framework?

2017-06-07 Thread Guillaume MM

Le 29/05/2017 à 23:53, Scott Kostyshak a écrit :


It would be nice to make the process of temporarily using -shell-escape
more user-friendly.[...]

One solution is to add a set of converters, one for each LaTeX flavor,
and then to specify the "needauth" flag for those converters.[...]

Any thoughts?


It has been well explained why the needauth option is bad in terms of
security, notably in messages by Helge.

https://www.mail-archive.com/lyx-devel@lists.lyx.org/msg198111.html
https://www.mail-archive.com/lyx-devel@lists.lyx.org/msg198123.html

I did not raise issues with its inclusion because it was still an
improvement compared to the situation before. But if needauth becomes an
argument to include "--shell-escape" converters, it will actually have
managed to make LyX less safe, I believe.



Re: [LyX/2.2.x] Fix bugs #9598 and #10650

2017-06-07 Thread Guillaume MM

Le 05/06/2017 à 23:15, Enrico Forestieri a écrit :

commit 59c22bd7b604a3ba9e0e78f7c51cb601f08d0192
Author: Enrico Forestieri
Date:   Mon Jun 5 23:14:48 2017 +0200

 Fix bugs #9598 and #10650
---



+// gcc < 4.8.0 and msvc < 2015 do not support C++11 thread_local
+#if defined(__GNUC__) && ((__GNUC__ == 4 && __GNUC_MINOR__ < 8) || __cplusplus 
< 201103L)
+#define THREAD_LOCAL_STATIC static __thread
+#elif defined(_MSC_VER) && ((_MSC_VER < 1900) || __cplusplus < 201103L)
+#define THREAD_LOCAL_STATIC static __declspec(thread)
+#else
+#define THREAD_LOCAL_STATIC thread_local static
+#endif
+



According to Stephan in this discussion:

https://www.mail-archive.com/lyx-devel@lists.lyx.org/msg196176.html

it is unfortunately not possible to use thread_local on Mac before some
time, it seems. I would actually be happy to hear the contrary.

(Also, it had been decided that LyX requires msvc ≥ 2015 so the second
branch would not be necessary.)



Re: [LyX/master] Fix bug #9101

2017-06-07 Thread Guillaume MM

Le 07/06/2017 à 01:08, Enrico Forestieri a écrit :

commit 8dab1cfe7ee6a3bb6d5e57afb55cb357e1e8ec23
Author: Enrico Forestieri 
Date:   Wed Jun 7 00:55:23 2017 +0200

 Fix bug #9101
 
 Update the listings inset to optionally use the minted package

 (instead of the listings one) for typesetting code listings.
 Only one of the two packages can be used in a document, but it
 is possible to switch packages without issues if the used options
 are the same. If a switch is made and the options differ, one needs
 to manually adjust them if they were entered in the advanced options
 tab, or apply again the gui settings.
 Note that minted requires the -shell-escape option for the latex
 backend and the installation of additional software (python pygments).
---

...

BabelPreamble
-   
\addto\captions$$lang{\renewcommand{\lstlistlistingname}{_(Listings[[List of 
Listings]])}}
+   \ifx\minted\undefined
+   
\addto\captions$$lang{\renewcommand{\lstlistlistingname}{_(Listings[[List of 
Listings]])}}\else
+   
\addto\captions$$lang{\renewcommand{\listoflistingscaption}{_(Listings[[List of 
Listings]])}}\fi
EndBabelPreamble
-   # The command does not need to be defined in LangPreamble, since
-   # listings.sty does that already. However it needs to be redefined
-   # in order to be used for non-english single-language documents.
+   # Either commands do not need to be defined in LangPreamble, since
+   # listings.sty or minted.sty do that already. However they need to be
+   # redefined in order to be used for non-english single-language
+   # documents.
LangPreamble
-   \renewcommand{\lstlistlistingname}{_(Listings[[List of 
Listings]])}
+   \ifx\minted\undefined
+   \renewcommand{\lstlistlistingname}{_(Listings[[List of 
Listings]])}\else
+   \renewcommand{\listoflistingscaption}{_(Listings[[List of 
Listings]])}\fi
EndLangPreamble
FixedWidthPreambleEncoding true
HTMLTag h2
@@ -298,13 +303,18 @@ End
  
  InsetLayout Include:Listings

BabelPreamble
-   
\addto\captions$$lang{\renewcommand{\lstlistingname}{_(Listing)}}
+   \ifx\minted\undefined
+   
\addto\captions$$lang{\renewcommand{\lstlistingname}{_(Listing)}}\else
+   
\addto\captions$$lang{\renewcommand{\listingscaption}{_(Listing)}}\fi
EndBabelPreamble
-   # The command does not need to be defined in LangPreamble, since
-   # listings.sty does that already. However it needs to be redefined
-   # in order to be used for non-english single-language documents.
+   # Either commands do not need to be defined in LangPreamble, since
+   # listings.sty or minted.sty do that already. However they need to be
+   # redefined in order to be used for non-english single-language
+   # documents.
LangPreamble
-   \renewcommand{\lstlistingname}{_(Listing)}
+   \ifx\minted\undefined
+   \renewcommand{\lstlistingname}{_(Listing)}\else
+   \renewcommand{\listingscaption}{_(Listing)}\fi
EndLangPreamble
FixedWidthPreambleEncoding true
  End



Is it possible to make layoutName() depend on the setting and avoid
\ifx\minted\undefined in the output?



Re: [LyX/master] Fix bugs #10650 and #9598

2017-06-04 Thread Guillaume MM

Le 04/06/2017 à 02:27, Enrico Forestieri a écrit :

On Sat, Jun 03, 2017 at 11:27:25PM +0200, Guillaume MM wrote:


I am wondering since this results in a different vertical spacing.


No, this will not change the vertical spacing.

It actually does, as shown with diffpdf.



Re: lyx-2.3.0alpha1-1 crash

2017-06-11 Thread Guillaume MM

Le 10/06/2017 à 00:34, Scott Kostyshak a écrit :

On Thu, Jun 01, 2017 at 01:43:11PM -0400, PhilipPirrip wrote:

On 05/28/2017 08:20 AM, Guillaume MM wrote:

Here is a patch, reviews are welcome.


Can't say much about the patch, but want to confirm that LyX is not crashing
any more.

Also pinging others to review the code, I think this is a pretty nasty bug
that definitely needs more testing.


+1 can anyone review the patch?



It is now committed at db581113.




Re: [LyX/master] Add accelerator

2017-06-11 Thread Guillaume MM

Le 11/06/2017 à 16:23, Jürgen Spitzmüller a écrit :

And LyX should add the -shell-escape flag for
minted documents (but warn the user before issuing it).




Hi Jürgen, this is being discussed in this thread:

https://www.mail-archive.com/lyx-devel@lists.lyx.org/msg200515.html



Re: Fix for vertical table border for added column

2017-05-07 Thread Guillaume MM

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

On Tue, Oct 18, 2016 at 11:03:21PM +0200, racoon wrote:

On 18.10.2016 21:35, racoon wrote:

I think the attached fix leads to more intuitive results for added table
borders.

This solves one strange case:
1. create a new table (with the default borders, in particular the last
row has the borders |c|)


column not row I meant.


2. add a column after the last most right) column

Actual result:
- The last two rows have the borders |c||c


Same here. Need sleep. :)




Hi racoon,

Is this patch still pending? If so, I can take a look at it.



Same question here.

Guillaume



Re: [LyX/master] Add support to cross out characters

2017-05-07 Thread Guillaume MM

Le 05/04/2017 à 00:01, Uwe Stöhr a écrit :

commit e575e7eebd32d687f3e23be0eeca185adb3b341b
Author: Uwe Stöhr 
Date:   Wed Apr 5 00:01:19 2017 +0200

Add support to cross out characters

- adds support for the command \xout of the LateX package ulem
- fileformat change
...
index 4ad5a78..e5328a5 100644
--- a/src/FuncCode.h
+++ b/src/FuncCode.h
@@ -431,6 +431,7 @@ enum FuncCode
LFUN_SECTION_SELECT,// vfr, 20090503
LFUN_FONT_UNDERLINE,
LFUN_FONT_STRIKEOUT,
+   LFUN_FONT_CROSSOUT, // uwestoehr 20170402
LFUN_FONT_UNDERUNDERLINE,
// 335
LFUN_FONT_UNDERWAVE,


After this addition the comments "// 335" etc. are no longer in sync
with the enum value, I fear.



Re: [coverity again] missing move constructors

2017-05-07 Thread Guillaume MM

Le 21/04/2017 à 00:11, Guillaume MM a écrit :


I am thinking about something along the lines of the attached patches.
But to be clear, one should not expect any performance gain. Only some
review, clarification, and simplification of the code.

Speaking of review, I found that setMouseHover was never used, making
the variable useless. What do you think?



Any opinion on the patches?

Guillaume



Re: #10455: Preferences shows current zoom instead of preference's default zoom

2017-05-07 Thread Guillaume MM

Le 29/10/2016 à 22:31, racoon a écrit :

On 29.10.2016 22:17, racoon wrote:

On 29.10.2016 10:33, racoon wrote:

I am working on a patch for

http://www.lyx.org/trac/ticket/10455

Seemed to me like a good suggestion. Attached is a not yet working
patch.

Unfortunately, I have no idea in which header to define the new
currentZoom variable. Any recommendations?

- It will be saved via QSettings (highest hirachy). The alternative to
save it to the preferences does not seem right since preferences are
normally only saved when the settings are manually saved. However, the
currentZoom should be saved whenever LyX is quit.

- Basically this variable is needed everywhere where now there is
lyxrc.zoom except for the preferences itself (and a new LFUN which
resets currentZoom to the default lyxrc.zoom). That is why still putting
it in LyXRC.h is tempting. This is what I did in the patch and what
seems to break it. Probably since the variable is somehow reset when
loading the preferences which happens after the currentZoom is loaded
via QSettings?


Here is an updated version that fixes the bug and makes the default zoom
accessible via binding M+0.

QSettings save and restore does not work properly yet. (Due to the
problem mentioned in my last post.)


Okay, I got it to work:

http://www.lyx.org/trac/ticket/10455#comment:1

Daniel





Hi Daniel,

I have pushed several of your patches including the above one, and
commented on others. Thank you for these contributions. I would love to
see more patches from you in the future, small or big. Also, sorry for
the small delay in reviewing them.

I have the impression that they were written very carefully, most of the
times there was nothing to correct. In the future, the most efficient
way to get your patches committed will be to send them when you find
that they are polished enough directly to the list fordiscussion (you
can also ask for advice beforehand as you have already done). I am sure
that developers will be quick to let you commit your patches by yourself.

I had to rewrite the "author" line every time because of encoding
issues. Can you please check your configuration on that point? Can you
also please document the new features at http://wiki.lyx.org/LyX/NewInLyX23?

Thank you
Guillaume



Re: Default to Qt 4 or Qt 5 with our build systems?

2017-05-01 Thread Guillaume MM

Le 01/05/2017 à 18:43, Jean-Marc Lasgouttes a écrit :

We can define a lower bound for acceptable qt5 version like we do for python 3 
vs 2. I would not want a crappy qt 5.1 on a system with a perfectly valid qt4.8.

Note that the end of life warning is not relevant on old systems (in Linux 
world).

So, what is the minimal good qt5 version ?


Qt 5.5.1 AFAICR



Re: #9841: use Qt's session management?

2017-05-01 Thread Guillaume MM

Le 21/04/2017 à 05:51, Scott Kostyshak a écrit :

Please see

  http://www.lyx.org/trac/ticket/9841

Guillaume proposed to use Qt's session management, which would simplify
things. Does anyone have an opinion?

I'm not sure if Guillaume would have the time and interest to implement
this for 2.3.0, but at least we could make a decision now.



To be clear this was about using Qt's session inside the main files as
an alternative to Session. Qt's session is already used for the GUI.

I retract that suggestion because I found that the session is lost too
often (e.g. at every reinstall). We want LyX's session to be robust
and copyable.

I had planned to do more work towards 2.3 yesterday but with the server
issues this will have to wait one or two more weeks
(hoping the bugtracker is usable then).

Guillaume



Re: copy inside math and paste in text mode gives LaTeX

2017-05-01 Thread Guillaume MM

Le 27/02/2017 à 18:41, Scott Kostyshak a écrit :

I'm often asked by LyX users why LyX behaves in the following way:

1. Start math and type X to the power \alpha.
2. Inside the math inset, copy the contents.
3. Outside the math inset, paste.

The text "X^{\alpha}" is pasted. The user seems to expect a math inset to be
created and for it to contain X to the power alpha.

The workaround is easy: just create a math inset before pasting (or in
this case, select the entire math inset instead of its contents).

However, enough users have asked me about this behavior and expressed
their confusion, and I realized I don't know why we do it this way,
since it seems uncommon that the user would actually want the LaTeX
"X^{\alpha}".

I understand if pasting outside of LyX, we need some plain-text
representation, but when inside LyX, would it make sense to always
create an inline math inset (e.g. even if the copy was made inside a
display equation) and paste the contents in it?

Is there a missing feature here?

Scott





Yes, this annoys me too and I think this deserves a bug report.

The selection is placed as application/x-lyx in the clipboard but the
math selection is converted into a string before that. The problem is
the use of grabSelection in copySelectionToStack in CutAndPaste.cpp
which converts into a docstring. This part could be improved to keep the
inset structure. As far as I can see this will not be
immediate for multi-cell selections. For single-cell selections it
should be simpler.

For the alternative workaround you suggest, one could keep the string
format but let ourselves know that it is tex math by giving it some mime
type application/x-tex-math (which does not exist). Only then would we
know that one has to create a math hull when pasting.

The former is the proper way, the latter the simplest.

Guillaume



Re: Release of LyX version 2.3.0alpha1-1

2017-05-05 Thread Guillaume MM

Le 05/05/2017 à 17:46, Jean-Marc Lasgouttes a écrit :


Hi Scott,

One thing to note for next release: you should make it clear that this
is just a preview of LyX 2.3.0 and that nobody should use it for serious
work (although I trust it is not too bad).



Another reason is that the file format can still change.




Re: [patch] support for document class option leqno

2017-05-05 Thread Guillaume MM

Le 05/05/2017 à 19:52, Jean-Marc Lasgouttes a écrit :

Le 05/05/2017 à 18:38, Jean-Marc Lasgouttes a écrit :

So please fell free to ad support reqno if others agree.


No, I am not going to "feel free" to implement it. I already "fell free"


"felt" ;)


to rewrite partly the fleqn patch and to implement the GUI drawing. I
also "fell free" to propose to help with the drawing of equation numbers
when it is ready. Here I am only doing code/feature review, these are
not issues on my TODO list.


This came out harsher than I meant, sorry. To get a feeling of what it
is like to propose a small feature and end up being whipped by Guillaume
until everything is more or less correct, please have a look at ticket
#8883 (with its 222 comments):
http://www.lyx.org/trac/ticket/8883

I can tell you that I am very grateful to Guillaume for his persistence.


Thanks to you for listening. It is not finished...

Guillaume



Support for acmart.cls

2017-05-06 Thread Guillaume MM

Dear list,

Here are a few patches recommended for inclusion related to the support
of the new ACM class provided by John Perry
(http://www.lyx.org/trac/ticket/10632).

1. InPreamble styles can be freely fixed with InTitle styles.
2. The layout by John Perry and a few changes by me, fully tested.
3. An addition to algorithm2e.module to typeset the algorithm supplied
with the example article.
4. The example article converted to LyX.
5. Make old ACM layouts obsolete.
6. Move obsolete layouts to a separate category.

I am ready to push except for 4. where I wait for some permission from
the maintainer because of the license.

Guillaume
>From a0e92e58f8625c282cc4646cadbe53d6717ad663 Mon Sep 17 00:00:00 2001
From: Guillaume MM <g...@lyx.org>
Date: Fri, 5 May 2017 23:49:52 +0200
Subject: [PATCH 1/6] InTitle: ignore InPreamble styles for outputting
 \maketitle.

---
 src/output_latex.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/output_latex.cpp b/src/output_latex.cpp
index a479b28..9cfeefb 100644
--- a/src/output_latex.cpp
+++ b/src/output_latex.cpp
@@ -1358,7 +1358,7 @@ void latexParagraphs(Buffer const & buf,
 			<< "}\n";
 }
 			}
-		} else if (was_title && !already_title) {
+		} else if (was_title && !already_title && !layout.inpreamble) {
 			if (tclass.titletype() == TITLE_ENVIRONMENT) {
 os << "\\end{" << from_ascii(tclass.titlename())
 		<< "}\n";
-- 
2.7.4

>From 647886022047b3da43ee1af8a2fcab7e6294556a Mon Sep 17 00:00:00 2001
From: Guillaume MM <g...@lyx.org>
Date: Mon, 1 May 2017 20:04:16 +0200
Subject: [PATCH 2/6] ACM article layout (#10632)

Contributed by John Perry
---
 lib/layouts/acmart.layout | 735 ++
 1 file changed, 735 insertions(+)
 create mode 100644 lib/layouts/acmart.layout

diff --git a/lib/layouts/acmart.layout b/lib/layouts/acmart.layout
new file mode 100644
index 000..5b5903b
--- /dev/null
+++ b/lib/layouts/acmart.layout
@@ -0,0 +1,735 @@
+#% Do not delete the line below; configure depends on this
+#  \DeclareLaTeXClass[acmart]{Association for Computing Machinery (ACM) article}
+#  \DeclareCategory{Articles}
+#
+# Layout for typesetting publications of the Association for Computing Machinery.
+#
+# Author : John Perry <john.pe...@usm.edu>
+#  Guillaume Munch-Maccagnoni <g...@lyx.org>
+
+Format 60
+
+Provides amscls 1
+Provides amsmath 1
+Provides amstext 1
+Provides binhex 1
+Provides caption 1
+Provides comment 1
+Provides cm-super 1
+Provides cmap 1
+Provides draftwatermark 1
+Provides environ 1
+Provides fancyhdr 1
+Provides float 1
+Provides fontaxes 1
+Provides geometry 1
+Provides graphics 1
+Provides hyperref 1
+Provides ifluatex 1
+Provides ifxetex 1
+Provides inconsolata 1
+Provides latex-tools 1
+Provides libertine 1
+Provides manyfoot 1
+Provides microtype 1
+Provides mmap 1
+Provides ms 1
+Provides mweights 1
+Provides natbib 1
+Provides nccfoots 1
+Provides newtx 1
+Provides oberdiek 1
+Provides pdftex-def 1
+Provides totpages 1
+Provides trimspaces 1
+Provides setspace 1
+Provides upquote 1
+Provides url 1
+Provides xcolor 1
+Provides xkeyval 1
+
+# Input general definitions
+Input stdclass.inc
+Input stdcounters.inc
+
+ClassOptions
+  FontSize		9|10|11|12
+  Other   "format=manuscript,authordraft"
+End
+
+Style Standard
+	Category  MainText
+	MarginStatic
+	LatexType Paragraph
+	LatexName dummy
+	ParIndent MM
+	ParSkip   0.4
+	Align Block
+	AlignPossible Block, Left, Right, Center
+	LabelType No_Label
+	# FIXME This ought to be set dynamically.
+	HTMLStyle
+		div.standard {
+			margin-bottom: 2ex;
+		}
+	EndHTMLStyle
+End
+
+Style Author
+  Align   Left
+  InTitle 1
+End
+
+Style Thanks
+  Category  FrontMatter
+  InTitle 1
+  Align Left
+  AlignPossible Left
+  Font
+Family Roman
+  EndFont
+  LabelFont
+Color Blue
+Shape Italic
+Family Roman
+  EndFont
+  LabelType Static
+  LabelString "Thanks: "
+  LatexType command
+  LatexName thanks
+  Margin Dynamic
+End
+
+Style ACM_Journal
+  Align left
+  AlignPossible left
+  Category Preamble
+  InPreamble  1
+  LabelString "Journal's Short Name: "
+  LabelType Static
+  LabelFont
+Color Blue
+Shape Italic
+  EndFont
+  LatexName acmJournal
+  LatexType Command
+  Margin Dynamic
+End
+
+Style ACM_Conference
+  CopyStyle ACM_Journal
+  Argument 1
+LabelString "Short name"
+Mandatory 0
+  EndArgument
+  Argument 2
+LabelString "Full name"
+Mandatory 1
+  EndArgument
+  Argument 3
+LabelString "Date"
+Mandatory 1
+  EndArgument
+  Argument 4
+LabelString "Venue"
+Mandatory 1
+  EndArgument
+  LabelString "Conference Name: "
+  LatexName acmConference
+  Margin Dynami

Re: Default to Qt 4 or Qt 5 with our build systems?

2017-05-04 Thread Guillaume MM

Le 04/05/2017 à 19:43, Pavel Sanda a écrit :

Guillaume MM wrote:

Le 01/05/2017 ?? 18:43, Jean-Marc Lasgouttes a écrit :

We can define a lower bound for acceptable qt5 version like we do for
python 3 vs 2. I would not want a crappy qt 5.1 on a system with a
perfectly valid qt4.8.

Note that the end of life warning is not relevant on old systems (in Linux
world).

So, what is the minimal good qt5 version ?


Qt 5.5.1 AFAICR


-> RELEASE_NOTES ?



The INSTALL file is up to date. It recommends 5.6 and suggests 5.4 at 
least. 5.4 is good in theory (this was lowered after some bugs were 
fixed), 5.5.1 got more testing. There is no definitive answer here.




  1   2   >