Re: [LyX/master] Use ObsoletedBy in fixme and todonotes modules.

[Georg Baum]

Juergen Spitzmueller wrote:

> +InsetLayout Flex:MakeTableOfToDos
> + ObsoletedBy   List_of_TODOs
> +End

This one produces an error message

tex2lyx/../insets/InsetLayout.cpp (354): Cannot replace with unknown 
InsetLayout `List of TODOs'


since List_of_TODOs is a style, not an InsetLayout. Which one is correct, 
Style or InsetLayout?


Georg

Re: [LyX/master] Make trivstring class ready for use

[Georg Baum]

Kornel Benko wrote:

> Done. Clang detection is somehow hackish, checking cxx-path only.

Thanks a lot!

The hack is still better than autotools (which do not know about clang at 
all).


Georg

Re: [LyX/master] Make trivstring class ready for use

[Georg Baum]

Enrico Forestieri wrote:

> Does not compile on cygwin:

I forgot to include ostream, which gets pulled in by something else here.

Better now?


Georg

Re: [LyX/master] Make trivstring class ready for use

[Georg Baum]

Kornel Benko wrote:

> Am Sonntag, 7. Dezember 2014 um 19:25:02, schrieb Kornel Benko
>> 
>> Compilation error:
>> 
>> [ 26%] Building CXX object
>> [ src/support/CMakeFiles/support.dir/trivstring.cpp.o
>> cd /usr/BUILD/BuildLyxGitQt5/src/support && /usr/bin/c++  
>> -DBOOST_SIGNALS_NO_DEPRECATION_WARNING=1 -DQT_CORE_LIB -Wall
>> -Wunused-parameter --std=gnu++11 -fno-strict-aliasing  -Wall
>> -Wunused-parameter --std=gnu++11 -fno-strict-aliasing -O0 -g3 -D_DEBUG
>> -fPIC -I/usr/BUILD/BuildLyxGitQt5 -I/usr/src/lyx/lyx-git/src
>> -I/usr/include/enchant -I/usr/src/lyx/lyx-git/boost
>> -I/usr/src/lyx/lyx-git/src/support
>> -I/usr/BUILD/BuildLyxGitQt5/src/support
>> -I/usr/src/lyx/lyx-git/src/support/mythes -isystem /usr/include/qt5
>> -isystem /usr/include/qt5/QtCore -isystem
>> /usr/lib/x86_64-linux-gnu/qt5/mkspecs/linux-g++-64   
>> -DBOOST_USER_CONFIG="" -o
>> CMakeFiles/support.dir/trivstring.cpp.o -c
>> /usr/src/lyx/lyx-git/src/support/trivstring.cpp In file included from
>> /usr/src/lyx/lyx-git/src/support/trivstring.cpp:13:0:
>> /usr/src/lyx/lyx-git/src/support/trivstring.h:107:1: error: expected
>> declaration before ‘}’ token
>>  } // namespace lyx

Sorry, I forgot to test without the new define. I fixed it now.

> I can compile (cmake) with the patch applied.

It would be nice if you could fix this and also implement clang detection 
(for clang STD_STRING_USES_COW should not be defined).


Georg

Re: [LyX/master] tex2lyx: support for KOMA-script's \caption* commands

[Georg Baum]

Uwe Stöhr wrote:

> diff --git a/src/tex2lyx/text.cpp b/src/tex2lyx/text.cpp
> index 7c72b64..3f9446a 100644
> --- a/src/tex2lyx/text.cpp
> +++ b/src/tex2lyx/text.cpp
> @@ -3427,15 +3430,26 @@ void parse_text(Parser & p, ostream & os, unsigned
> flags, bool outer,
>  else if (is_known(t.cs(), known_phrases) ||
>  (t.cs() == "protect" &&
>  p.next_token().cat() == catEscape &&
> -   is_known(p.next_token().cs(), known_phrases))) {
> - // LyX sometimes puts a \protect in front, so we have 
> to ignore it
> - // FIXME: This needs to be changed when bug 4752 is 
> fixed.
> - where = is_known(
> - t.cs() == "protect" ? p.get_token().cs() : 
> t.cs(),
> - known_phrases);
> - context.check_layout(os);
> - os << known_coded_phrases[where - known_phrases];
> - skip_spaces_braces(p);
> +   is_known(p.next_token().cs(), known_phrases)) ||
> +  (t.cs() == "protect" &&
> +   (p.next_token().cs() == "caption" ||
> +p.next_token().cs() == "captionabove" ||
> +p.next_token().cs() == "captionbelow"))) {
> + if (p.next_token().cs() == "caption" ||
> + p.next_token().cs() == "captionabove" ||
> + p.next_token().cs() == "captionbelow")
> + // we must ignore if \protect is in front of 
> \caption*
> + ;
> + else {
> + // LyX sometimes puts a \protect in front, so 
> we have to ignore 
it
> + // FIXME: This needs to be changed when bug 
> 4752 is fixed.
> + where = is_known(
> + t.cs() == "protect" ? 
> p.get_token().cs() : t.cs(),
> + known_phrases);
> + context.check_layout(os);
> + os << known_coded_phrases[where - 
> known_phrases];
> + skip_spaces_braces(p);
> + }
>  }
>  
>  // handle refstyle first to catch \eqref which can also occur

This part breaks the test case test-refstyle-theorems.tex (now you get
\protect\protect instead of \protect in front of a caption, so the old
output is definitely correct). What is this code supposed to do?


Georg

Re: Thread problems with std::basic_string and GNU libstdc++

[Georg Baum]

Jean-Marc Lasgouttes wrote:

> I like it, but I was hoping we could have a drop-in replacement (so that
> for example clang would still use a real std:string). This would require
> to get rid of the str(). Isn't it possible to desclare a an operator()
> to translate transparently to std::string?

I believe it would be possible technically, but do we want that? This would 
mean that people developing with clang or MSVC would break the build from 
time to time, because they would have no possibility to notice if the use a 
std::string member which does not exist in trivstring.


Georg

Re: [LyX/master] tex2lyx: support for KOMA-script's \caption* commands

[Georg Baum]

Uwe Stöhr wrote:

> I put the comment back. But I did not have understood how the correct
> comment would read and what is to fix here in which way. I mean what is
> hardcoded here?

Thanks. The hardcoding is this if-condition:

else if (t.cs() == "caption" || t.cs() == "captionabove" ||
t.cs() == "captionbelow") {

and the LyX code which is output for the tested LaTeX commands.
The whole block inside the if-condition should not be needed anymore.

> There is only one argument so "hardcoding" 1 seems
> correct. Moreover there is another occurrence of "Argument 1" in
> text.cpp without a FIXME. With findInsetLayout I can find a layout but I
> must nevertheless handle the different LyX code for each caption type.
> How would a solution with findInsetLayout look like?

The idea is that the insetlayout (which is retrieved by findInsetLayout()) 
defines the LyX code to output. The advantage is that this automatically 
works for new caption types which might be added in the future.

I just tried to comment the whole caption code block. The tex2lyx output is 
a bit different than before (mainly whitespace and "\begin_inset Caption 
Standard" (old) vs. "\begin_inset Flex Caption:Standard" (new), but I did 
not see at a quick glance whether the new output is correct.


Georg

Re: Thread problems with std::basic_string and GNU libstdc++

[Georg Baum]

 	/// preamble settings after babel was called
-	std::string const & babel_postsettings() const { return babel_postsettings_; }
+	std::string const babel_postsettings() const { return babel_postsettings_.str(); }
 	/// preamble settings before babel is called
-	std::string const & babel_presettings() const { return babel_presettings_; }
+	std::string const babel_presettings() const { return babel_presettings_.str(); }
 	/// This language internally sets a font encoding
 	bool internalFontEncoding() const { return internal_enc_; }
 	/// This language needs to be passed to babel itself (not the class)
@@ -82,40 +83,40 @@ public:
 	///
 	bool readLanguage(Lexer & lex);
 	///
-	typedef std::map TranslationMap;
+	typedef std::map TranslationMap;
 	///
 	void readLayoutTranslations(TranslationMap const & trans, bool replace);
 	// for the use in std::map
 	friend bool operator<(Language const & p, Language const & q);
 private:
 	///
-	std::string lang_;
+	trivstring lang_;
 	///
-	std::string babel_;
+	trivstring babel_;
 	///
-	std::string polyglossia_name_;
+	trivstring polyglossia_name_;
 	///
-	std::string polyglossia_opts_;
+	trivstring polyglossia_opts_;
 	///
-	std::string quote_style_;
+	trivstring quote_style_;
 	///
-	std::string requires_;
+	trivstring requires_;
 	///
-	std::string display_;
+	trivstring display_;
 	///
 	bool rightToLeft_;
 	///
-	std::string encodingStr_;
+	trivstring encodingStr_;
 	///
 	Encoding const * encoding_;
 	///
-	std::string code_;
+	trivstring code_;
 	///
-	std::string variety_;
+	trivstring variety_;
 	///
-	std::string babel_postsettings_;
+	trivstring babel_postsettings_;
 	///
-	std::string babel_presettings_;
+	trivstring babel_presettings_;
 	///
 	bool internal_enc_;
 	///
@@ -135,7 +136,7 @@ class Languages
 {
 public:
 	///
-	typedef std::map LanguageList;
+	typedef std::map LanguageList;
 	///
 	typedef LanguageList::const_iterator const_iterator;
 	///
diff --git a/src/support/Makefile.am b/src/support/Makefile.am
index ac43818..3abddbc 100644
--- a/src/support/Makefile.am
+++ b/src/support/Makefile.am
@@ -101,6 +101,8 @@ liblyxsupport_a_SOURCES = \
 	Translator.h \
 	Timeout.cpp \
 	Timeout.h \
+	trivstring.cpp \
+	trivstring.h \
 	types.h \
 	userinfo.cpp \
 	userinfo.h \
diff --git a/src/support/strfwd.h b/src/support/strfwd.h
index 10411cd..43e3cc0 100644
--- a/src/support/strfwd.h
+++ b/src/support/strfwd.h
@@ -95,6 +95,10 @@ std::string const & empty_string();
 // defined in docstring.cpp
 bool operator==(docstring const &, char const *);
 
+template class trivial_string;
+typedef trivial_string trivstring;
+typedef trivial_string trivdocstring;
+
 } // namespace lyx
 
 #endif
diff --git a/src/support/tests/check_lstrings.cpp b/src/support/tests/check_lstrings.cpp
index 3509e45..78cea8a 100644
--- a/src/support/tests/check_lstrings.cpp
+++ b/src/support/tests/check_lstrings.cpp
@@ -1,6 +1,7 @@
 #include 
 
 #include "../lstrings.h"
+#include "../trivstring.h"
 
 #include 
 
@@ -25,8 +26,42 @@ void test_uppercase()
 	cout << uppercase('a') << endl;
 }
 
+void test_trivstring()
+{
+	string const input[] = {
+		"",
+		"a",
+		"42",
+		"max sso", // max. string with sso on 64 bit
+		"something which does not fit into sso"
+	};
+	size_t const n = sizeof(input) / sizeof(input[0]);
+	for (size_t i = 0; i < n; ++i) {
+		// construction from std::string
+		trivstring const a(input[i]);
+		// construction from trivstring
+		trivstring const b(a);
+		// assignment from trivstring
+		trivstring const c = a;
+		// assignment from std::string
+		trivstring const d = input[i];
+		// assignment from trivstring
+		string const e = a.str();
+		// assignment from trivstring via C string
+		string const f = a.c_str();
+		cout << a.length() << endl;
+		cout << a.str() << endl;
+		cout << b.str() << endl;
+		cout << c.str() << endl;
+		cout << d.str() << endl;
+		cout << e << endl;
+		cout << f << endl;
+	}
+}
+
 int main()
 {
 	test_lowercase();
 	test_uppercase();
+	test_trivstring();
 }
diff --git a/src/support/tests/regfiles/lstrings b/src/support/tests/regfiles/lstrings
index cc2481b..146f2e6 100644
--- a/src/support/tests/regfiles/lstrings
+++ b/src/support/tests/regfiles/lstrings
@@ -5,3 +5,38 @@ alle
 A
 ALLE
 A
+0
+
+
+
+
+
+
+1
+a
+a
+a
+a
+a
+a
+2
+42
+42
+42
+42
+42
+42
+7
+max sso
+max sso
+max sso
+max sso
+max sso
+max sso
+37
+something which does not fit into sso
+something which does not fit into sso
+something which does not fit into sso
+something which does not fit into sso
+something which does not fit into sso
+something which does not fit into sso
diff --git a/src/support/trivstring.cpp b/src/support/trivstring.cpp
index 22ea956..71085ee 100644
--- a/src/support/trivstring.cpp
+++ b/src/support/trivstring.cpp
@@ -0,0 +1,180 @@
+/**
+ * \file

Re: [LyX/master] tex2lyx: support for KOMA-script's \caption* commands

[Georg Baum]

Uwe Stöhr wrote:

> - To my knowledge tex2lyx supports now all caption constructs provided
> by LyX. - InsetArgument is correct here, I therefore deleted the
> FIXME.

Please put it back. InsetArgument is indeed correct, but the hardcoding 
should be removed and findInsetLayout() should be used instead.


Georg

Re: Thread problems with std::basic_string and GNU libstdc++

[Georg Baum]

Pavel Sanda wrote:

> Georg Baum wrote:
>> 1) I could not find any write access to string instances created as copy
>> of Language::babel_. They are all const, so my first example is actually
>> not used here (but might be used elsewhere, one area to test would be our
>> VCS and graphics converter code, both use multiple threads as well).
> 
> IIRC think VCS does not use threading.

All VCS operations that call external tools use it indirectly: Via 
SystemCall.

>> I am now 100% sure and if needed I can try to give better explanations.
> 
> Amazing that we don't crash all the time if the analysis in the previous
> mails is correct.

I'd see it the other way round: It is amazing that it crashes reliably on my 
machine, since the time window for it to happen is very, very small. 
However, it is not the first multi-threading bug I saw that heavily depends 
on the "right conditions" to happen.

Anyway, I id some experiments with different solutions:

forcecopy.diff shows how a solution with a small helper function looks like. 
This is quite simply, but it requires to manually use it in all places where 
strings could be copied in multiple threads. Not nice, but I think it is 
doable.

stringclass.diff shows an incomplete solution with an own string class. I 
used docstring (this would be needed for std::string as well), since a 
replacement of std::string would produce a huge diff. The advantage is that 
you do not need to think about using a deep copy or not, but the 
disadvantage is that you need to duplicate a lot of base class 
functionality, and it produces warnings about ambigous operator+ (but 
without these ambiguities, other parts of the code don't compile).

I did not try Jean-Marcs suggestion about de-parallelizing the export and 
only calling the external tools in parallel. I am pretty sure that it works, 
however if we go this way then we basically say that we don't want parallel 
code in LyX itself. Despite I like the idea very much I am not so sure 
whether we should go this way, since the processor trend goes towards more 
cores and more energy efficency (which often means lower clock frequency), 
so we might need to use parallelism even more in the future.

Currently I prefer the deep_copy() method: The own string class which I 
initially liked more has too many side effects, then we can as well use a 
small helper which is better to understand. What do others think? How should 
we proceed?



Georgdiff --git a/config/lyxinclude.m4 b/config/lyxinclude.m4
index 95cb1f0..01d57bc 100644
--- a/config/lyxinclude.m4
+++ b/config/lyxinclude.m4
@@ -297,6 +297,7 @@ if test x$GXX = xyes; then
 	  ;;
   esac
   fi
+  AC_DEFINE(STD_STRING_USES_COW, 1, [std::string uses copy-on-write])
 fi
 test "$lyx_pch_comp" = yes && lyx_flags="$lyx_flags pch"
 AM_CONDITIONAL(LYX_BUILD_PCH, test "$lyx_pch_comp" = yes)
diff --git a/src/output_latex.cpp b/src/output_latex.cpp
index 3595a08..b8a8aef 100644
--- a/src/output_latex.cpp
+++ b/src/output_latex.cpp
@@ -656,13 +656,13 @@ void TeXOnePar(Buffer const & buf,
 
 	bool const use_polyglossia = runparams.use_polyglossia;
 	string const par_lang = use_polyglossia ?
-		getPolyglossiaEnvName(par_language): par_language->babel();
+		getPolyglossiaEnvName(par_language): deep_copy(par_language->babel());
 	string const prev_lang = use_polyglossia ?
-		getPolyglossiaEnvName(prev_language) : prev_language->babel();
+		getPolyglossiaEnvName(prev_language) : deep_copy(prev_language->babel());
 	string const doc_lang = use_polyglossia ?
-		getPolyglossiaEnvName(doc_language) : doc_language->babel();
+		getPolyglossiaEnvName(doc_language) : deep_copy(doc_language->babel());
 	string const outer_lang = use_polyglossia ?
-		getPolyglossiaEnvName(outer_language) : outer_language->babel();
+		getPolyglossiaEnvName(outer_language) : deep_copy(outer_language->babel());
 	string lang_begin_command = use_polyglossia ?
 		"\\begin{$$lang}" : lyxrc.language_command_begin;
 	string lang_end_command = use_polyglossia ?
@@ -953,7 +953,7 @@ void TeXOnePar(Buffer const & buf,
 		: outer_language;
 string const current_lang = use_polyglossia
 	? getPolyglossiaEnvName(current_language)
-	: current_language->babel();
+	: deep_copy(current_language->babel());
 if (!current_lang.empty()) {
 	os << from_ascii(subst(
 		lang_begin_command,
@@ -1131,7 +1131,7 @@ void latexParagraphs(Buffer const & buf,
 	// language on at start
 	string const mainlang = runparams.use_polyglossia
 		? getPolyglossiaEnvName(bparams.language)
-		: bparams.language->babel();
+		: deep_copy(bparams.language->babel());
 	string const lang_begin_command = runparams.use_polyglossia ?
 		"\\begin{$$lang}" : lyxrc.language_command_begin;
 
diff --git a/src/support/lstrings.h b/src/support/lstrings.h
ind

Re: [LyX/master] tex2lyx: support for glue lengths in InsetSpace

[Georg Baum]

Uwe Stöhr wrote:

> commit 1427b6fe316e257ce40512872c44cd438a51c498
> Author: Uwe Stöhr 
> Date:   Tue Nov 25 00:50:39 2014 +0100
> 
> tex2lyx: support for glue lengths in InsetSpace

I like these small improvements that you do! I don't have time to look at 
them in detail, but it is very good that you extend the tests for each new 
feature. That way, we know a) that the new code does not break existing 
functionality and b) for which examples it actually works. This way, we 
easily spot if future modifications would break the new feature again.


Georg

Re: Thread problems with std::basic_string and GNU libstdc++

[Georg Baum]

Georg Baum wrote:

> Jean-Marc Lasgouttes wrote:
> 
>> OK, could you know explain the same in terms of Language::babel(), which
>> is a const function? Between which objects is the representation shared?
> 
> The fact that Language::babel() is const does not matter, since
> std::basic_string::_M_rep() lies: It is a const method but returns a non-
> const pointer to internal data. Therefore Language::babel_ can be (and is)
> modified each time a copy is made through the const accessor
> Language::babel(). During export a lot of these copies is being made, e.g.
> in output_latex.cpp.
> 
> Creating the copy is a subset of the example I showed: If you don't look
> at the subsequent assignment, but compare only the difference after
> executing the first and second line, then it looks like this:
> 
> std::string s1("foo");
> assert(s1._M_rep()->_M_refcount == 0);
> 
> Now do the assignment:
> 
> std::string s2(s1);
> assert(s1._M_rep()->_M_refcount == 1);
> 
> Whether this is a race condition depends on whether increasing the ref
> counter is atomic or not. The wikipedia page mentions that it can be
> implemented in a thread-safe way without using locks by compare-and-swap
> (see http://en.wikipedia.org/wiki/Compare-and-swap), but the actual
> implementation is so complicated that I could not verify whether this is
> used at a quick glance. I only know that no locks are used. So my whole
> theory might be wrong actually, we'll need further investigation.

OK, I found out more:

1) I could not find any write access to string instances created as copy of 
Language::babel_. They are all const, so my first example is actually not 
used here (but might be used elsewhere, one area to test would be our VCS  
and graphics converter code, both use multiple threads as well).

2) Increasing the ref counter is an atomic operation (see 
std::basic_string::_Rep::_M_refcopy(), but this is not enough. The operation 
which needs to be atomic is the test whether a ref count increase is neded 
plus the increasing, but this is not the case, since the method _M_grab() is 
implemented using several helpers:

_CharT* _M_grab(const _Alloc& __alloc1, const _Alloc& __alloc2)
{
return (!_M_is_leaked() && __alloc1 == __alloc2)
? _M_refcopy() : _M_clone(__alloc1);
}

Conclusion: With GNU libstdc++, creating a copy of a std::basic_string does 
actually modify internal data of the original string in a non-atomic way, 
and therefore the programmer is responsible to protect these operations in 
multithreaded environments by some locking mechanism.

If anybody still doubts that this is true please tell (but look at the 
helgrind log in trac first, around line 972 you have a demonstration of what 
happens when a copy of Language::babel_ is being made). I am now 100% sure 
and if needed I can try to give better explanations.

I am going to think more about possible solutions to this problem, but this 
will take a few days. For now thanks to everybody who took part in the 
discussion, without the right questions which were asked I would not have 
understood the problem completely.


Georg

Re: Thread problems with std::basic_string and GNU libstdc++

[Georg Baum]

Jean-Marc Lasgouttes wrote:

> Le 24/11/2014 21:53, Georg Baum a écrit :
>>
>> I guess the basic principle how copy-on-write works is clear? If not, see
>> e.g. http://en.wikipedia.org/wiki/Copy-on-write.
>>
>> The problem is not the read access. This is safe. The problem is write
>> access and can be illustrated by the following code:

Oops, this is not 100% correct. The problem _might_ actually also be read 
access, at least in one special case: Creating a copy of a const string.

>> std::string s1("foo");
>> std::string s2(a);

This was wrong, it should have been read
std::string s2(s1);

>> std::string s3 = s1;
> 
> OK, could you know explain the same in terms of Language::babel(), which
> is a const function? Between which objects is the representation shared?

The fact that Language::babel() is const does not matter, since 
std::basic_string::_M_rep() lies: It is a const method but returns a non-
const pointer to internal data. Therefore Language::babel_ can be (and is) 
modified each time a copy is made through the const accessor 
Language::babel(). During export a lot of these copies is being made, e.g. 
in output_latex.cpp.

Creating the copy is a subset of the example I showed: If you don't look at 
the subsequent assignment, but compare only the difference after executing 
the first and second line, then it looks like this:

std::string s1("foo");
assert(s1._M_rep()->_M_refcount == 0);

Now do the assignment:

std::string s2(s1);
assert(s1._M_rep()->_M_refcount == 1);

Whether this is a race condition depends on whether increasing the ref 
counter is atomic or not. The wikipedia page mentions that it can be 
implemented in a thread-safe way without using locks by compare-and-swap 
(see http://en.wikipedia.org/wiki/Compare-and-swap), but the actual 
implementation is so complicated that I could not verify whether this is 
used at a quick glance. I only know that no locks are used. So my whole 
theory might be wrong actually, we'll need further investigation.

> It might be that understanding that is useless because the problem is
> general as you say. But it might happen too that it helps us to
> understand better what happens.

No, I think it helps, since this is a little bit different from the previous 
example. Actually I don't know if a copy of Language::babel() is modified 
later.


Georg

Re: Thread problems with std::basic_string and GNU libstdc++

[Georg Baum]

Richard Heck wrote:

> Can you explain in language I can understand why simply /reading/ this
> variable
> in separate threads could cause this kind of problem? I'm very confused.

I guess the basic principle how copy-on-write works is clear? If not, see 
e.g. http://en.wikipedia.org/wiki/Copy-on-write.

The problem is not the read access. This is safe. The problem is write 
access and can be illustrated by the following code:

std::string s1("foo");
std::string s2(a);
std::string s3 = s1;

After these three lines all strings share the same pointer to the rep 
object:

assert(s2._M_rep() == s1._M_rep());
assert(s3._M_rep() == s1._M_rep());

are both true. To understand the following, we need to know that 
std::basic_string::_M_p is a member variable pointing to the actual 
character data, and std::basic_string::_M_rep() returns a pointer to an 
object of type std::basic_string::_Rep which holds the metadata. The only 
relevant member in our case is std::basic_string::_Rep::_M_refcount which 
contains 0 if this particular _Rep object is used by one std::basic_string 
object. The actual implementation is much more complicated (and this is also 
the reason I used the accessor _M_rep() and not a member variable for thsi 
illustration), but this is not needed to understand the basic problem. At 
this point we know that

assert(s1._M_rep()->_M_refcount == 2);


After changing one string (e.g. a):

s1 = "bar";

the following conditions are true:

assert(s2._M_rep() == s3._M_rep());
assert(s2._M_p == s3._M_p);
assert(s1._M_rep() != s2._M_rep());
assert(s1._M_p != s2._M_p);
assert(s1._M_rep()->_M_refcount == 0);
assert(s2._M_rep()->_M_refcount == 1);

Now imagine that s1 is used in thread a and s2 is used in thread b => there 
you have your race condition: Changing the object s1 in thread a which is 
not used in thread b does still modify internal data of object s2 which is 
used in thread b. If this happens while s2 is being deleted or altered in 
thread b you are in trouble and will likely see a crash. Of course the 
chance for this to happen is very very small, so I am actually not surprised 
that others could not reproduce the crash.


Georg

Re: Thread problems with std::basic_string and GNU libstdc++

[Georg Baum]

Abdelrazak Younes wrote:

> This string might be modified by some class in main thread while being
> read at the same time by a buffer clone in the export thread. If the
> memory reserved by basic-string is big enough we should be fine but if
> not basic_string will allocate another chunk of memory and delete
> previously used one. So, if this is really the culprit, 2 choices:
> 1) easy one: reserve some memory for babel_ in Langage ctor
> (basic_string::reserve() should exist).
> 2) protect babel_ with a mutex.

The babel_ member variable is not changed at all during the export. 
Therefore it cannot be a question of reserved memory.


Georg

Re: Thread problems with std::basic_string and GNU libstdc++

[Georg Baum]

Enrico Forestieri wrote:

> I tried hard but was not able to reproduce the crash on debian wheezy
> (gcc 4.7.2), cygwin (gcc 4.8.3) and solaris (gcc 4.9.0).

Thanks for tsting. One further thing that I unfortunately forgot to tell was 
that I had the LaTeX log window open. This adds more processing and might 
also increase the crash probability.

> BTW, the test case did not compile without errors and I had to add the
> following lines to the preable:

Yes, it does not compile, but this does not matter if the error occurs: Then 
it crashes before calling LaTeX.


Georg

Re: Thread problems with std::basic_string and GNU libstdc++

[Georg Baum]

Abdelrazak Younes wrote:

> On 20/11/2014 21:04, Georg Baum wrote:
>> I don't think so in this case. The crash happens because internal
>> std::basic_string members are corrupt, and it is almost always a string
>> created from Language::babel_. Furthermore, I was not able to reproduce
>> the crash after changing
>>
>> std::string const & babel() const { return babel_; }
>>
>> to
>>
>> std::string const babel() const { return babel_.c_str(); }
>>
>> in Language.h (which circumvents copy-on-write).
> 
> Hum, well, maybe Language object is shared between the buffer clones?
> That could well explain the issue... babel_ is accessed at the same time
> between 2 threads which creates a race condition.
> 
> So my solution would be to protect the Language object through a mutex.

This would be a performance problem.

> If I am right then this crash is due to a flow in our design, not in the
> STL string.

Lets put it that way: Our design requires that modification of a 
std::basic_string instance does not alter any other std::basic_string 
instance. This requirement is not met by a COW implementation of 
std::basic_string, and it is met by non-COW implementations.

>> I also did tests with valgrind. memcheck reported an unrelated error in
>> LaTeX log file parsing which I fixed, and helgrind pointed at
>> std::basic_string (see log in trac). I am not 100% sure if the helgrind
>> log does not contain false positives, but it does at least not contain
>> any hints to other causes of the problem.
> 
> For me the fact that this is all related to the Latex exports means that
> the latex export functions are not thread safe and we should work on that.

Yes, but the question is why they are not thread-safe. My claim is that they 
are thread-safe for non-COW-implementations of std::basic_string, since 100% 
of the errors I saw were in std::string instances which were only used by 
one thread.

>>> Why not compile with c++11 option? I guess basic_string won't do
>>> copy-on-write if compiled this way?
>> Because the GNU libstdc++ is not conformant in this aspect: It uses
>> copy-on- write even with C++11.
> 
> Hum, I thought libstc++ was mostly about iostream and basic_stream is
> coded all in template header?

GNU libstdc++ contains the complete STL implemenation used by gcc. Whether 
it is in headers or not does not matter for the question we are discussing. 
You can see at 
https://gcc.gnu.org/onlinedocs/libstdc++/manual/status.html#status.iso.2011 
item 21.4 what the developers themselves say about C++11 conformance of 
std::basic_string.

>> BTW, I would love to be proven wrong, because I find this issue rather
>> scary, but up to now I did not find any convincing argument, so please
>> try harder;-)
> 
> I tried ;-)

I am still not convinced ;-)

Georg

Re: Thread problems with std::basic_string and GNU libstdc++

[Georg Baum]

Richard Heck wrote:

> Yes, I understand that. But it's hard to see what relevant difference
> there might be
> between the document set that is causing this crash---which Georg says
> in the bug
> report he can produce "reliably"---and other large documents sets that
> other people
> have used frequently without producing any crash.

I think I found the relevant difference: The LaTeX preview panel needs to be 
open for current paragraph with automatic update on. Otherwise it does not 
crash. I uploaded a reduced, anonymized test case to trac, so I would be 
curios whether anybody can reproduce the crash. If yes, I'd also like to 
know the gcc version and linux distribution which was used.


Georg

Re: Thread problems with std::basic_string and GNU libstdc++

[Georg Baum]

Georg Baum wrote:

> I see. I installed pLyX, but I fear my way of thinking is very different
> from the one in the pLyX system. At least I did not manage to get it to do
> what I want.

With the help of Andrew I got pLyX to work. Unfortunately the crash is not 
reproducible with the anonymized version, but the good thing is that I now 
know which child of the multipart document causes the crash.

I am now going to reduce the test case by manual bisection, but this will 
take some time.


Georg

Re: Thread problems with std::basic_string and GNU libstdc++

[Georg Baum]

Jean-Marc Lasgouttes wrote:

> AFAICS, the threaded export is interesting mostly because of the cost of
> running external programs. Could we do the latex export in the main
> thread and then just do our converter stuff in a multithreaded way? How
> difficult is that?

Good idea.

> And if exporting to LaTeX is too slow, we just have to profile it and
> speed it up.

This should not be difficult.

> It is a bit strange to me that libstdc++ can have this bug without a
> solution in view. LyX is probably not the only GUI program that uses
> threads with g++, is it?

This is indeed astonishing. On the other hand clang became very popular as 
an alternative recently, and this may be one reason. I maybe would disregard 
the gcc bug report as unimportant, but it comes from James Kanze who really 
knows what he is doing.


Georg

Re: Thread problems with std::basic_string and GNU libstdc++

[Georg Baum]

Enrico Forestieri wrote:

> Cannot a deep copy be forced after cloning? What about recursing over
> all non-const strings and doing something like str = str.c_str() ?

I would think so, but it would be a bit cumbersome to find all places (and 
the same problem could arise unnoticed elswhere, if something else is done 
in other threads).


Georg

Re: Thread problems with std::basic_string and GNU libstdc++

[Georg Baum]

Richard Heck wrote:

> Yes, I understand that. But it's hard to see what relevant difference
> there might be
> between the document set that is causing this crash---which Georg says
> in the bug
> report he can produce "reliably"---and other large documents sets that
> other people
> have used frequently without producing any crash.

I guess we will find out that difference once you all can test with an 
anonymized document.

>> In any case I think it's no problem to sacrifice a bit of performance
>> to prevent possible crashes, even if the crashes are rare.
> 
> Of course, but it'd be good to be sure what's causing those crashes.

Definitely. We should only do something if we understood the problem, 
otherwise we don't know if we introduce other problems.


Georg

Re: Python 3 incompatibilities still exist right?

[Georg Baum]

Richard Heck wrote:

> It might be possible to specify the full path to the python version you
> want to use in
> the relevant converter definition. But I am not sure.

This is possible. The magic happens only for "python -tt". Even specifying 
only "python3" or "python3 -tt" as the interpreter for self defined 
converters should work, if python3 is in the path. If it does not, please 
report a bug, this can probably be fixed easily.


Georg

Re: Thread problems with std::basic_string and GNU libstdc++

[Georg Baum]

Richard Heck wrote:

> I'm on Fedora 20, which has gcc 4.8.3.

I have gcc 4.7.2.

> Scott mentioned something there.

I see. I installed pLyX, but I fear my way of thinking is very different 
from the one in the pLyX system. At least I did not manage to get it to do 
what I want.


Georg

Re: Thread problems with std::basic_string and GNU libstdc++

[Georg Baum]

Abdelrazak Younes wrote:

> Hi Georg,
> 
> Warning: I haven't read LyX source code for a long long time :-)

This is no problem, I think the main point can be understood quite well 
without code details.

> I am not questioning your reasoning but I think STL thread safety
> limitation has limited impact in our case. AFAIR, the Buffer is cloned
> in in the current and main thread so we have no race condition here.
> After that the cloned buffer shares nothing with the original Buffer and
> can thus be used safely in another thread.

Yes, this is true if you look at the involved data structures from the 
outside. If you look at the internal std::basic string members in gdb after 
the crash then you see that two instances which where both originally 
created from the document language Language::babel_ use the same pointer to 
the string data internally. This is of course no surprise if you know that 
copy-on-write is used, but it also means that the two buffers are _not_ 
completely independant from each other. BTW, I am pretty sure that the 
cloning itself works very reliably.

> I believe that there could be many reasons that could cause a crash
> before incriminating the stl string, etc:
> * Gui elements (message passing?)
> * Graphics preview elements

I don't think so in this case. The crash happens because internal 
std::basic_string members are corrupt, and it is almost always a string 
created from Language::babel_. Furthermore, I was not able to reproduce the 
crash after changing

std::string const & babel() const { return babel_; }

to

std::string const babel() const { return babel_.c_str(); }

in Language.h (which circumvents copy-on-write).

I also did tests with valgrind. memcheck reported an unrelated error in 
LaTeX log file parsing which I fixed, and helgrind pointed at 
std::basic_string (see log in trac). I am not 100% sure if the helgrind log 
does not contain false positives, but it does at least not contain any hints 
to other causes of the problem.

> Why not compile with c++11 option? I guess basic_string won't do
> copy-on-write if compiled this way?

Because the GNU libstdc++ is not conformant in this aspect: It uses copy-on-
write even with C++11.

> Well, IIRC I developed buffer cloning and initial threaded export using
> Linux :-)
> Maybe Peter Kuemmel continued the work under Windows but I don't think
> so...

Sorry, then my memory was wrong. I thought that Vincent developed this.

BTW, I would love to be proven wrong, because I find this issue rather 
scary, but up to now I did not find any convincing argument, so please try 
harder;-)


Georg

Re: Thread problems with std::basic_string and GNU libstdc++

[Georg Baum]

Richard Heck wrote:

> I'm not well informed enough about implementation details to have an
> opinion about whether this
> diagnosis is correct. But I am somewhat puzzled that we are only seeing
> this issue now. Of course,
> there may have been other random crashes that people haven't reported.

This is an important point, and I asked myself the same question as well.  I 
gues the most plausible explanation is what James wrote in the gcc bug 
report: "In this particular case, there is only a very, very small window
of time in which the error can occur." It might also be influenced 
considerably by compiler version and/or settings. Which linux distro do you 
use? Both the bug reporter and I am on debian.

> But I have used LyX with
> threaded export enabled, on Linux, for a very long time, with large
> documents with lots of children,
> and I've never seen anything like this crash. I thus remain curious
> about the details of the document
> itself. Is there something special about it that is triggering this?

It uses xelatex and biber, apart from that I don't know, but I did not 
really look at the contents. As I wrote in trac: Do we have a tool to 
anonymize documents currently? If not I'll write an lfun, because we 
definitely need a test case which can be tested by others, and I am not 
going to anonymize the document by hand.

> Obviously, we need some substantial discussion before making any
> decision about what to do. I'm
> going to cc a bunch of people to try to get their attention.

Definitely.


Georg

Thread problems with std::basic_string and GNU libstdc++

[Georg Baum]

Hi,

while investigating http://www.lyx.org/trac/ticket/9336 I found out a 
fundamental problem with our multithreaded export: The GNU libstdc++ gives 
only a relatively weak thread-safety guarantee about std::basic string. It 
is not completely thread-safe in the POSIX sense. The reason for this lies 
in the employed copy-on-write technique, which is not used in other STL 
implementations such as the one in clang or MSVC. It is even explicitly 
forbidden in the new C++11 standard, and the gcc folks already acknowledged 
that they will get rid of it, but since this is an ABI incompatible change 
they want to wait until they have several such changes to do all in one go. 
For details please see the excellent gcc bug report by James Kanze (who is a 
C++ expert) at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=21334, 
especially https://gcc.gnu.org/bugzilla/show_bug.cgi?id=21334#c21, which 
describes almost exactly the situation we have with the document language.

This looks all rather theoretical, but in practice it means that it is quite 
easy to produce crashes if you use LyX built with #define EXPORT_in_THREAD 1 
and gcc, and start an export, depending on the document (master-child setups 
seem to be much more vulnerable). The thing which seems to be most affected 
is Language::babel_, which is copied a lot, but problems could arise from 
all std::string and docstring variables.

What do we do now? I can think of several alternatives:

1) Do nothing, because my findings are incorrect. I don't believe that of 
course, but I would appreciate it if somebody would try to find flaws in my 
reasoning.

2) Disable EXPORT_in_THREAD if GNU libstdc++ is found. This is an easy and 
safe solution, with the disadvantage that it removes a feature (but I think 
that this is much better than risking crashes).

3) Derive an own class lyx::basic_string from std::basic_string which 
circumvents copy-on-wrtite in the copy-constructor and assignment operator 
by creating the copy from c_str(). Then use it

namespace lyx {
typedef basic_string docstring;
typedef basic_string string;
}

and replace all occurances of std::string with lyx::string. This would of 
course need a lot of testing, but I believe it could work.

4) Do not use std::basic_string at all, but an own implementation which 
could be stolen from STLport or clang (if the licenses permit that and are 
compatible to the GPL, I did not check that). However, I believe that this 
would create lots of interface problems with other libraries.


Does anybody see any other solution? I think for the stable branch we do not 
really have any choice and need to implement 2).


The other question is: What can we learn for the future? AFAIK 
EXPORT_in_THREAD was developed on windows, and it was probably well tested 
on that platform. However, one basic lesson to learn is that such important 
infrastructure changes need thorough testing on all supported platforms, and 
I don't think this did happen in this case. Despite qt hides many 
differences for us there are still a lot of platform specific issues left.


Cheers,
Georg

Re: Adding new builtin math macros

[Georg Baum]

Richard Heck wrote:

> You know what would be really cool? The ability to custom-define default
> macros in a module, including ones with arguments.

Yes. And I guess it would not be difficult to implement. If I find some time 
I might at least extend lib/symbols to support macros with arguments, since 
the machinery is already there. From that it would probably not be too 
difficult to read the macros from a module instead.


Georg

Re: #9333: LyX crashes after bibtex database update (via LyZ)

[Georg Baum]

Jean-Marc Lasgouttes wrote:

> Le 18/11/2014 12:38, MAILLARD Serge a écrit :
>> Good morning,
>> Actually, my organization tells I may not install the new LyX version on
>> my machine because there is no official Debian package for it. Is there
>> a possibility for you to create such a package?
> 
> Please answer in the bug report, it is easier for us.
> 
> It is indeed a problem that wheezy is stuck to LyX 2.0.3. While there is
> no hope that they allow LyX 2.1 (I guess), it would be nice to ask for
> 2.0.8 in wheezy.

The main problem is that the official debian package of LyX has currently no 
maintainer, and it was big luck that 2.1.2 made it into the next stable 
release very shortly before the freeze. I hope that they will find a new 
maintainer, and then it would also be much easier to provide something in 
backports.debian.org for older releases.


Georg

Re: [LyX/master] Add diagnostic messages for enchant spell checker dictionary requests

[Georg Baum]

Stephan Witt wrote:

> Yes, it didn't work. I had to remove the const keyword to get it to
> compile. Yesterday I couldn't figure it out.

Yes, this is the standard way to catch exceptions (unless you work with 
libraries that ignore the standards and define their own thing, like the 
Microsoft Foundation Classes which require to catch exceptions by pointer 
and to call a Delete() method). But I did not notice that eitehr when 
looking at your patch.

> But the message text is empty nevertheless.
> 
> I put the improved version in, thank you.

Very good! Now it is the responsibility of enchant to create a more sensible 
error message.


Georg

Re: [LyX/master] Fix memory error detected by valgrind

[Georg Baum]

Georg Baum wrote:

> commit e5845fea49e6c8a2cae0dfd8c708a31ae3d67719
> Author: Georg Baum 
> Date:   Mon Nov 17 22:08:21 2014 +0100
> 
> Fix memory error detected by valgrind
> 
> The assignment name = sub.str(1) reads from the first argument given
> to regex_match(), but previously this was a temporary object which was
> already out of scope. This did probably not matter much in practice,
> but invoked undefined behaviour, and as we all know this is allowed
> ton format your hard disk or kill to your cat, so better fix this.

Richard,

I'd like to put this in branch (after I searched for similar code and found 
a comment stating that I found exactly the same problem already some years 
ago I am not convinced anymore that this is harmless). OK?


Georg

Re: [LyX/master] Add diagnostic messages for enchant spell checker dictionary requests

[Georg Baum]

Stephan Witt wrote:

>  catch (const enchant::Exception & e) {
>  // FIXME error handling?
> + // unfortunately the message of enchant::Exception is 
unreachable

Why? Does e.what() not work?


Georg

Re: Adding new builtin math macros

[Georg Baum]

Jean-Marc Lasgouttes wrote:

> The answer really depend on the macro you want to define. In the case of
> \pmod, I did not find any inset (in src/mathed) that seems to be similar
> enough. You want an inset with only one cell, like in
> InsetMathDecoration, which implements accents. So the inset would be a
> /very/ simplified version of this.
> 
> However, it seems a bit strange to create a new inset class for this
> simple example. I am not sure what the simplest solution would be. A
> math macro inset gives a not too bad interface, so a solution might be
> to have a way to define a set of default macros that are known to LyX
> from start. Then the macro inset machinery would be sue for the display
> part.
> 
> What do the others think?

Something like a set of default macros is already implemented. It is read 
from lib/symbols (where \pmod is already defined, that is why you don't see 
it in the sources). Unfortunately the syntax of lib/symbols is completely 
undocumented, but if you want to define a new predefined macro named \foo, 
you can do it like this:

\def\foo{a^2+b^2} myfeature

The first token is the "display" part of the macro and must not contain 
spaces. The second token is a feature from LaTeXFeatures.cpp which can 
either be a package (e.g. "mathtools"), or use a hard coded definition in 
LaTeXFeatures.cpp (e.g. "binom").

Maybe this is enough for your use case. If not, please give a more detailed 
example what you want to do, then we might be able to help.


Georg

Re: [LyX/master] Prevent accidental usage of wrong copy constructor

[Georg Baum]

Abdelrazak Younes wrote:

> Nitpick: private stuff is at the end of the class not at the top.

Sure, but the private noncopyable stuff is the only thing I prefer at the 
top, since it is then near the other contructors.

> Abdel.
> 
> PS: I got some nice ideas today about teletext charset handling thanks
> to the work we did together on Unicode ;-)

:-) I also make use of what I learned back then on a regular basis.


Georg

Re: [LyX/master] Fix recursive math macro crash (bug #9140)

[Georg Baum]

Georg Baum wrote:

> commit 0f2069b8a5967aac1b4f841214294f1fe21d2cad
> Author: Georg Baum 
> Date:   Fri Nov 14 21:30:42 2014 +0100
> 
> Fix recursive math macro crash (bug #9140)
> 
> This is different from bug #8999, since in this case a new macro
> instance is created. You still get a TeX capacity exceeded error if
> you try to typeset the exported document, but this is the same as for
> bug #8999 and better than a crash.

Richard, OK for branch?


Georg

Re: [LyX/master] Prevent accidental usage of wrong copy constructor

[Georg Baum]

Jean-Marc Lasgouttes wrote:

> Le 11/11/2014 20:58, Georg Baum a écrit :
>> commit 8f93600d3fa8182ba43973075cf37e7ecb2be8d3
>> Author: Georg Baum 
>> Date:   Tue Nov 11 07:22:14 2014 +0100
>>
>>  Prevent accidental usage of wrong copy constructor
> 
> Is this what we use boost:noncopyable for in some places? It would be
> nice to keep the same form in all these places.

This used to be boost::noncopyable a long time ago, but IIRC this is now 
considered evil, because it pulls in a header just for very little benefit.

Please correct me if I am wrong, I'll change this if that is the case.



Georg

Re: Problem with copy

[Georg Baum]

Scott Kostyshak wrote:

> Do you think there is a possibility that your commit fixed
> http://www.lyx.org/trac/ticket/8695 ?

Not very likely, but it should be easy to retest this now.


Georg

Re: Problem with copy

[Georg Baum]

Richard Heck wrote:

> In fact, the dangling pointer is harmless, since the empty paragraph to
> which it is attached almost immediately disappears when we paste the new
> content. So we don't really need to reset the paragraphs: We never do
> anything with them.

Valgrind would not complain if we really would not do anything with them: 
The previously dangling layout pointers are used in pasteSelectionHelper() 
(pars[pit].layout() and pars[pit].getMaxDepthAfter()).

> That makes me wonder if the following would not also
> silence valgrind with less work:
> 
> DocumentClassConstPtr olddc = buffer->params().documentClassPtr();
> 
> Then the document class will hang around until olddc goes out of scope
> at the end of the routine.

That would prevent the dangling pointers as well, but I prefer the other 
solution, since in this case the used layouts would not be the ones from the 
current document class.

> Another possibility, which would get rid of setDocumentClass, would be
> to allow cloneBufferOnly() to take an optional DocumentClassConstPtr
> object, and to set the document class when we do the clone if it is
> given. Then we do not make a new DocumentClass only immediately to
> destory it.

Indeed. For the time being I did not want too many changes, one thing we 
also need to take care of eventually is the missing thread-safety of the 
static buffer cloning. One thing I always wanted to do is to profile the 
performance gain of the static buffer: Maybe we can get the same performance 
by some other optimizations? This would be my preferred solution, if we did 
not need to use a static buffer then we can easily create a new one from a 
DocumentClassConstPtr.


Georg

Re: Temp file locking problems on windows

[Georg Baum]

Enrico Forestieri wrote:

> On Tue, Nov 04, 2014 at 10:51:00PM +0100, Georg Baum wrote:
>> However, after further thinking on it, I believe that I still do not
>> understand 100% what is happening, and that the extended fix is not
>> correct: Why does the renaming fail? I do understand that other processes
>> cannot do anything to the file while it is open, because QTemporaryFile
>> opens it in exclusive mode. However, the process that created the temp
>> file must be able to qwrite to the file, otherwise the whole
>> QTemporaryFile class would be useless. What is the deifference between
>> writing to the file and renaming it?
> 
> On Windows you need different permissions for writing or moving a file:
> http://msdn.microsoft.com/en-us/library/bb727008.aspx

Thanks for this information, this explains the current behaviour quite well. 
Therefore the current fix seems to be OK, the only thing I'll do when I'll 
find some more time is to audit our sources for other cases where a file 
create by the TempFile class needs to be moved or deleted, since these would 
not work either.


Georg

Re: Problem with copy

[Georg Baum]

Richard Heck wrote:

> On 10/29/2014 05:43 PM, Georg Baum wrote:
>>
>> Maybe the cloning does not work correctly, so deleting the clone does
>> actually delwte parts of the static buffer?
> 
> Hmm. That may be. I'm not sure if we copy absolutely everything over.
> And the Buffer object
> is so, so big

That was a wrong guess. The cloning works, but setDocumentClass() causes the 
old document class to be deleted, and this invalidates the layout pointers 
in the paragraphs of the buffer (actually there is only one paragraph).

You usually don't notice the problem, because it is quite unlikely that the 
just freed memory of the deleted document class is immediately reused for 
something else, and therefore there is still a valid layout data structure 
at the old location in memory. However, especially if you paste huge amounts 
of data, this might be different.

I fixed the problem in 5deaaa3b137c, and I propose to also backport this to 
branch. OK?


Georg

Re: Problem with copy

[Georg Baum]

Richard Heck wrote:

> On 10/29/2014 05:43 PM, Georg Baum wrote:
>>
>> Maybe the cloning does not work correctly, so deleting the clone does
>> actually delwte parts of the static buffer?
> 
> Hmm. That may be. I'm not sure if we copy absolutely everything over.
> And the Buffer object
> is so, so big

I tried, but this is not the problem: If I kill the static buffer and create 
a new one every time I still get an invalid read. I'll investigate further, 
but this may take some time.


Georg

Temp file locking problems on windows

[Georg Baum]

Hi,

while investigating bug 9234 we found out that our TempFile class opens the 
temp file in such a way that it cannot be renamed on windows (see 
http://www.lyx.org/trac/ticket/9234 for details). I developed a simple fix 
for that particular bug which Richard submitted at 
http://www.lyx.org/trac/changeset/2d338ee3/lyxgit, but afterwards I believed 
that the same problem exists at other places as well, and I extended the fix 
(see attachment).
However, after further thinking on it, I believe that I still do not 
understand 100% what is happening, and that the extended fix is not correct: 
Why does the renaming fail? I do understand that other processes cannot do 
anything to the file while it is open, because QTemporaryFile opens it in 
exclusive mode. However, the process that created the temp file must be able 
to qwrite to the file, otherwise the whole QTemporaryFile class would be 
useless. What is the deifference between writing to the file and renaming 
it?

I am pretty sure that the places where we create a temp file for use by an 
external command (e.g. for the various VCS log files, or for pasting HTML or 
LaTeX from the clipboard) do not work on windows. I am not sure at all about 
the cases where we write to the temp file in LyX itself. It would be nice if 
a windows user could test the VCS and pasting operations with and without 
the attached patch and report whether they work. Furthermore, I'd like to 
know whether the places where we write ourselves to the temp file do work as 
expected (e.g. autosave without the attached patch applied).


Georgdiff --git a/src/Buffer.cpp b/src/Buffer.cpp
index f236df3..c8d6244 100644
--- a/src/Buffer.cpp
+++ b/src/Buffer.cpp
@@ -1237,6 +1237,9 @@ Buffer::ReadStatus Buffer::convertLyXFormat(FileName const & fn,
 		<< ' ' << quoteName(fn.toSafeFilesystemEncoding());
 	string const command_str = command.str();
 
+	// work around qt file locking bug on windows
+	tempfile.close();
+
 	LYXERR(Debug::INFO, "Running '" << command_str << '\'');
 
 	cmd_ret const ret = runCommand(command_str);
@@ -1301,14 +1304,12 @@ bool Buffer::save() const
 	// proper location once that has been done successfully. that
 	// way we preserve the original file if something goes wrong.
 	string const justname = fileName().onlyFileNameWithoutExt();
-	boost::scoped_ptr
-		tempfile(new TempFile(fileName().onlyPath(),
-	   justname + "-XX.lyx"));
+	TempFile tempfile(fileName().onlyPath(), justname + "-XX.lyx");
 	bool const symlink = fileName().isSymLink();
 	if (!symlink)
-		tempfile->setAutoRemove(false);
+		tempfile.setAutoRemove(false);
 
-	FileName savefile(tempfile->name());
+	FileName savefile(tempfile.name());
 	LYXERR(Debug::FILES, "Saving to " << savefile.absFileName());
 	if (!writeFile(savefile))
 		return false;
@@ -1342,10 +1343,10 @@ bool Buffer::save() const
 		}
 	}
 
-	// Destroy tempfile since it keeps the file locked on windows (bug 9234)
-	// Only do this if tempfile is not in autoremove mode
+	// work around qt file locking bug on windows
 	if (!symlink)
-		tempfile.reset();
+		tempfile.close();
+
 	// If we have no symlink, we can simply rename the temp file.
 	// Otherwise, we need to copy it so the symlink stays intact.
 	if (made_backup && symlink ? savefile.copyTo(fileName(), true) :
@@ -3782,6 +3783,9 @@ int AutoSaveBuffer::generateChild()
 	tempfile.setAutoRemove(false);
 	FileName const tmp_ret = tempfile.name();
 	if (!tmp_ret.empty()) {
+		// work around qt file locking bug on windows
+		tempfile.close();
+
 		if (!buffer_.writeFile(tmp_ret))
 			failed = true;
 		else if (!tmp_ret.moveTo(fname_))
@@ -3868,6 +3872,9 @@ bool Buffer::autoSave() const
 	tempfile.setAutoRemove(false);
 	FileName const tmp_ret = tempfile.name();
 	if (!tmp_ret.empty()) {
+		// work around qt file locking bug on windows
+		tempfile.close();
+
 		writeFile(tmp_ret);
 		// assume successful write of tmp_ret
 		if (tmp_ret.moveTo(fname))
diff --git a/src/CutAndPaste.cpp b/src/CutAndPaste.cpp
index 7329c24..12c21d3 100644
--- a/src/CutAndPaste.cpp
+++ b/src/CutAndPaste.cpp
@@ -491,6 +491,8 @@ void putClipboard(ParagraphList const & paragraphs,
 	tempfile.setAutoRemove(false);
 	static Buffer * staticbuffer = theBufferList().newInternalBuffer(
 			tempfile.name().absFileName());
+	// work around qt file locking bug on windows
+	tempfile.close();
 
 	// These two things only really need doing the first time.
 	staticbuffer->setUnnamed(true);
diff --git a/src/LyXVC.cpp b/src/LyXVC.cpp
index 4dbcfd2..1953b04 100644
--- a/src/LyXVC.cpp
+++ b/src/LyXVC.cpp
@@ -370,6 +370,8 @@ string const LyXVC::getLogFile() const
 		LYXERR(Debug::LYXVC, "Could not generate logfile " << tmpf);
 		return string();
 	}
+	// work around qt file locking bug on windows
+	tempfile.close();
 	LYXERR(Debug::LYXVC, "Generating logfile " << tmpf);
 	vcs->getLog(tmpf);
 	return tmpf.absFileName();
diff --git a/src/VCBackend.cpp b/src/VCBackend.cpp
index 0e10aaa..e37fae9 100644
--- a/src/VCBackend.cp

Re: Is the COPYING file still part of the LyX distribution?

[Georg Baum]

aparsloe wrote:

> OK, thank you Stephan. Mine is just a standard Windows 'small' install
> (35 MB) and the file is definitely not included. Could you forward it to
> me?

Please file this as a bug of the windows installer at 
http://www.lyx.org/trac/wiki/BugTrackerHome. Every installer must also 
install a copy  of the license so that it can be found easily (I don't think 
it needs to be called COPYING though, if there is some other standard on the 
operating system like e.g. on debian).


Georg

Re: Problem with copy

[Georg Baum]

Richard Heck wrote:

> My problem is that I don't really know how to read the valgrind output.
> But from what you've said, my guess would be that the problem is in
> putClipboard, in CutAndPaste.cpp. You will see that a static Buffer does
> get created there, so you could try modifying the static bit and see if
> that helps. The other thing that happens is that this static Buffer gets
> cloned, and then the clone is deleted at the end. That might be the
> freed memory, but I'm not sure where it would get read.

Maybe the cloning does not work correctly, so deleting the clone does 
actually delwte parts of the static buffer?

BTW, my experience with valgrind is the same: It is right in almost all 
cases.


Georg

Re: Coding practice: OK to leave dangling pointer if not currently used?

[Georg Baum]

Scott Kostyshak wrote:

> My solution is to simply refresh the pointer (I can get it from
> currentBufferView()). However, this got me thinking: shouldn't we set
> the pointer to null every time we know that it was destroyed? What if
> we don't use that pointer again (as was the case before my commit)?

I don't know the code well, so I don't comment on that, but in my experience 
it is a good idea to always set a pointer to null if you know that it is no 
longer valid, regradless whether you plan to use it again or not (except 
member variables in destructors, in that case it is in most cases pretty 
clear). Otherwise, you can sometimes spend much time debugging strange 
problems.


Georg

Re: [LyX/master] Remove unsafe method FileName::tempName()

[Georg Baum]

Scott Kostyshak wrote:

> I'm seeing a regression and git bisect led me back to here. Georg, can
> you take a look to see if you can reproduce? If not, I will try to dig
> deeper. The regression is that the attached document shows the date
> before this commit for me but does not show the date (it shows nothing
> where the date is inserted) as of this commit. Attached is the
> document that leads to different outputs before and after this commit.
> As for how I created the document, I can reproduce on Ubuntu 14.04
> with the following:
> 1. Start a new document and wrote "Test: "
> 2. Go to Insert > File > External Material
> 3. Do not put anything in the "File" text box (I believe a "." is
> automatically inserted).
> 4. Change "Template" to "Date".
> 5. Compile

Does the temporary file in the buffer temp dir contain the date output, or 
is it just empty?

> Note that I'm not sure if this just resurfaced a different bug. I
> think I heard somewhere that the "Date" external template was not
> meant to be serious and was just put in as an example.

One difference to the old version is that the temp file does now exist when 
the date command is executed (this is part of the thread-safetyness). This 
should not matter, since the command reads

"python -tt $$s/scripts/date.py %d-%m-%Y > $$o"

but maybe it is locked somehow? Or something goes wrong with the extension 
of the temp file (it might be different now if I made a mistake). Another 
possibility is the one you mentioned: There might be another bug somewhere 
which is now uncovered. Unless you find something I'll have a look, but I am 
currently very busy.


Georg

Re: Absolute Paths in LyX-LaTeX documents

[Georg Baum]

dd3fmlli wrote:

> Hi all,
> I am a long-time LyX user and I can program  in C++ and Python, so I would
> like to  to give a  hand in the development of my favorite software.

You are very welcome! We can always need helping hands.

> Apart from this, I'm writing because  I was wondering how to use only
> relative paths in LyX documents, in particular in LaTeX preamble or TeX
> inserts. As far as I know this is impossible at the moment (e.g.  in the
> biblatex guide http://wiki.lyx.org/BibTeX/Biblatex is reported that the
> absolute path is needed).

Absolute paths are only needed for unsupported features. All file related 
insets allow relative paths, and preserve them even in the temp dir (either 
by copying the files, or by adding some search paths to the LyX generated 
preamble part, as you found out already, or by setting an environment 
variable with search paths).

> A possible, very naive, work around  of this issue would be to
> automatically add in the latex temporary file (I guess its name is
> "buffer", right?)
> 
> \def\relativePath{/from/relative/toAbs}
> 
> 
> or even
> 
> \newcommand{\makeAbsPath}[1] {\relativePath/#1}
>>
> and when  the document is exported in latex  the definition can be
> replaced by
> 
>> \def\relativePath{./}
> 
> 
> In the latex temp files  already exists something similar:
> 
>> \batchmode
>> \makeatletter
>> \def\input@path{{/home/random3f/Dropbox/EsperimentiTeX//}}
>> \makeatother
>>
> but it  doesn't work for bibtex and  possible packages a user want to add.
> 
> Is in your opinion a possible enhancement or could it create problems? 

I don't think that this would work in general. Like the current solution, it 
would only work for specific packages.

> Did
> you already considered  this, or similar, options? I could not find
> anything in the lists.
> I know that in case of white spaces in the paths it could create problems,
> but in many case it would solve this issue.

There have been discussions about relative and absolute paths, and also bug 
entries, but I don't remember the details. The current solution has evolved 
over the years. The general idea is: Be friendly to user code in the 
preamble and in ERT, i.e. try to make it work in as many cases as possible, 
but do never parse the preamble or ERT.

If you can find a solution along this line that improves the situation for 
some packages, then we should discuss it. However, I believe it will be 
quite difficult to do so, and I suggest a different approach: Why not 
starting to implement native biblatex support? See 
http://www.lyx.org/trac/ticket/4065 for details what has been thought 
already. The implementation part should not be difficult (many 
infrastructure parts are already there), what is currently missing is 
somebody who understands the interface of biblatex well enough to propose 
how to integrate it with LyX. If you are familiar with biblatex and want to 
give it a try then I think that many people would appreciate it very much!



Georg

Re: [LyX/master] Set the default locale at startup.

[Georg Baum]

Enrico Forestieri wrote:

> On Thu, Jul 10, 2014 at 10:33:14PM +0200, Georg Baum wrote:
>> 
>> This breaks the tex2lyx tests with a german locale, probably since
>> support::convert(std::string const s) wants now a comma instead
>> of a period as decimal point, so the string "0.3359375" gets now
>> converted to 0 instead of 0.3359375. Other conversions are probably
>> broken as well, and unfortunately this is also in 2.1.1.
> 
> I don't know how to run those tests and, most probably, also packagers.
> So, this is not a big deal.

This is not the point. The point is that there was a regression in the code 
which did not exist before. This regression was made visible by the tests 
(and if you want to know how to run them: see section 3 of 
lib/doc/Development.lyx).

> As regards other possible brekages, I only see one instance of
> convert in the tex2lyx sources, namely in scale_as_percentage(),
> and this is about converting a scale parameter for fonts. So, I tried
> to import the attached latex document with either 2.1.0 and 2.1.1.
> Both were broken for different reasons. The 2.1.0 version produced
> a line "\font_sf_scale 92.5" which was later rejected as an integer
> and treated as 0. The 2.1.1 version directly produced 0, thus avoiding
> a warning ;-)

I am not sure whether restricting scale values to integers is a good idea. 
Scales are inherently floating point values, and LaTeX uses floating point 
values as well. Therefore, LyX should be fixed to accept floating point 
values for all font scale parameters, and the integer restriction in tex2lyx 
should be reverted.
 
> So, it seems there is no regression...

There was a regression, see the attached diff (which I should have attached 
yesterday, but forgot). It might not be caused by convert, but 
tex2lyx created valid LyX files before the change, and after the change they 
were still valid, but the contents was not in line with the TeX file 
anymore. After your fix every test is fine again.


Georgdiff --git a/src/tex2lyx/test/XeTeX-polyglossia.lyx.lyx b/src/tex2lyx/test/XeTeX-polyglossia.lyx.lyx
index 99b5d0e..64b043e 100644
--- a/src/tex2lyx/test/XeTeX-polyglossia.lyx.lyx
+++ b/src/tex2lyx/test/XeTeX-polyglossia.lyx.lyx
@@ -27,7 +27,7 @@
 \use_non_tex_fonts true
 \font_sc false
 \font_osf false
-\font_sf_scale 75
+\font_sf_scale 0
 \font_tt_scale 100
 \graphics default
 \default_output_format pdf4
diff --git a/src/tex2lyx/test/box-color-size-space-align.lyx.lyx b/src/tex2lyx/test/box-color-size-space-align.lyx.lyx
index e83330b..d16997a 100644
--- a/src/tex2lyx/test/box-color-size-space-align.lyx.lyx
+++ b/src/tex2lyx/test/box-color-size-space-align.lyx.lyx
@@ -84,7 +84,7 @@
 \justification true
 \use_refstyle 0
 \notefontcolor #ff
-\backgroundcolor #ff5500
+\backgroundcolor #ff
 \boxbgcolor #00
 \index Index
 \shortcut idx

Re: [LyX/master] Set the default locale at startup.

[Georg Baum]

Am Mittwoch, 11. Juni 2014 schrieb Enrico Forestieri:
> commit 82faa6619239c2e57fba9128899bafe29d728e51
> Author: Enrico Forestieri 
> Date:   Wed Jun 11 18:23:44 2014 +0200
> 
> Set the default locale at startup.
> 
> On startup, the default locale is "C", meaning that all system
> functions assume an ascii codeset. The environment's locale
> settings should be selected by calling setlocale(LC_ALL,"").
> This is done by Qt during the QCoreApplication initialization
> but this inizialization is never performed for batch processing
> and, as a result, LyX is not able to process files whose names
> contain non-ascii characters. This is not an issue on Windows,
> where the file names are always stored as UTF-16, so the call is
> only performed for unix-like platforms (this also includes cygwin,
> due to its own filenames management that allows using characters
> which are forbidden to native programs).
> 
> diff --git a/src/support/os_cygwin.cpp b/src/support/os_cygwin.cpp
> index 92cbf15..4179d49 100644
> --- a/src/support/os_cygwin.cpp
> +++ b/src/support/os_cygwin.cpp
> @@ -215,6 +215,9 @@ void init(int argc, char * argv[])
>   argc_ = argc;
>   argv_ = argv;
> 
> + // Set environment's default locale
> + setlocale(LC_ALL, "");
> +
>   // Make sure that the TEMP variable is set
>   // and sync the Windows environment.
>   setenv("TEMP", "/tmp", false);
> diff --git a/src/support/os_unix.cpp b/src/support/os_unix.cpp
> index b85bdb2..03dfb38 100644
> --- a/src/support/os_unix.cpp
> +++ b/src/support/os_unix.cpp
> @@ -46,6 +46,9 @@ void init(int argc, char * argv[])
>  {
>   argc_ = argc;
>   argv_ = argv;
> +
> + // Set environment's default locale
> + setlocale(LC_ALL, "");
>  }

This breaks the tex2lyx tests with a german locale, probably since 
support::convert(std::string const s) wants now a comma instead of 
a period as decimal point, so the string "0.3359375" gets now converted to 
0 instead of 0.3359375. Other conversions are probably broken as well, and 
unfortunately this is also in 2.1.1.

The question is: Do we need two versions of support::convert() (one taking 
the current locale into account for user inout/output, and one always using 
the "C" locale for internal conversions), or was the old behaviour correct?


Georg

Re: [LyX/master] Make theWordList() thread safe.

[Georg Baum]

Scott Kostyshak wrote:

> I think I understand more now. But why in this commit do you choose a
> mutex? Why is it obvious that "a deadlock can't happen" ? And why "it
> should give better performance" ?

A deadlock would mean that thread x has the lock and waits for thread y, 
while thread y waits for thread x. This can't happen, because

a) Formats::isZippedFile() does not call any method or function that 
communicates in any way with other threads, and

b) it does not use any shared resource. Even if one tries to construct a 
situation where the file being queried in thread x is currently opened 
exclusively in another thread or process, and thread y enters 
isZippedFile(), you may get wrong information about the zipped status in 
thread x, but no deadlock. The wrong information is caused by the missing 
error handling of guessFormatFromContents(), and this problem would even 
exist in a single threaded LyX.

Therefore, if one thread enters isZippedFile(), it will eventually finish 
without waiting for any other thread, so the worst thing that happens if two 
threads enter isZippedFile() is that one has to wait.

The better performance is only my guess. My line of thought was this: 
getFormatFromFile() is expensive (otherwise the whole cache would not be 
needed at all), and the probability that two threads arrive at the same time 
in isZippedFile() is low, and the cost of obtaining a lock without conflict 
is low as well => share the result of getFormatFromFile() between threads.
As always when you want to know something about performance, you should 
measure, but this can be quite difficult, and we are not talking about an 
inner loop of a complicated numerical procedure, so I was lazy and did not 
measure.

> In terms of efficiency, in general if you can use either a mutex or
> local thread storage, if the object is light it seems like local
> thread storage would be more efficient because no thread would ever
> have to wait to get access to the shared object. But if the object is
> heavy, then maybe the overhead of creating it in multiple threads is
> more than the time saved from threads waiting their turn. Is that at
> all correct?

Basically yes, but you need to take into account the computation of the 
object as well: The object might be very light (in this case it is a bool), 
but the computation might be quite expensive. memory-wise it would not be 
any problem to use thread-local data, but computation time wise you would 
see a difference.

In LyX we have a quite comfortable situation: It is a small project (so you 
can check for deadlocks easily in many cases), and it has only a few 
performance-critical areas, so in many cases it does not matter if we take 
the wrong decision performance wise;-)



Georg

Re: [LyX/master] Make theWordList() thread safe.

[Georg Baum]

Scott Kostyshak wrote:

> How do you decide between a mutex or local thread storage? My wild
> guess is (I have no experience with concurrency issues but have wanted
> to study them): local thread storage is better if threads do not need
> to share memory between them.

Basically yes. In some cases there is no choice, e.g. in 0de4bc224af3: The 
mangled name must be the same for any given input name, regardless of the 
thread => use mutex. Another case is the wordlist: Cloned buffers must not 
disturb the wordlist of any other buffer => use thread local storage. In 
other cases it does not really matter, e.g. 4a2250a.


Georg

Re: [LyX/master] Make theWordList() thread safe.

[Georg Baum]

Richard Heck wrote:

> On 07/07/2014 06:46 AM, Jürgen Spitzmüller wrote:
>> Am Sonntag 06 Juli 2014, 21:03:14 schrieb Kornel Benko:
 This one was the only one where I realy saw a crash. From the remaining
 ones 2a677592 and 5093893b5 also fixed other bugs, and 5a8b8ba8 is
 important, so I propose these as well. For the remaining ones I don't
 know: I am pretty sure that these fixes are correct, but they are morre
 exotic. What do others think?
>>> I think all of them should go to branch as well.
>> Me, too. Thanks for this work, BTW.
> 
> Go ahead, then, Georg. (And thanks also from me.)

OK, I'll merge all fixes then. You are all welcome. It was quite easy btw: 
We have two needed tools (a mutex and local thread storage), so my algorithm 
was:

1) search for FIXME THREAD, pick one that looks interesting
2) decide what to do: Is it possible to get rid of the static data 
completely? If not, is it better to use a mutex or local thread storage?
3) implement and test the solution
4) goto 1) until I could not be bothered anymore

Unfortunately some of the remaining FIXME THREAD occurances are more 
difficult. I might have a look at them as well, but I don't know yet.


Georg

Re: [LyX/master] Fix Tabular::CellData::operator=()

[Georg Baum]

Jean-Marc Lasgouttes wrote:

> Is there a reason for keeping CellData::swap now? What does it do that
> std::swap(CellData&, CellData&) does not do?

Good question, I did not think about it (Length::swap() is similar btw). It 
does exactly the same, the only reason to want it that comes to my mind is 
that std containers have swap() as member function as well, so you can do

a.swap(b);

However, the code duplication is worse, so I'll remove both mentioned swap() 
methods.


Georg

Re: [LyX/master] Make theWordList() thread safe.

[Georg Baum]

Richard Heck wrote:

> How many of the other thread safety commits do you think are needed in
> branch? I'm kind of guessing they'd all be nice...

This one was the only one where I realy saw a crash. From the remaining ones 
2a677592 and 5093893b5 also fixed other bugs, and 5a8b8ba8 is important, so 
I propose these as well. For the remaining ones I don't know: I am pretty 
sure that these fixes are correct, but they are morre exotic. What do others 
think?


Georg

Re: [LyX/master] Make theWordList() thread safe.

[Georg Baum]

Richard Heck wrote:

> Good catch. That said, clones are wasting their (and our!) time
> inserting things into the WordList.  Would it be hard to stop them from
> bothering?

Yes, that would be better. Unfortunately I am not familiar with the clone 
machinery nor the wordlist, so I did nothing more than making the existing 
code threadsafe.


Georg

Re: [GSoC 2014]ODT-->LyX Conversion

[Georg Baum]

Prannoy Pilligundla wrote:

> Hi Everyone,
> 
> I am almost done with LyX to ODT part of my project and I started work on
> the roundtrip i.e ODT to LyX part of the project. For LyX to ODT I
> converted LyX to LaTeX first and then used tex4ht for converting LaTeX to
> ODT. In case of math I am also storing the latex expression as it as in
> the ODT file(using annotation property) as meta data.

Very nice!

> Now for getting back to LyX from ODT I am thinking of writing a python
> script which does this conversion. As of now I am planning to use the xml
> parsers expat library .I
> also had a look at lyx2lyx scripts. I tried using Writer2LaTeX but it
> doesn't suit our purpose of getting back the same LyX file. I am not very
> clear on how to go ahead from this juncture. For example I am not clear on
> how I will get information about all the packages used from the ODT file.
> I feel I am not aware of many issues which need to be taken care of during
> this conversion process. It would be great to hear your comments and
> suggestions on how to go ahead

Did you have a look at the old discussions we had before your GSoC project 
started? There were many ideas floating around, but I don't remember the 
details anymore.

I think the most important goal would be to get the structure right. Then it 
becomes complicated: You need to find out if certain modules are needed for 
a certain feature (packages are LaTeX stuff, these should be irrelevant if 
you go from odt to LyX). This can be quite challenging, since you basically 
need to use LyX to get the available modules etc. At this point, you would 
need to reimplement a lot of LyX infrastructure in python.

Another approach would be to interface with the LyX server, query it for 
modules, and let it write the LyX file. Then you would not need to care 
about changing file formats. However, it might be needed to extend the LyX 
server interface, I don't know if it is powerful enough.

Finally, it would also be possible to hack tex2lyx to read odt. You could 
use a lot of the existing infrastructure. Maybe it is not so much work to 
use a C++ XML parser (e.g. from qt) to read the document, then process it 
and use existing code to write the LyX file?


Georg

Re: [LyX/master] Make theWordList() thread safe.

[Georg Baum]

Georg Baum wrote:

> commit cf89851374163be2e8a390dd917d7ef435978979
> Author: Georg Baum 
> Date:   Fri Jul 4 22:19:43 2014 +0200
> 
> Make theWordList() thread safe.
> 
> Without this, you get crashes in a few second when you set the
> autosave interval to one second and edit quickly (typing new words
> etc). The reason is that the cloned buffer wants to insert words into
> the word list and remove them again, but it lives in a different
> thread.

This should go to the 2.1 branch after 2.1.1 is released IMHO. This fixes a 
crash that can easily be triggered by autosave (although not the one we are 
hunting).


Georg

Re: Debug information for lyx crash with data loss

[Georg Baum]

Jean-Marc Lasgouttes wrote:

> Le 02/07/14 17:07, Richard Heck a écrit :
>> There was a report from someone who did not have autosave enabled.

Maybe there are two different triggers? When I find some time, I'll play a 
bit with autosave.

> The other possibility is to look at changes that occurred in Tabular
> code. The main difference that I see is new support for
> insert/duplicate-row. Do we have indications that people have used these
> functions in their editing session?
> 
> I find the code for CellData::operator= quite weird:
> 
>Tabular::CellData & Tabular::CellData::operator=(CellData cs)
>{
> swap(cs);
> return *this;
>}
> 
>void Tabular::CellData::swap(CellData & rhs)
>{
> std::swap(cellno, rhs.cellno);
> std::swap(width, rhs.width);
>...
> 
> Is it possible that this code does weird things when duplicating a row?
> I do not thin that an operator= implementation with a parameter passed
> by value is very standard. And I do not know how this mixes with the
> semantics of shared_ptr (used by the inset member).

This looks indeed strange. I'd definitely change this to standard signature 
in trunk, but from a quick glance I don't see any immediate problem with 
this code.


Georg

Re: Debug information for lyx crash with data loss

[Georg Baum]

Richard Heck wrote:

> On 07/01/2014 04:36 PM, Georg Baum wrote:
>> Richard Heck wrote:
>>
>>> That was what I was thinking, too. Just guessing at this point.
>> Yes. It would be really nice if we had a more reliable way to reproduce
>> the crash. OTOH it is good that it does not occur very often;-)
>>
>> I went over the code again and found and fixed a memory leak (and missing
>> iconv_close() call caused by that), but I doubt that this has anything to
>> do with the crash: An IconvProcessor instance is only copied when
>> inserting a new one into the map in ucs4To8bitProcessors(), but to_utf8
>> uses its own dedicated IconvProcessor which is never copied.
> 
> Do you think that should go to 2.1.1? Or does it need testing?

I think it would need testing. I also think that it probably does not affect 
the bug, so I'd rather keep it out.


Georg

Re: Debug information for lyx crash with data loss

[Georg Baum]

Richard Heck wrote:

> That was what I was thinking, too. Just guessing at this point.

Another wild guess: Since the backtrace shows autoSave(), and autoSave() 
runs in a separate thread and operates on the cloned buffer: Could the non-
thread-safety of the cloning business be the real problem? At least I saw 
some comments about that in Buffer.cpp, maybe there are more places where 
the cloned buffer is used in an unsafe way?

It may be possible to verify whether this guess is true by doing autoSave() 
in a loop, and then trying to do any other editing operations at the same 
time.


Georg

Re: Debug information for lyx crash with data loss

[Georg Baum]

Richard Heck wrote:

> That was what I was thinking, too. Just guessing at this point.

Yes. It would be really nice if we had a more reliable way to reproduce the 
crash. OTOH it is good that it does not occur very often;-)

I went over the code again and found and fixed a memory leak (and missing 
iconv_close() call caused by that), but I doubt that this has anything to do 
with the crash: An IconvProcessor instance is only copied when inserting a 
new one into the map in ucs4To8bitProcessors(), but to_utf8 uses its own 
dedicated IconvProcessor which is never copied.


Georg

Re: Debug information for lyx crash with data loss

[Georg Baum]

Richard Heck wrote:

> On 06/30/2014 06:54 PM, Pavel Sanda wrote:
>> Richard Heck wrote:
>>> Paragraph::write, so the crashing call to to_utf8 could come in any of
>>> them. The crashing call certainly isn't the first one, since
>>> "\begin_inset
>> smells little bit like concurrency problem inside to_utf8.

to_utf8() should not have any concurrency problem anymore. Each thread uses 
its own data, and in one thread you cannot be twice in to_utf8 at any given 
time.

>> we might try doing autosave while using to_utf8 somewhere in ui?
> 
> Is it possible that, somehow, the use of per-thread storage is causing
> crashes now in exactly the
> same cases where we previously got file corruption?

I am pretty sure that our own code related to iconv is completely thread 
safe now. However, if there was a bug in QThreadStore, or we misunderstood 
how QThreadStore works, then this could indeed be the case.


Georg

Re: Debug information for lyx crash with data loss

[Georg Baum]

Richard Heck wrote:

> The key part is:
> 
> #15 0x00a99af0 in lyx::to_utf8(std::basic_string #std::char_traits, std::allocator > const&) ()
> No symbol table info available.
> #16 0x005c6577 in lyx::Paragraph::write(std::ostream&,
> #lyx::BufferParams const&, unsigned long&) const ()
> No symbol table info available.
> #17 0x005ed142 in lyx::Text::write(std::ostream&) const ()
> No symbol table info available.
> #18 0x00830afa in lyx::Tabular::write(std::ostream&) const ()
> No symbol table info available.
> 
> So the crash actually is within a table, but it's happening in the call
> to to_utf8. The only such calls in Paragraph::write are at the very
> beginning and the very end, the latter hidden in the call to flushString
> (which may be inlined).

Well, flushString() is called several times.

> Is invalid data being passed to to_utf8?

Either that, or ucs4_to_utf8() returned invalid data, or some memory 
corruption happened earlier (or in ucs4_to_utf8()) which kicks now in when 
new objects are allocated. I can't see any other possibility for a crash in 
the two lines of to_utf8().


Georg

Re: cannot compile LyX with MSVC 2013 and Qt 5 or Qt 4 - need CMake help

[Georg Baum]

Uwe Stöhr wrote:

> I was now able to compile Master with Qt 5.3. LyX works so far, but
> crashes immediately when trying to view a file as PDF. So yes, Qt 5 and
> LyX does not yet work but we should try to make this possible for LyX
> LyX 2.2.

Yes. There have been many discussion already on that topic, and part of the 
work has been done, but it is not finished. Somebody needs to volunteer.

> Now I spent some more hours and found out that having .NET 4 installed
> side by side with .Net 4.5.1 is possible if .Net 4 is installed before
> .NET 4.5.1
> 
> I am therefore now able to compile LyX 2.1 using Qt 4.8.6 and MSVC 2010.

Very good. For the record: If you want to build with a never MSVC version, 
then you need all dependencies built with that version as well. Otherwise 
you'll get either linker errors or crashes at runtime, since both the C 
runtime library and the C++ libraries (runtime and STL) are not binary 
compatible between different MSVC releases. Sometimes you even need the MSVC 
service pack to match (this was the case with a MSVC 2005 bug fix).

> So what is wrong with using a merged build? I am pretty sure that I used
> merged builds for the release of LyX 2.0.8 and it worked fine.

Nothing is wrong with it, but AFAIK it is not tested, and code that is not 
tested breaks (always, the only question is how long it takes). However, for 
the resulting binary it should not matter whether you use a merged build or 
not (otherwise the merged build would be broken). The only difference should 
be build speed.


Georg

Re: LyX 2.1.1, Again

[Georg Baum]

Uwe Stöhr wrote:

> I get this warning when compiling current LyX 2.1.x branch:
> 
> "D:\LyXGit\Master\compile-result\src\LyX.vcxproj" (Standard target) (3) ->
> (ClCompile target) ->
> ..\..\src\Paragraph.cpp(3855): warning C4018: '<': conflict between
> 'signed' and 'unsigned' [D:\LyXGit\Master\compile-resul\src\LyX.vcxproj]
> 
> Maybe this can be fixed before LyX 2.1.1 is released?

Not needed. Line 3855 contains only a closing brace '}' in 2.1 branch. The 
warning makes sense if you look at master. There you have

if (pos < from + lyxrc.completion_minlength)

which is indeed a comparison between signed and unsigned values. I'll fix 
that, but nothing needs to be done for 2.1.1.


Georg

Re: cannot compile LyX with MSVC 2013 and Qt 5 or Qt 4 - need CMake help

[Georg Baum]

Kornel Benko wrote:

> Am Mittwoch, 25. Juni 2014 um 23:29:34, schrieb Uwe Stöhr
> 
>> 
>> That would be very helpful. This way I can try Qt 5 for master and use
>> Qt 4 to compile releases of 2.1. Please commit.
> 
> Done. I am pretty sure, this does not break users using Qt4.
> 
> I hope, Richard does not object?

Please commit to the stable branch after getting the OK, not before, 
especially when a release is due in the very near future. The change is 
good, but not too urgent, and it really helps to have very clear rules.


Georg

Re: LyX support for including unnumbered sections in TOC

[Georg Baum]

Scott Kostyshak wrote:

> A module sounds like a good idea. So the module would add new layouts
> such as Section* (TOC), Chapter* (TOC). Any other ideas for the name?
> Section* (+TOC) ?

I'd steal the ones from src*.layout. This has the advantage that the styles 
stay intact if you change the document class to or from e.g. scrbook. The 
are called Addchap, Addsec etc. in LyX and addchap, addsec etc. in LaTeX. 
Another advantage is that no new translations are needed.

Georg

Re: LyX support for including unnumbered sections in TOC

[Georg Baum]

Richard Heck wrote:

> On 06/24/2014 07:35 PM, Scott Kostyshak wrote:
>> This has come up twice (in the context of LyX) in the past week on
>> Stack Exchange:
>> http://tex.stackexchange.com/questions/185367/add-an-unnmbered-section-
to-the-table-of-contents-in-lyx
>> http://tex.stackexchange.com/questions/186398/how-to-remove-enumerated-
chapter-latex-in-lyx
>>
>> Is this something that would be useful to add support for in LyX?
> 
> Yes, it's a very common question.

Yes indeed. And it is already supported, depending on the document class.

>> Thoughts on interface:
>> One way that would not clutter the GUI too much would be a checkbox
>> that says something like "add unnumbered divisions to TOC if numbered
>> counterparts are added".
>> I think there is more demand for a categorical solution than a
>> solution that allows the user to decided whether each individual
>> unnumbered division should be in the TOC.

Is it really the common case that all unnumbered sections of a certain kind 
should be added? IMHO  the solution offered by the KOMA classes is the best 
one: They offer additional unnumbered environements with TOC entries. This 
is even supported by LyX. For other classes one could easily write a module 
that adds equivalent environments.

> I think I would go with (a). That kind of redefinition scares me, since
> we support so many
> document classes, etc, and you never know exactly how sectioning
> commands are defined
> in any particular one.

I don't like the redefinition either. I'd put the \addcontentsline in a 
module (and not hardcoded in LyX).


Georg

Re: Pdflatex crashes on command line with university pdf logo

[Georg Baum]

Jean-Marc Lasgouttes wrote:

> 12/06/2014 13:53, Enrico Forestieri:
>> As regards LyX behaving similarly, apart this specific case (that, BTW,
>> only seems affecting LyX 2.1 and 2.2, most probably due to the gettext
>> removal, and that, BTW2, does not manifests with Qt5), I think that this
>> is already the case, as a QCoreApplication would only give you an event
>> loop that we, apparently, don't need. But I am not an expert in this
>> field, so take that only as a IMHO comment.
> 
> I remember a discussion where Georg hinted that we may have to turn
> tex2lyx into a real QApplication because of problems with the QFileName
> class. I wonder whether it is the same issue.

Unfortunately I don't remember anymore where I read it, and a quick search 
in the archives did not turn up that conversation. What I do remember is 
that QFile (or QFileInfo?) relies on some initialization which is done in 
QApplication, so it could be the same issue indeed.


Georg

Re: [PATCH] CMake missing libMagic on mac

[Georg Baum]

Stephan Witt wrote:

> Am 12.06.2014 um 15:35 schrieb Kornel Benko :
> 
>> Also missing call to 'find_package(Magic REQUIRED)' at CMakeLists.txt:574
> 
> I don't understand. It cannot be required. It's not available on Mac.

It is intended to be optional (but LyX works better if it is available). At 
least in th source code it is correctly protected by HAVE_MAGIC_H.


Georg

Re: Data Saving Patch for 2.1.x

[Georg Baum]

Richard Heck wrote:

> On 06/10/2014 11:35 AM, Pavel Sanda wrote:
>> Richard Heck wrote:
>>> On 06/10/2014 02:50 AM, Pavel Sanda wrote:
 Richard Heck wrote:
> There are some issues (e.g, with symlinks)
 So are the symlinks preserved or not with this patch?
>>> The exact same thing happens as if backups are enabled and the save
>>> fails: If the save works, the new file is still a symlink; if the save
>>> fails, then, since the backup was made by copying, the restored file is
>>> not a symlink, but a copy.
>> I see, thanks.
> 
> Since I see no other objection, I have committed this.

Finally I had some time to look at it, and don't have any objections either, 
but it looks like you forgot to push;-)


Georg

Re: Data Saving Patch 2

[Georg Baum]

Richard Heck wrote:

> On 06/09/2014 07:27 AM, Georg Baum wrote:
>>
>> Therefore I vote for taking it out for 2.1.1 (but keep the higher number
>> of flush()). If you disagree then both bf782ee02a and f792e70d0a should
>> be backported of course.
> 
> That sounds as if it may be necessary. What about the simpler approach I
> used
> https://www.mail-archive.com/lyx-devel@lists.lyx.org/msg184575.html
> there? The basic idea, which could be made even simpler, was just to
> copy the
> existing LyX file to a new location before saving the new one. Then it
> does not
> get over-written. JMarc had criticisms of it as implemented that
> depended upon
> details that could be fixed.

Yes, that looks like it can be implemented more safe.


Georg

Re: Data Saving Patch 2

[Georg Baum]

Jürgen Spitzmüller wrote:

> I tried with latest master (including your recent changes), and I still
> get the warning.
> * Open document
> * Edit
> * Save
> => No warning
> * Edit more
> * Save again
> => This time, the warning dialog pops up.

I see it as well with this recipe, and fixed it at f792e70d0a. This shows 
clearly that there was a gap both in Richards and my testing, otherwise we 
would have seen it. Given the fact that there is still a highly nontrivial 
change missing to fix the last (known) regression (the file permission 
problem) I don't think anymore that saving to a temp file first and then 
renaming it is safe enough for 2.1.1. We simply cannot test it enough to 
ensure that it does not make the situation worse.

Therefore I vote for taking it out for 2.1.1 (but keep the higher number of 
flush()). If you disagree then both bf782ee02a and f792e70d0a should be 
backported of course.


Georg

Re: Data Saving Patch 2

[Georg Baum]

Richard Heck wrote:

> Nitpicking is exactly what we need here, for the reasons you give. I'd
> suggest you go ahead and commit this, and we can deal with the
> permissions issue next.

A slightly modified version which does not crash with circular symlinks is 
in at bf782ee02ac. Note that circular symlinks are not an issue for this 
particular use of copyTo() (such a file cannot be opened anyway), but I 
wanted to make sure to avoid such a crash for future users of copyTo.

> That doesn't seem that bad, does it? We just need to find the original
> permissions and then set the new file to have them as well?

Yes. However, I believe this can be quite tricky if ACLs come into the play.

> By the way, did you see this report?
>> 2014-06-07 9:06 GMT+02:00 Scott Kostyshak:
>>
>> I've been getting the following message frequently when doing a
>> normal save (and I have not been doing anything tricky such as
>> editing the file externally):
>> "Document xyz.lyx has been externally modified. Are you sure you want
>> to overwrite this file?"
>> I did not confirm that the above commit is what changed this
>> behavior, but I thought I would check in to see if anyone else sees
>> this?
>>
>>
>> Yes, I see this, too.
>>
>> Jürgen
> 
> Did you notice if this still happens with your patch?

I saw the report, but I did not see it happening, with or without my patch. 
Scott and Jürgen, maybe you try to save on a network file system? Maybe it 
has something to do with the checksumming.


Georg

Re: Data Saving Patch 2

[Georg Baum]

Georg Baum wrote:

> Some of my remarks may look like nitpicking. However, we are trying to
> work around a problem which does not occur very often (although it causes
> big damage if it occurs), and this is a central infrastructure part.
> Therefore we have to be extra careful not to destroy the many working use
> cases (which might contain subtle details, see e.g. bug 6587).

And I forgot: Because of this the crash for circle symlinks in my patch 
needs to be fixed as well.


Georg

Re: Data Saving Patch 2

[Georg Baum]

Richard Heck wrote:

> On 06/02/2014 12:57 PM, Jean-Marc Lasgouttes wrote:
>> Le 02/06/14 15:05, Richard Heck a écrit :
>>>
>>> I propose to commit the attached. The effect is: (i) We always create a
>>> backup file in the current directory, but in the backup directory if
>>> "Make Backups" is true; (ii) If we are unable to do so, we ask the user
>>> if s'he wants to proceed; (iii) We delete the backup file after a
>>> successful save, unless "Make Backups" is true.
>>
>> Does this mean that we delete old backup when saving without backups
>> enabled?
>>
>> What's wrong with what I proposed nitially, that is to create the file
>> under some special name, and rename it to the right name on success?
>> This means that there is no file to copy.
> 
> Try this one, then.

Finally I had some time to do tests and study the code. It works well in 
general, but I also found some problems:

- The name of the temp file is very predictable, and does not depend on the 
document name. _If_ the user has some problems, and LyX crashes frequently 
before the temp file is removed, then you will quickly end up with 100 temp 
files. Afterwards, you will not be able to save any file anymore.

- The generation of the temp file name contains a race condiction regarding 
another LyX instance saving in the same directory.

- Symlinks do not survive saving anymore

- File permissions do not survive saving anymore. This could lead to 
unwanted permission widening if the default permissions for new files are 
less restrictive than the particular file you are working on. This was also 
the case with the backups already, but these can be switched off.

All problems but the last one are fixed by the attached patch. I reused the 
TempFile class for atomic and random temp file name generation to fix the 
first two problems. Fixing the last problem correctly is probably more work, 
but it is still important IMHO.

Some of my remarks may look like nitpicking. However, we are trying to work 
around a problem which does not occur very often (although it causes big 
damage if it occurs), and this is a central infrastructure part. Therefore 
we have to be extra careful not to destroy the many working use cases (which 
might contain subtle details, see e.g. bug 6587).


Georgdiff --git a/src/Buffer.cpp b/src/Buffer.cpp
index 7bc75e4..866e4dc 100644
--- a/src/Buffer.cpp
+++ b/src/Buffer.cpp
@@ -1284,25 +1284,8 @@ bool Buffer::save() const
 	// we first write the file to a new name, then move it to its
 	// proper location once that has been done successfully. that
 	// way we preserve the original file if something goes wrong.
-	string const savepath = fileName().onlyPath().absFileName();
-	int fnum = 1;
-	string const fname = fileName().onlyFileName();
-	string savename = "tmp-" + convert(fnum) + "-" + fname;
-	FileName savefile(addName(savepath, savename));
-	while (savefile.exists()) {
-		// surely that is enough tries?
-		if (fnum > 100) {
-			Alert::error(_("Write failure"),
- bformat(_("Cannot find temporary filename for:\n  %1$s.\n"
-			 "Even %2$s exists!"),
-	 from_utf8(fileName().absFileName()),
-			 from_utf8(savefile.absFileName(;
-			return false;
-		}
-		fnum += 1;
-		savename = "tmp-" + convert(fnum) + "-" + fname;
-		savefile.set(addName(savepath, savename));
-	}
+	TempFile tempfile(fileName().onlyPath(), "tmpXX.lyx");
+	FileName savefile(tempfile.name());
 
 	LYXERR(Debug::FILES, "Saving to " << savefile.absFileName());
 	if (!writeFile(savefile))
@@ -1310,6 +1293,7 @@ bool Buffer::save() const
 
 	// we will set this to false if we fail
 	bool made_backup = true;
+	bool const symlink = fileName().isSymLink();
 	if (lyxrc.make_backup) {
 		FileName backupName(absFileName() + '~');
 		if (!lyxrc.backupdir_path.empty()) {
@@ -1321,7 +1305,7 @@ bool Buffer::save() const
 
 		// Except file is symlink do not copy because of #6587.
 		// Hard links have bad luck.
-		made_backup = fileName().isSymLink() ?
+		made_backup = symlink ?
 			fileName().copyTo(backupName):
 			fileName().moveTo(backupName);
 
@@ -1333,8 +1317,13 @@ bool Buffer::save() const
 			//LYXERR(Debug::DEBUG, "Fs error: " << fe.what());
 		}
 	}
-	
-	if (made_backup && savefile.moveTo(fileName())) {
+
+	// If we have no symlink, we can simply rename the temp file.
+	// Otherwise, we need to copy it so the symlink stays intact.
+	if (!symlink)
+		tempfile.setAutoRemove(false);
+	if (made_backup &&
+	(symlink ? savefile.copyTo(fileName(), true) : savefile.moveTo(fileName( {
 		markClean();
 		return true;
 	}
diff --git a/src/support/FileName.cpp b/src/support/FileName.cpp
index 47d6669..f26b9b5 100644
--- a/src/support/FileName.cpp
+++ b/src/support/FileName.cpp
@@ -217,9 +217,15 @@ void FileName::erase()
 }
 
 
-bool FileName::copyTo(FileName const & name) const
+bool FileName::copyTo(FileName const & name, bool keepsymlink) const
 {
 	LYXERR(Debug::FILES, "Copying " << name);
+	if (keepsymlink && name.isSymLink(

Re: [LyX/master] build with mingw on Windows and Linux

[Georg Baum]

Peter Kümmel wrote:

> commit 080fca85a8fb6714afd7c34e4070687705fa1cb2
> Author: Peter Kümmel 
> Date:   Sat Jun 7 07:59:52 2014 +0200
> 
> build with mingw on Windows and Linux

Very nice!


Georg

Re: Can no longer paste PNG's automatically on trunk (intended?)

[Georg Baum]

Scott Kostyshak wrote:

> Thanks for taking a look, Georg. From what I understand, the only
> missing piece to the patch is the XML parser.

Yes.

> But I added a line
> lyxerr << "str is: " << str << std::endl;
> right before the "Now use some XML parser" comment, and it is only
> triggered when pasting from Firefox, not from Chromium.

This indicates either a bug (maybe GuiClipboard::on_dataChanged() is not 
called although it should), or I misunderstood how 
GuiClipboard::on_dataChanged() works.
However, I think you got the general idea.


Georg

Re: Lyx<-->Word conversion and images: suggestions sought

[Georg Baum]

stefano franchi wrote:

> You're right, I didn't make myself clear again. LIbreoffice supports a
> number of graphic formats. Not pdf though. It does support EPS and SVG,
> plus a number of common bitmap formats such as png, gif, jpeg, etcetera.
> (SVG does not seem to work well, thougg, at least not with my
> inkscape-generated SVG files).

Thanks for the explanation, I understand it now.

> Prannoy has worked out how to instruct tex4ht to use "convert"  and
> produce png from pdf. I guess he could substitute a conversion to EPS
> instead (with pdftops -eps), but I am not sure if there is any real
> benefit. I am really having troubles imagining a scenario in which a
> self-contained, converted ODT file would be used to produce high-quality
> printed output. But I may be lacking imagination.

I have no idea whether that scenario is used. However, it is not important 
right now IMHO: If tex4ht can be made to call convert, then it could use any 
converter later if needed (in theory).


Georg

Re: LyX 2.1.1

[Georg Baum]

Richard Heck wrote:

> 
> With this patch in place, I would like to move toward a 2.1.1 release as
> soon as possible. This does not fix the random crash people have been
> seeing, but it should prevent really serious dataloss, and that is a
> very good reason for a release.

I agree.

> What other bugs do people think we should try to fix before 2.1.1? Any?
> Other than the one that is such a mystery?

I am not aware of any. There are a number of other problems, but those can 
wait.

However, this patch needs very good testing during the next days, because it 
has the potential to make the situation even worse. I'll also try to look 
carefully over it when I find some time.


Georg

Re: Can no longer paste PNG's automatically on trunk (intended?)

[Georg Baum]

Scott Kostyshak wrote:

> On Mon, Jun 2, 2014 at 2:52 PM, Georg Baum
>  wrote:
> 
> You are correct. What I listed was the difference between Chromium
> (which does not work) and Firefox (which does work). If you want me to
> list all the MIME types, let me know.

No, I don't need that, it is enough to know that image/png is also on the 
clipboard.

>> I think it is sensible to try to detect pure image contents on the
>> clipboard, however I don't think we should hard code any browser specific
>> stuff. What I could imagine is to inspect the text/HTML further: If
>> (after cleaning) the only tag in the body is , and if the clipboard
>> contains image contents as well, then ignore the HTML for pasting without
>> argument and prefer the image.
> 
> I like this idea. Where should this be done? The place where Qt is
> used to get the text is inGuiClipboard::getAsText at
> 
> case PlainTextType:
> str = qApp->clipboard()->text(QClipboard::Clipboard)
> .normalized(QString::NormalizationForm_C);
> 
> I guess the idea is we should never get to this point?

I think it needs to be done in two places: In GuiClipboard and in 
Text::dispatch() (or a new service routine called from there). The reason 
for this is that I don't want to change the behaviour of the special pasting 
methods: If somebody explicitly says he wants to paste text, then he should 
get the image link.
Attached is a sketch of the idea, without the actual HTML parsing part to 
find out whether the whole body conatins only an  tag, and without any 
documentation.


Georgdiff --git a/src/Text3.cpp b/src/Text3.cpp
index b78ba15..cd360bf 100644
--- a/src/Text3.cpp
+++ b/src/Text3.cpp
@@ -1272,15 +1272,20 @@ void Text::dispatch(Cursor & cur, FuncRequest & cmd)
 		// without argument?
 		string const arg = to_utf8(cmd.argument());
 		if (arg.empty()) {
-			bool tryGraphics = true;
+			bool tryGraphics = false;
 			if (theClipboard().isInternal())
 pasteFromStack(cur, bv->buffer().errorList("Paste"), 0);
 			else if (theClipboard().hasTextContents()) {
-if (pasteClipboardText(cur, bv->buffer().errorList("Paste"),
-   true, Clipboard::AnyTextType))
-	tryGraphics = false;
-			}
-			if (tryGraphics && theClipboard().hasGraphicsContents())
+bool const hasGraphics = theClipboard().hasGraphicsContents();
+if (hasGraphics &&
+!theClipboard().hasTextContents(Clipboard::AnyTextTypeNotOnlyImageLink))
+	tryGraphics = true;
+else if (!pasteClipboardText(cur, bv->buffer().errorList("Paste"),
+ true, Clipboard::AnyTextType))
+	tryGraphics = hasGraphics;
+			} else
+tryGraphics = theClipboard().hasGraphicsContents();
+			if (tryGraphics)
 pasteClipboardGraphics(cur, bv->buffer().errorList("Paste"));
 		} else if (isStrUnsignedInt(arg)) {
 			// we have a numerical argument
diff --git a/src/frontends/Clipboard.h b/src/frontends/Clipboard.h
index faf4e0a..0dafed6 100644
--- a/src/frontends/Clipboard.h
+++ b/src/frontends/Clipboard.h
@@ -48,6 +48,7 @@ public:
 		HtmlTextType,
 		LaTeXTextType,
 		LyXTextType,
+		AnyTextTypeNotOnlyImageLink,
 	};
 
 	/**
diff --git a/src/frontends/qt4/GuiClipboard.cpp b/src/frontends/qt4/GuiClipboard.cpp
index d96dd26..2cbd2d7 100644
--- a/src/frontends/qt4/GuiClipboard.cpp
+++ b/src/frontends/qt4/GuiClipboard.cpp
@@ -378,6 +378,7 @@ docstring const GuiClipboard::getAsText(TextType type) const
 	case AnyTextType:
 	case LyXOrPlainTextType:
 	case PlainTextType:
+	case AnyTextTypeNotOnlyImageLink:
 		str = qApp->clipboard()->text(QClipboard::Clipboard)
 .normalized(QString::NormalizationForm_C);
 		break;
@@ -465,6 +466,8 @@ bool GuiClipboard::hasTextContents(Clipboard::TextType type) const
 	case LaTeXTextType:
 		return cache_.hasFormat(latexMimeType()) ||
 		   cache_.hasFormat(texMimeType());
+	case AnyTextTypeNotOnlyImageLink:
+		return has_text_contents_not_only_image_link_;
 	}
 	// shut up compiler
 	return false;
@@ -568,6 +571,15 @@ void GuiClipboard::on_dataChanged()
 
 	has_text_contents_ = hasTextContents();
 	has_graphics_contents_ = hasGraphicsContents();
+	has_text_contents_not_only_image_link_ = hasTextContents(LyXOrPlainTextType) ||
+		hasTextContents(LaTeXTextType);
+	if (!has_text_contents_not_only_image_link_ && hasTextContents(HtmlTextType)) {
+		QString subtype = "html";
+		QString str = qApp->clipboard()->text(subtype, QClipboard::Clipboard)
+.normalized(QString::NormalizationForm_C);
+		str = tidyHtml(str);
+		// Now use some XML parser to find out whether  contains more than 
+	}
 }
 
 
diff --git a/src/frontends/qt4/GuiClipboard.h b/src/frontends/qt4/GuiClipboard.h
index 392f059..b0dff0c 100644
--- a/src/frontends/qt4/GuiClipboard.h
+++ b/src/frontends/qt4/GuiClipboard.h
@@ -87,6 +87,7 @@ private 

Re: Lyx<-->Word conversion and images: suggestions sought

[Georg Baum]

stefano franchi wrote:

> On Fri, May 30, 2014 at 3:42 PM, Georg Baum
>  wrote:
> 
> Sorry, I didn't explain the problem correctly. Of course, the internal
> structure of the ODT file should not matter. But tex4ht, instead, tries
> (and fails) to recreate the folder structure *inside* the odt zipped
> archive. That's the (known) bug.
> 
> Example: Say I have a tex file test.tex in folder "Folder" which contains
> a
> png  image picture.png in the same folder. tex4ht will create ODT file
> containing a subfolder called "Pictures" and put image.png in it, then
> insert the correct reference in the content.xml file, and everything works
> fine.
> 
> But now say the image is in Folder/Figures/picture.png. Tex4ht will create
> a folder called Pictures/Figures inside the ODT archive, and will screw up
> the reference to it in the content.xml file. What it should do is to act
> similarly to the previous case: copy the image to the Pictures folder an
> *not* try to recreate the folder structure.

Ah, OK. I guessed what the problem of tex4ht was, but I misunderstood the 
wanted behaviour.

> Your comments make me think  we should perhaps look at the problem a bit
> differently. The two basic scenarios for Word export are:
> 
> 1. cooperation with word user (roundtrip)
> 
> 2. interaction with publishers who only accept .doc format
> 
> 
> Given the limitations of odt's graphic formats, we may assume that the
> images contained *in* the ODT file are really just draft/placeholders and
> the real (higher def, vector, etc.) images are stored separately and will
> either be sent to the publisher separately (scenario 2) or dealt
> appropriately by LaTeX for final pdf production (scenario 1).
> 
> Under this assumption, we are free to delegate both image conversion and
> directory flattening to LyX and we only need to make sure the ODT file has
> the correct metadata to recreate the image reference as originally stored
> in the LyX file.
> And we can avoid storing the original image *in* the ODT file (if that's
> possible, I don't know).

Sounds sensible. However, it is hard to believe that the odt format does not 
support at least one vector graphics and one bitmap graphics format. I would 
guess that it is possible (with some effort) to store good quality images 
inside the .odt (but this is rather a long term thought, nothing for now).


Georg

Re: Can no longer paste PNG's automatically on trunk (intended?)

[Georg Baum]

Scott Kostyshak wrote:

> Hi Georg,
> 
> This has come back for me with Qt 5 (5.2.1 on Ubuntu 14.04). If I go
> to www.google.com in Chromium and right-click on the Google icon and
> select "Copy image" and then do a normal paste in LyX, I get the
> following:
> %00G%00o%00o%00g%00l%00e
> If I do the same in Firefox, it works as I expect and tries to paste the
> image. 'paste png' works after copying in Chromium.
> The difference appears to be that after copying in Chromium I have the
> following MIME types:
> 
> text/html
>  src="https://www.google.com/images/srpr/logo11w.png"; alt="Google"/>
> 
> text/uri-list
> https://www.google.com/images/srpr/logo11w.png
> 
> where with Firefox I also have text/html but with a content of:
>  src="https://www.google.com/images/srpr/logo11w.png";
> style="padding-top:112px" onload="window.lol&&lol()"
> height="95" width="269">
> 
> If I remove the text/html MIME type, LyX then pastes
> https://www.google.com/images/srpr/logo11w.png
> which is the contents of text/uri-list
> If I remove text/uri-list then the picture is pasted.

Are these MIME types the only ones? I would think that image/png exists as 
well?

> From what I understand, LyX asks Qt to convert HTML to a QString in
> tidyHtml(). If that string is empty, then it moves to a different MIME
> type. Perhaps the HTML conversion has changed in Qt 5 versus Qt 4?
> I'm not sure how the treatment of text/uri-list has changed.

The logic is in Text::dispatch(): It looks if the clipboard has text 
contents, and if that is true, pastes text, otherwise it pastes graphics.

> Note that this isn't annoying to me personally. But is this something
> we want to try to fix? Or do you think that trying to make this sort
> of paste work with all browsers and all the ways that MIME types are
> set is an impossible goal?

I think it is sensible to try to detect pure image contents on the 
clipboard, however I don't think we should hard code any browser specific 
stuff. What I could imagine is to inspect the text/HTML further: If (after 
cleaning) the only tag in the body is , and if the clipboard contains 
image contents as well, then ignore the HTML for pasting without argument 
and prefer the image.


Georg

Re: Lyx<-->Word conversion and images: suggestions sought

[Georg Baum]

stefano franchi wrote:

> Dear all:
> 
> Prannoy just hit upon a problem in text4ht conversion of images to odt
> format.
> This is what text4ht  should do:
> 
> 1. Read the correct reference from the tex file
> 2. If necessary, covert the image to a suitable (bitmap) format for
> inclusion in the odt file

The image conversion could also be delegated to LyX: If LyX would know that 
a LaTeX export is in reality for odt production, it could use its own 
converter machinery and provide both the file needed for tex4ht and for 
inclusion in odt. The advantage would be that the conversion would always 
start from the original image, which can result in higher quality or no 
conversion at all (e.g. if the original image is suitable for odt but not 
tex).

> 3. copy the converted image to a proper subfolder in the odt file (which
> really is a zipped archive, as you know).
> 
> The system works fine if the images are in the same directory/folder as
> the .tex source file, but tex4ht gets its references screwed up otherwise.
> It copies the images to the right "internal" folder (inside the odt file),
> but the xml code is incorrect and libreoffice cannot find the images when
> you open the file.

Why is it important to have the same directory structure inside the .odt as 
the user uses for LyX outside? This structure is internal, the user never 
sees it. Therefore I'd simply use a flat structure internally, and no tex4ht 
changes are required.

> Rather than messing up with with tex4ht code, it seems to me the easier
> workaround is to copy all the images and the tex source file to a
> temporary folder and do the conversion there. This is a similar approach
> to what LyX itself does before compiling LaTeX, in fact, and it is also
> what the native HTML converter does, I believe.

If you execute the converter as part of the standard conversion chain in 
LyX, then LyX will automatically flatten the directory structure, and all 
included files are in the same directory as the .tex file itself.

> Issues on which feedback is welcome:
> 
> 0. General thoughts about the approach (copying images vs. fixing tex4ht
> processing)
> 
> 1. My thinking is that it may be relatively easy to do the copying by
> operating on the LyX source code, i.e. via a python script scanning the
> Graphics inset. That would tie the tool to the file format, though.

I would avoid that if possible, since this introduces ambiguity: Suddenly 
you read contents both from .tex and .lyx files. How do you ensure that 
these two files are consistent?. If it is really too difficult to fix 
tex4ht, then I'd rather scan the .tex file (similar to 
lib/scripts/tex_copy.py) instead.

> 2. For the roundtrip conversion, we also need to keep the original
> filename references and store them somewhere in the odt file (in an
> annotation field). Are there any platform-related issues on filenames we
> should be aware of (encoding, folder delimiters, etc)?

There are several (http://wiki.lyx.org/LaTeX/FilesWithSpecialChars might 
help). What would you do with absolute paths? It may be impossible to 
restore them. Folder delimiters are easy, you can use forward slashes for 
internal storage, just as in LyX. The encoding is always the one which is 
active in the .tex file at the place where the filename occurs. Also keep in 
mind that for master/child documents relative paths are always relative to 
the master in .tex files.

Preserving the paths is only important for roundtrip. For one-way export it 
does not matter. To me it does not look like the most important problem, and 
I'd postpone it. If the image conversion works in general (for files in the 
same directory as the main .tex file), then one could think which additional 
metadata would be needed to restore them to the original location for 
roundtrip. This might not only be the file name, it might also be the 
complete image in the original format.


Georg

Re: Lyx<-->Word conversion and images: suggestions sought

[Georg Baum]

Cyrille Artho wrote:

> Dear all,
> My thoughts:
> 
> I think it is not too uncommon to have two images with the same name in
> different directories, such as pics/old/arch.fig and pics/new/arch.fig.
> 
> Obviously in such cases the renaming scheme needs to take this into
> account, by converting path separators ("/") to another character.
> However, that in turn means that that "other character" should not appear
> in the file name, so a means of "escaping" it would be needed as well.
> 
> For example: Path separators get converted to "_-"; "_" get converted to
> "__". In that case, a reverse substitution is always possible.
> 
> In short, the solution is workable but has its drawbacks.

This assumes that the original file name is not stored as additional 
metadata (otherwise no bidirectional mapping is needed). This is elegant, 
but may lead to very long file names inside the archive. I am not sure which 
alternative is better.

> If it's possible to fix tex4ht instead, that would be better as tex4ht
> users would also benefit. I agree that the code is not so easy to
> read/maintain, but this may be a good starting point.
> 
> A full "flattening" of path separators has the advantage that we don't
> have to worry about the platform the resulting document is used on. In
> other words, if we export a nested path name, then the target computer has
> to understand the path delimiter in the same way as the source computer. I
> guess nowadays any computer understands "/" so maybe this is not an issue
> with .odt. If "/" can indeed by used for .odt path names even on Windows,
> then keeping the original path names may be the more elegant solution,
> even if fixing tex4ht may be a bit difficult.

The delimiter is not the problem. Even if '/' would not work on windows (it 
does in 99% of the cases), the converter could easily convert it to '\' when 
converting from .odt to .lyx on windows. Unfortunately you need to consider 
the platform even for flat names (see the wiki page in my other message) A 
colon ':' or a double quote '"' is not allowed in windows filenames, but 
works fine on unix.


Georg

Re: [LyX/master] Add layout tag that determines if/when a paragraph can be indented.

[Georg Baum]

Enrico Forestieri wrote:

> They are fine. The only cleanup would be removing the entries that
> got doubled after replacing the CopyStyle entry and also removing
> the now orphaned comment in seminar.layout just before the removed
> "Style EndOfSlide" definition. I can do that after the update, if
> you prefer.

That would be nice, I would have to think more about it if I had to do it.


Georg

Re: LyX 2.0.8.1???

[Georg Baum]

Richard Heck wrote:

> 
> Where are we after Enrico's latest?
> 
> I suppose if need be I can always release a 2.0.8.2.

Looks good. The recent developments made me think about lyx2lyx tests again, 
and I'll try to come up with a proposal. After we have that, we can think 
about further beamer fixes, but without automated tests I personally would 
not dare to do more changes to the code. However, this will take some more 
weeks, and this is too late for 2.0.8.1.


Georg

Re: [LyX/master] Add layout tag that determines if/when a paragraph can be indented.

[Georg Baum]

Juergen Spitzmueller wrote:

> commit 20bcaec061193a3984d42040ee65d2723872ecf4
> Author: Juergen Spitzmueller 
> Date:   Thu May 29 14:10:32 2014 +0200
> 
> Add layout tag that determines if/when a paragraph can be indented.
> 
> Fixes: #7327, #7458, #8670

Nice work!

It looks like the layout file format number update is missing. If I update 
the number in layout2layout.py and TextClass.cpp, and then run 
development/tools/updatelayouts.sh, I get more changes than expected: See 
attachment, where I stripped all changes which consisted only of the updated 
file format number.

Enrico, could you please have a look the separator changes and adapt those 
files? Afterwards, we could then cleanly do the 50->51 update.


Georg
diff --git a/lib/layouts/beamer.layout b/lib/layouts/beamer.layout
index b02f162..0e5c3ca 100644
--- a/lib/layouts/beamer.layout
+++ b/lib/layouts/beamer.layout
@@ -6,7 +6,7 @@
 #   Richard Heck , Martin Vermeer  and probably others.
 
 
-Format 49
+Format 51
 
 #
 # GLOBAL SETTINGS
@@ -1243,28 +1243,6 @@ End
 # MISC.
 #
 
-Style Separator
-  Category MainText
-  LatexTypeParagraph
-  LatexNamedummy
-  ParIndentMM
-  ParSkip  0.4
-  AlignLeft
-  AlignPossibleBlock, Left, Right, Center
-  Margin   First_Dynamic
-  TopSep   0
-  BottomSep0
-  ParSep   0
-  LabelTypeStatic
-  LabelBottomSep   0
-  LabelString  "___"
-  KeepEmpty1
-  LabelFont 
-Series Medium
-Size   Normal
-Color  latex
-  EndFont
-End
  
 Style LyX-Code
   Category MainText
diff --git a/lib/layouts/g-brief2.layout b/lib/layouts/g-brief2.layout
index 6eaa9fa..3f3869d 100644
--- a/lib/layouts/g-brief2.layout
+++ b/lib/layouts/g-brief2.layout
@@ -6,7 +6,7 @@
 #  Thomas Hartkens 
 
 # Input general definitions
-Format 49
+Format 51
 Input stdfloats.inc
 Input stdcounters.inc
 Input stdinsets.inc
@@ -1018,7 +1018,6 @@ End
 Input stdlists.inc
 Input stdlayouts.inc
 NoStyle			Verse
-NoStyle			--Separator--
 
 # Input lyxmacros.inc
 
diff --git a/lib/layouts/moderncv.layout b/lib/layouts/moderncv.layout
index 1d204e8..4f613e1 100644
--- a/lib/layouts/moderncv.layout
+++ b/lib/layouts/moderncv.layout
@@ -6,7 +6,7 @@
 
 
 # General textclass parameters
-Format 49
+Format 51
 	Columns		1
 	Sides		1
 	SecNumDepth	-1
@@ -484,23 +484,6 @@ Style Bibliography
 	EndFont
 End
 
-Style --Separator--
-	KeepEmpty	1
-	Margin		Dynamic
-	LatexType	Paragraph
-	LatexName	dummy
-	ParIndent	MM
-	Align		Block
-	LabelType	Static
-	LabelString	"--- Separate Environment ---"
-	LabelFont
-	  Family	Roman
-	  Series	Medium
-	  Size		Normal
-	  Color		Blue
-	EndFont
-	HTMLLabel	NONE
-End
 
 Style Recipient
 	Margin		Dynamic
diff --git a/lib/layouts/seminar.layout b/lib/layouts/seminar.layout
index 6c86edc..7d16411 100644
--- a/lib/layouts/seminar.layout
+++ b/lib/layouts/seminar.layout
@@ -11,7 +11,7 @@
 #   1.4 2008-10-08 Günter Milde (use --Separator-- "look")
 #   1.5 2011-06-09 Günter Milde (Use Flex insets)
 
-Format 49
+Format 51
 Sides	1
 Columns	1
 
@@ -84,7 +84,22 @@ End
 # It is recommended to use the custom insets instead of paragraph styles.
 
 Style LandscapeSlide
-	CopyStyle		--Separator-- 
+	Category  MainText
+	KeepEmpty 1
+	MarginDynamic
+	LatexType Paragraph
+	LatexName dummy
+	ParIndent MM
+	Align Block
+	LabelType Static
+	LabelString   "--- Separate Environment ---"
+	LabelFont
+	  Family  Roman
+	  Series  Medium
+	  SizeNormal
+	  Color   Blue
+	EndFont
+	HTMLLabel NONE
 	LatexType		Environment
 	LatexName		slide
 	NextNoIndent		1
@@ -111,12 +126,24 @@ End
 # EndOfSlide was a dummy style whose main purpose is to separate subsequent
 # Slide environments. Nowadays lyx has the special --Separator-- style as
 # workaround:
-Style EndOfSlide
-	ObsoletedBy		--Separator--
-End
 
 Style ListOfSlides
-	CopyStyle		--Separator--
+	Category  MainText
+	KeepEmpty 1
+	MarginDynamic
+	LatexType Paragraph
+	LatexName dummy
+	ParIndent MM
+	Align Block
+	LabelType Static
+	LabelString   "--- Separate Environment ---"
+	LabelFont
+	  Family  Roman
+	  Series  Medium
+	  SizeNormal
+	  Color   Blue
+	EndFont
+	HTMLLabel NONE
 	LatexType		Command
 	LatexName		listofslides
 	TopSep			0.5
diff --git a/lib/layouts/stdlayouts.inc b/lib/layouts/stdlayouts.inc
index 73d928d..38cb32e 100644
--- a/lib/layouts/stdlayouts.inc
+++ b/lib/layouts/stdlayouts.inc
@@ -7,7 +7,7 @@
 # quotations and such.
 
 
-Format 49
+Format 51
 
 Style Quotation
 	Category  MainText
@@ -96,21 +96,3 @@ Style Verbatim
 End
 
 
-Style --Separator--
-	Category  

Re: LyX 2.0.8 Finally Read?

[Georg Baum]

Enrico Forestieri wrote:

> However, note that, after your patch, if a line ends with '\r', it
> is not stripped anymore. I think this may happen on Mac.

Thanks for the hint, I was aware of \r as line end but then forgot to test 
it. AFAIK, this is only used on pre-OS X macs, so not too important, but I 
fixed in nevertheless.


Georg

Re: Build bot on travis

[Georg Baum]

Peter Kümmel wrote:

> On 26.05.2014 20:57, Georg Baum wrote:
>>
>> Very nice! Especially the MinGW part is very interesting. Do you have any
>> plans to put this into the main repo?
> 
> No, and I've lost commit rights with the switch to git.
> But feel free to chery-pick from the build-bot-* branches.

This is a pity. Unfortunately my git skills are not good enough to chery-
pick something from a different repo. Each time I try to do something non-
trivial with git I fail to find the right --pretty-please --don't-assume-
what-i-want --do-what-i-typed options to make it actually do what I want, 
and end up feeling like the creators of http://git-man-page-
generator.lokaltog.net/.


Georg

Re: LyX 2.0.8 Finally Read?

[Georg Baum]

Enrico Forestieri wrote:

> On Tue, May 27, 2014 at 10:35:15PM +0200, Georg Baum wrote:
>> 
>> With this document I don't see an \end_layout problem, but a broken
>> \end_document if you run it htough lyx2lyx: It becomes \end_documen, and
>> the last line end is removed as well. This happens even for the no-op
>> conversion to format 474.
> 
> I actually don't see this. Looking at the converted file, I rather see
> that an EndFrame layout gets wrongly nested as follows:

It seems that the \end_document problem shadowed the real one. With your fix 
I can now convert the document correctly as well.

>> Something is really fishy here, but I don't understand what is wrong. Why
>> is the document corrupted even if you tell lyx2lyx to convert it to the
>> version it already has? This should not have anything to do with beamer,
>> since the beamer routines should not be called in this case.
> 
> As already said, I cannot reproduce this.

I found the problem and fixed it at c75c6e44. For some reason the file which 
I saved from the attachment did not have a trailing newline (maybe an off-
by-one error somewhere when the file length is determined;-), but lyx2lyx 
assumed that all lines end with a newline. This assumption is not needed, 
and I removed it.


Georg

Re: LyX 2.0.8 Finally Read?

[Georg Baum]

Pavel Sanda wrote:

> Georg Baum wrote:
>> release notes that there are known issues with complex beamer documents.
> 
> I don't know whether you consider this document complex, but I still see
> the end_layout issue, see attachment.

With this document I don't see an \end_layout problem, but a broken 
\end_document if you run it htough lyx2lyx: It becomes \end_documen, and the 
last line end is removed as well. This happens even for the no-op conversion 
to format 474.

Something is really fishy here, but I don't understand what is wrong. Why is 
the document corrupted even if you tell lyx2lyx to convert it to the version 
it already has? This should not have anything to do with beamer, since the 
beamer routines should not be called in this case.

Anyway, since nobody has time to have a deep look and set up a test suite, I 
still think it is a good idea to do the final 2.0 release, and I also think 
that the current warning is fine.


Georg

Re: file format question

[Georg Baum]

Richard Heck wrote:

> On 05/26/2014 05:02 PM, José Matos wrote:
>>
>> I favour the second option for exactly the same arguments you have
>> used. It is also reasonable to assume that people that relied on the
>> "bug"/lyx current behaviour are capable to workaround the problems.
>>
> 
> Same here.

I hoped for these answers :-) This is implemented now.


Georg

file format question

[Georg Baum]

Hi,

I am currently cleaning up my stash pile and finishing native support for 
\smash[t] and \smash[b] (this came from bug 8967) and \notag (same as 
\nonumber, but uses amsmath, this is helpful if you want to import AMS 
example documents with tex2lyx).

These commands cause now amsmath to be automatically loaded, and before they 
did not. This could in theory cause LaTeX erros, if a user used an own macro 
with a name used by amsmath. Therefore, we could say that this is a file 
format change, and disable amsmath in lyx2lyx if it was previously auto, one 
of the new commands is used and no other amsmath command is used.

Alternatively, we could consider the current status as a bug: Since we have 
the automatic amsmath loading already we could say that it should be 
complete, consider the current behaviour as a bug and no file format change 
is needed (this is different to the case where a new package is 
automatically loaded: In that case it is always a file format change).

The disadvantage of the first option is that the non-loading of amsmath is  
set for the future of this document which could cause similar issues as in 
bug 9069. The disadvantage of the second option is that it could create 
uncompilable documents.

What do you think? We cannot get a 100% correct solution, and I slightly 
tend towards the second option, since IMHO amsmath is needed for any serious 
math document anyway.


Georg

Re: LyX 2.0.8 Finally Read?

[Georg Baum]

Jürgen Spitzmüller wrote:

> 2014-05-23 14:17 GMT+02:00 Richard Heck :
> 
>>
>> What's the view about the status of lyx2lyx for 2.0.8? Did the last round
>> of changes take care of the issues we'd discovered, or is there more work
>> to do?
>>
> 
> Beamer conversion is probably not yet completely sane. The problem here is
> that every fix could break things elsewhere. A good test set (and multiple
> testers) would be needed.

Indeed. I believe that further development of the beamer conversion is not 
possible without such a test set, and I don't see that coming very soon.

Therefore I'd suggest to release 2.0.8.1 now and put a note somewhere into 
the announce,ent and/or release notes that there are known issues with 
complex beamer documents. Apart from beamer I think the conversion is OK.


Georg

Re: Build bot on travis

[Georg Baum]

Peter Kuemmel wrote:

> Finally I had the time to setup an automatic build for LyX:
> 
> http://syntheticpp.github.io/LyX-bleeding-edge
> 
> Runs since several weeks now, so I assume it is quite stable.
> 
> It uses MinGW (which is new at all) on Linux.
> It generates zips for Windows, master and 2.1.x.
> The spell checker stuff is missing due to mingw/3rdparty libs.
> 
> At least for Windows users this is a way to get an actual build,
> and it could be used by non-hackers to verify a bugfix.

Very nice! Especially the MinGW part is very interesting. Do you have any 
plans to put this into the main repo?


Georg

Re: [LyX/master] Fix crash when right-clicking into an inset with more paragraphs than the main text.

[Georg Baum]

Georg Baum wrote:

> I propose the attached fix. Of course this will break if we ever have real
> text insets for \mbox etc, but I guess there are many more places like
> that.

Attached is a better patch which will not break.


Georgdiff --git a/src/frontends/qt4/Menus.cpp b/src/frontends/qt4/Menus.cpp
index 7911c16..c68d02b 100644
--- a/src/frontends/qt4/Menus.cpp
+++ b/src/frontends/qt4/Menus.cpp
@@ -1665,9 +1665,13 @@ void MenuDefinition::expandEnvironmentSeparators(BufferView const * bv)
 {
 	if (!bv)
 		return;
+	InsetText const * text = bv->cursor().text();
+	// no paragraphs and no separators exist in math
+	if (!text)
+		return;
 
 	pit_type pit = bv->cursor().selBegin().pit();
-	Paragraph const & par = bv->cursor().text()->getPar(pit);
+	Paragraph const & par = text->getPar(pit);
 	docstring const curlayout = par.layout().name();
 	docstring outerlayout;
 	depth_type current_depth = par.params().depth();
@@ -1677,7 +1681,7 @@ void MenuDefinition::expandEnvironmentSeparators(BufferView const * bv)
 		if (pit == 0 || cpar.params().depth() == 0)
 			break;
 		--pit;
-		cpar = bv->cursor().text()->getPar(pit);
+		cpar = text->getPar(pit);
 		if (cpar.params().depth() < current_depth
 		&& cpar.layout().isEnvironment()) {
 outerlayout = cpar.layout().name();

Re: [LyX/master] Fix crash when right-clicking into an inset with more paragraphs than the main text.

[Georg Baum]

Juergen Spitzmueller wrote:

> commit c5753af50d8568b7f36797ec8bd67586572fce13
> Author: Juergen Spitzmueller 
> Date:   Sun May 18 18:03:06 2014 +0200
> 
> Fix crash when right-clicking into an inset with more paragraphs than
> the main text.
> 
> Fixes: #9123.
> 
> diff --git a/src/frontends/qt4/Menus.cpp b/src/frontends/qt4/Menus.cpp
> index 8e9fd54..7911c16 100644
> --- a/src/frontends/qt4/Menus.cpp
> +++ b/src/frontends/qt4/Menus.cpp
> @@ -1667,7 +1667,7 @@ void
> MenuDefinition::expandEnvironmentSeparators(BufferView const * bv)
>  return;
>  
>  pit_type pit = bv->cursor().selBegin().pit();
> - Paragraph const & par = bv->buffer().text().getPar(pit);
> + Paragraph const & par = bv->cursor().text()->getPar(pit);
>  docstring const curlayout = par.layout().name();
>  docstring outerlayout;
>  depth_type current_depth = par.params().depth();

This does still crash:
- Create new empty document
- Create empty math formula by C-m
- Click edit menu, the cursor should still be in the math formula => crash, 
because par is invalid (there are no paragraphs in mathed).

I propose the attached fix. Of course this will break if we ever have real 
text insets for \mbox etc, but I guess there are many more places like that. 
OK for master and 2.1?


Georgdiff --git a/src/frontends/qt4/Menus.cpp b/src/frontends/qt4/Menus.cpp
index 7911c16..5cd316f 100644
--- a/src/frontends/qt4/Menus.cpp
+++ b/src/frontends/qt4/Menus.cpp
@@ -1665,6 +1665,9 @@ void MenuDefinition::expandEnvironmentSeparators(BufferView const * bv)
 {
 	if (!bv)
 		return;
+	// no paragrapghs and no separators exist in math
+	if (bv->cursor().inMathed())
+		return;
 
 	pit_type pit = bv->cursor().selBegin().pit();
 	Paragraph const & par = bv->cursor().text()->getPar(pit);

Re: [LyX/master] Fix compilation after afc34c7a

[Georg Baum]

Enrico Forestieri wrote:

> commit 480f8d3115fd918472068ef344e7f9d56ede9eb1
> Author: Enrico Forestieri 
> Date:   Tue May 20 00:43:46 2014 +0200
> 
> Fix compilation after afc34c7a

Thanks. On linux, typeinfo gets pulled in by some other header.


Georg

Re: [PATCH] Fix a GCC warning: comparing signed vs. unsigned

[Georg Baum]

Am Dienstag, 20. Mai 2014 schrieb Cyrille Artho:
> > It is the correct type on linux as well. Therefore, the approach
> > suggested by Cyrille is IMHO the best one: Change the type of nRead
> > to ssize_t (note the two s), and change the if-condition to
> > 
> > if (static_cast(nRead) < buf.size() - 1) {
> > 
> > This is 100% safe: We know that nRead > 0 at the time the cast occurs,
> > and ssize_t has the same number of bits as size_t, and buf.size()
> > returns size_t, and buf.size() is never 0. The static_cast is better
> > than a C-style cast, since you can search for it, and it expresses
> > what is wanted (C-style casts could also be used to cast constness
> > away etc).
> 
> Georg made a small type: The cast should be
> 
>  > if (static_cast(nRead) < buf.size() - 1) {
> 
> as mentioned earlier (two s).

No, there is no typo, I meant size_t with one s. The reason for that is to 
avoid the signed vs. unsigned comparison warning, since ssize_t is signed, 
and the expression on the right hand side is unsigned. If the cast to 
ssize_t was correct then no cast would be needed at all, since nRead is 
already of type ssize_t.

> In general, though, I think it's better to
> adapt the variable type such that type casts are not necessary. In this
> case, "nRead" would be changed to ssize_t. Without looking at all the
> code, I'm not sure if this is the best solution here (sometimes
> changing one variable type requires many subsequent changes).

Yes, the type of nRead should be changed to ssize_t. However, a cast in the 
if-clause is still needed to avoid the signed vs. unsigned warning.


Georg