Re: Lyx2.1.0beta2 issues ?
Op 27-9-2013 23:53, Stephan Witt schreef: Regarding the details: * BufferEncoding.cpp is a manual copy of Encoding.cpp - that's why the wrong comment - that's why the name of the file - that's why the superfluous includes * CharInfo was a private struct - I've made a class to make it more functional but didn't want to make it public - It should be used inside Encoding(s) only - Yes, one can move it to a separate public class with header file - Yes, that's why the static methods in Encoding.cpp … Who is we? Well, I have no other choice than to fix this myself in the coming days and to prepare the release of beta 2 in the weekend, or to reject this branch for now and prepare the release of beta 2. I did it myself. Thank you, I've merged the branch now, after that I splitted and merged some commits in more logical building blocks. At least what seemed logical in my opinion. You will have to cherry-pick the commit "use com.apple.compilers.llvm.clang.1_0 for newer Xcode versions" yourself, because it didn't seem to be related to killing the TEX2LYX define. Vincent
Re: Lyx2.1.0beta2 issues ?
Am 25.09.2013 um 20:59 schrieb Vincent van Ravesteijn : > >>> - The commit about splitting the encoding stuff and moving helper functions >>> into a new class, should really have some explanation why this is needed, >>> and why we would need a new class for this. The construction around >>> CharInfo becomes rather strange. It is defined in Encoding.cpp, which >>> indicates that it should not be used outside Encoding.cpp. It is now used >>> outside this file in BufferEncoding.cpp. Because of this, there are a lot >>> of static functions added to the Encoding class just to allow >>> BufferEncoding to use CharInfo as an incomplete type. Moreover, most of >>> these functions aren't even used. Why is this moved into a new class ? >>> What was the problem that needed to be solved ? Can't we solve it with a >>> private header file ? >>> >>> - The file src/BufferEncoding.cpp starts with a comment that it is >>> Encoding.cpp >>> >>> - The class name is BufferEncodings so why don't we call the file >>> BufferEncodings.cpp ? (I know Encodings in Encoding.cpp.., but is that a >>> good reason?) >>> >>> - The following includes in BufferEncoding.cpp are not needed: >>> >>> #include "BufferList.h" >>> #include "Lexer.h" >>> #include "LyXRC.h" >>> #include "support/debug.h" >>> #include "support/gettext.h" >>> #include "support/FileName.h" >>> #include "support/textutils.h" >>> #include "support/unicode.h" >>> #include >>> #include >> The root problem is the fact Encoding(s) classes are used both by LyX and >> tex2lyx >> and these classes are using the Buffer stuff. >> >> This forces to link tex2lyx with the Buffer etc. making it another LyX binary >> or split the classes somehow. Originally it was done by a dirty hack - >> using the same code even without making a copy and compiling it twice with >> different >> compiler flags. That's not nice. Neither with C nor with C++. >> >> Nevertheless I'm standing on the "shoulder of giants" and want to change the >> code base >> only slightly if possible. I cannot use modern tools like Eclipse for >> refactoring the >> code. > > Well the "giants" have made a mess in certain areas of the code. And I > believe this is increasing the mess a bit. > >> Regarding the details: >> * BufferEncoding.cpp is a manual copy of Encoding.cpp >> - that's why the wrong comment >> - that's why the name of the file >> - that's why the superfluous includes >> * CharInfo was a private struct >> - I've made a class to make it more functional but didn't want to make it >> public >> - It should be used inside Encoding(s) only >> - Yes, one can move it to a separate public class with header file >> - Yes, that's why the static methods in Encoding.cpp … >> >> Who is we? > > Well, I have no other choice than to fix this myself in the coming days and > to prepare the release of beta 2 in the weekend, or to reject this branch for > now and prepare the release of beta 2. I did it myself. BTW, I think there was a bug in the parser… The removed lines - if (suffixIs(info.mathcommand, '}')) - info.flags |= CharInfoMathNoTermination; were above the assignment of the info.mathcommand member variable. Stephan
Re: Lyx2.1.0beta2 issues ?
- The commit about splitting the encoding stuff and moving helper functions into a new class, should really have some explanation why this is needed, and why we would need a new class for this. The construction around CharInfo becomes rather strange. It is defined in Encoding.cpp, which indicates that it should not be used outside Encoding.cpp. It is now used outside this file in BufferEncoding.cpp. Because of this, there are a lot of static functions added to the Encoding class just to allow BufferEncoding to use CharInfo as an incomplete type. Moreover, most of these functions aren't even used. Why is this moved into a new class ? What was the problem that needed to be solved ? Can't we solve it with a private header file ? - The file src/BufferEncoding.cpp starts with a comment that it is Encoding.cpp - The class name is BufferEncodings so why don't we call the file BufferEncodings.cpp ? (I know Encodings in Encoding.cpp.., but is that a good reason?) - The following includes in BufferEncoding.cpp are not needed: #include "BufferList.h" #include "Lexer.h" #include "LyXRC.h" #include "support/debug.h" #include "support/gettext.h" #include "support/FileName.h" #include "support/textutils.h" #include "support/unicode.h" #include #include The root problem is the fact Encoding(s) classes are used both by LyX and tex2lyx and these classes are using the Buffer stuff. This forces to link tex2lyx with the Buffer etc. making it another LyX binary or split the classes somehow. Originally it was done by a dirty hack - using the same code even without making a copy and compiling it twice with different compiler flags. That's not nice. Neither with C nor with C++. Nevertheless I'm standing on the "shoulder of giants" and want to change the code base only slightly if possible. I cannot use modern tools like Eclipse for refactoring the code. Well the "giants" have made a mess in certain areas of the code. And I believe this is increasing the mess a bit. Regarding the details: * BufferEncoding.cpp is a manual copy of Encoding.cpp - that's why the wrong comment - that's why the name of the file - that's why the superfluous includes * CharInfo was a private struct - I've made a class to make it more functional but didn't want to make it public - It should be used inside Encoding(s) only - Yes, one can move it to a separate public class with header file - Yes, that's why the static methods in Encoding.cpp … Who is we? Well, I have no other choice than to fix this myself in the coming days and to prepare the release of beta 2 in the weekend, or to reject this branch for now and prepare the release of beta 2. Vincent
Re: Lyx2.1.0beta2 issues ?
Am 10.09.2013 um 09:01 schrieb Vincent van Ravesteijn : > Op 6-9-2013 11:59, Jean-Marc Lasgouttes schreef: >> 28/08/2013 22:45, Vincent van Ravesteijn: >>> Hi all, >>> >>> I think it's time to start thinking about what is needed before we can >>> release beta2. >>> >>> Please reply if you know about any issues so that I won't be able to >>> overlook them. >> >> I would like to merge the branch features/kill-tex2lyx-define. It solves >> cleanly the problem with automake subdirs and is a long awaited cleanup IMO. >> The branch consists of small self-contained commits and is easy to review >> IMO. >> > > I had a look at the branch, and I have the following comments: > > Snip comments answered by JMarc already... > - The commit about splitting the encoding stuff and moving helper functions > into a new class, should really have some explanation why this is needed, and > why we would need a new class for this. The construction around CharInfo > becomes rather strange. It is defined in Encoding.cpp, which indicates that > it should not be used outside Encoding.cpp. It is now used outside this file > in BufferEncoding.cpp. Because of this, there are a lot of static functions > added to the Encoding class just to allow BufferEncoding to use CharInfo as > an incomplete type. Moreover, most of these functions aren't even used. Why > is this moved into a new class ? What was the problem that needed to be > solved ? Can't we solve it with a private header file ? > > - The file src/BufferEncoding.cpp starts with a comment that it is > Encoding.cpp > > - The class name is BufferEncodings so why don't we call the file > BufferEncodings.cpp ? (I know Encodings in Encoding.cpp.., but is that a good > reason?) > > - The following includes in BufferEncoding.cpp are not needed: > > #include "BufferList.h" > #include "Lexer.h" > #include "LyXRC.h" > #include "support/debug.h" > #include "support/gettext.h" > #include "support/FileName.h" > #include "support/textutils.h" > #include "support/unicode.h" > #include > #include The root problem is the fact Encoding(s) classes are used both by LyX and tex2lyx and these classes are using the Buffer stuff. This forces to link tex2lyx with the Buffer etc. making it another LyX binary or split the classes somehow. Originally it was done by a dirty hack - using the same code even without making a copy and compiling it twice with different compiler flags. That's not nice. Neither with C nor with C++. Nevertheless I'm standing on the "shoulder of giants" and want to change the code base only slightly if possible. I cannot use modern tools like Eclipse for refactoring the code. Regarding the details: * BufferEncoding.cpp is a manual copy of Encoding.cpp - that's why the wrong comment - that's why the name of the file - that's why the superfluous includes * CharInfo was a private struct - I've made a class to make it more functional but didn't want to make it public - It should be used inside Encoding(s) only - Yes, one can move it to a separate public class with header file - Yes, that's why the static methods in Encoding.cpp … Who is we? Stephan
Re: Lyx2.1.0beta2 issues ?
10/09/2013 13:40, Vincent van Ravesteijn: I'm in no position to tell you what you'll have to do. I'll rebase the branch when it is clear to me what Stephan exactly wanted to do with the Encodings stuff, and I might add your previous explanations to the logs. It's hardly any effort then. However, if you feel like you have anything to share with the world, Personally, although I agree that my commit logs should have been better, I do not think it is worth changing anything right now. Indeed a commit log at merge time would be nice. Concerning the encoding part (which was the hard work), I do not really know how much simpler it could have been. I agree that the BufferEncoding class is not needed (the code could have gone to buffer_funcs.cpp, for example), but I do not know much about the CharInfo stuff. JMarc
Re: Lyx2.1.0beta2 issues ?
On Tue, Sep 10, 2013 at 11:44 AM, Jean-Marc Lasgouttes wrote: > 10/09/2013 09:01, Vincent van Ravesteijn: > > I had a look at the branch, and I have the following comments: >> >> - In the commit logs, it is nowhere mentioned why the cleanups are >> necessary. >> > > > Indeed... But removing #ifdef's is always good, isn't it? > > I don't say it ain't good, I just would like to have an insight in what trouble the evil-#ifdefs caused... but I found it in on lyx-devel... > > - In commit ed55131e8, the "automake subdirs" is silently introduced. As >> this change has caused such a lot of work, it should really be a >> separate commit, and should explain that the other commits were needed >> to allow this. In this same commit, the TEX2LYX defines are removed. >> This should be a separate commit, or be merged with the work that was >> needed in removing the TEX2LYX defines. >> > > Yes, this should be the final commit. How can one split a commit in two? As the commits are already published and Stephan might also be working on them, it is safest not to change history now. If you want to do it anyway: git checkout features/kill-tex2lyx-defines git rebase -i master Mark the commit to be edited. When the rebase stops: git reset HEAD~1 Now, "git add" and "git commit" several times until all changes are committed again. git rebase --continue > - The change to src/tex2lyx/tex2lyx.cpp is unexpected in a commit that >> says it removes something from ModuleList.cpp. >> > > Most of my commits are build like that: a dummy implementation is created > to avoid an #ifdef in the code that needs the said function. I understand the idea, but I failed to see the relation between removing the #ifdef's in LyXModule and adding a dummy implementation for LaTeXFeatures::isAvailable(). > Do you want me to modify the commit logs? > I'm in no position to tell you what you'll have to do. I'll rebase the branch when it is clear to me what Stephan exactly wanted to do with the Encodings stuff, and I might add your previous explanations to the logs. It's hardly any effort then. However, if you feel like you have anything to share with the world, Vincent
Re: Lyx2.1.0beta2 issues ?
10/09/2013 09:01, Vincent van Ravesteijn: I had a look at the branch, and I have the following comments: - In the commit logs, it is nowhere mentioned why the cleanups are necessary. Indeed... But removing #ifdef's is always good, isn't it? - In commit ed55131e8, the "automake subdirs" is silently introduced. As this change has caused such a lot of work, it should really be a separate commit, and should explain that the other commits were needed to allow this. In this same commit, the TEX2LYX defines are removed. This should be a separate commit, or be merged with the work that was needed in removing the TEX2LYX defines. Yes, this should be the final commit. How can one split a commit in two? - The commit about splitting the encoding stuff and moving helper functions into a new class, should really have some explanation why this is needed, and why we would need a new class for this. [...] I let Stephan answer on those. - The change to src/tex2lyx/tex2lyx.cpp is unexpected in a commit that says it removes something from ModuleList.cpp. Most of my commits are build like that: a dummy implementation is created to avoid an #ifdef in the code that needs the said function. Do you want me to modify the commit logs? JMarc
Re: Lyx2.1.0beta2 issues ?
Op 6-9-2013 11:59, Jean-Marc Lasgouttes schreef: 28/08/2013 22:45, Vincent van Ravesteijn: Hi all, I think it's time to start thinking about what is needed before we can release beta2. Please reply if you know about any issues so that I won't be able to overlook them. I would like to merge the branch features/kill-tex2lyx-define. It solves cleanly the problem with automake subdirs and is a long awaited cleanup IMO. The branch consists of small self-contained commits and is easy to review IMO. I had a look at the branch, and I have the following comments: - In the commit logs, it is nowhere mentioned why the cleanups are necessary. - In commit ed55131e8, the "automake subdirs" is silently introduced. As this change has caused such a lot of work, it should really be a separate commit, and should explain that the other commits were needed to allow this. In this same commit, the TEX2LYX defines are removed. This should be a separate commit, or be merged with the work that was needed in removing the TEX2LYX defines. - The commit about splitting the encoding stuff and moving helper functions into a new class, should really have some explanation why this is needed, and why we would need a new class for this. The construction around CharInfo becomes rather strange. It is defined in Encoding.cpp, which indicates that it should not be used outside Encoding.cpp. It is now used outside this file in BufferEncoding.cpp. Because of this, there are a lot of static functions added to the Encoding class just to allow BufferEncoding to use CharInfo as an incomplete type. Moreover, most of these functions aren't even used. Why is this moved into a new class ? What was the problem that needed to be solved ? Can't we solve it with a private header file ? - The file src/BufferEncoding.cpp starts with a comment that it is Encoding.cpp - The class name is BufferEncodings so why don't we call the file BufferEncodings.cpp ? (I know Encodings in Encoding.cpp.., but is that a good reason?) - The following includes in BufferEncoding.cpp are not needed: #include "BufferList.h" #include "Lexer.h" #include "LyXRC.h" #include "support/debug.h" #include "support/gettext.h" #include "support/FileName.h" #include "support/textutils.h" #include "support/unicode.h" #include #include - The change to src/tex2lyx/tex2lyx.cpp is unexpected in a commit that says it removes something from ModuleList.cpp. Vincent
Re: Lyx2.1.0beta2 issues ?
Op 9-9-2013 15:50, Jean-Marc Lasgouttes schreef: 28/08/2013 22:45, Vincent van Ravesteijn: Hi all, I think it's time to start thinking about what is needed before we can release beta2. Please reply if you know about any issues so that I won't be able to overlook them. What is the status of the features/kill-tex2lyx-define branch? Can we merge it? JMarc I'm busy clearing up my todo-list. So I will have a look at it soon. Vincent
Re: Lyx2.1.0beta2 issues ?
28/08/2013 22:45, Vincent van Ravesteijn: Hi all, I think it's time to start thinking about what is needed before we can release beta2. Please reply if you know about any issues so that I won't be able to overlook them. What is the status of the features/kill-tex2lyx-define branch? Can we merge it? JMarc
Re: Lyx2.1.0beta2 issues ?
Op 9-9-2013 11:05, Jean-Marc Lasgouttes schreef: 06/09/2013 14:35, Vincent van Ravesteijn: Are you asking me? This is too much autotools for me to say something wise about it. All I can do is to ask people to test. If no-one reports a problem, it will be ok. Well, I pushed it for maximal exposure :) I will revert as needed. JMarc Good. Pavel tested it and didn't report a problem, so it must be ok. Vincent
Re: Lyx2.1.0beta2 issues ?
06/09/2013 14:35, Vincent van Ravesteijn: Are you asking me? This is too much autotools for me to say something wise about it. All I can do is to ask people to test. If no-one reports a problem, it will be ok. Well, I pushed it for maximal exposure :) I will revert as needed. JMarc
Re: Lyx2.1.0beta2 issues ?
Vincent van Ravesteijn wrote: > All I can do is to ask people to test. If no-one reports a problem, it will > be ok. I tested the patch under ubuntu & gentoo, both worked. P
Re: Lyx2.1.0beta2 issues ?
Pavel Sanda wrote: > Vincent van Ravesteijn wrote: > > Please reply if you know about any issues so that I won't be able to > > overlook them. > > #8783 #8830 is another build problem we have. Patch included there but I wonder it signals we miss some other code as well. Pavel
Re: Lyx2.1.0beta2 issues ?
On Fri, Sep 6, 2013 at 11:16 AM, Jean-Marc Lasgouttes wrote: > 06/09/2013 07:53, Pavel Sanda: > > Vincent van Ravesteijn wrote: >> >>> Please reply if you know about any issues so that I won't be able to >>> overlook them. >>> >> >> #8783 >> > > Here is a patch for this one. OK? > > JMarc > Are you asking me? This is too much autotools for me to say something wise about it. All I can do is to ask people to test. If no-one reports a problem, it will be ok. Vincent
Re: Lyx2.1.0beta2 issues ?
28/08/2013 22:45, Vincent van Ravesteijn: Hi all, I think it's time to start thinking about what is needed before we can release beta2. Please reply if you know about any issues so that I won't be able to overlook them. I would like to merge the branch features/kill-tex2lyx-define. It solves cleanly the problem with automake subdirs and is a long awaited cleanup IMO. The branch consists of small self-contained commits and is easy to review IMO. JMarc
Re: Lyx2.1.0beta2 issues ?
06/09/2013 07:53, Pavel Sanda:
Vincent van Ravesteijn wrote:
Please reply if you know about any issues so that I won't be able to
overlook them.
#8783
Here is a patch for this one. OK?
JMarc
>From d2a2cb11e0015f05da56dbe5d6e607d53d17 Mon Sep 17 00:00:00 2001
From: Jean-Marc Lasgouttes
Date: Fri, 6 Sep 2013 11:12:09 +0200
Subject: [PATCH] Fix compilation on Solaris 11.1 (bug #8783)
Make sure that the configure script only checks features using the C++ compiler.
Also get rid of our last C files, since they are not compiled nor distributed anyway.
---
configure.ac | 11 ++-
src/support/atexit.c | 18 --
src/support/strerror.c | 24
3 files changed, 6 insertions(+), 47 deletions(-)
delete mode 100644 src/support/atexit.c
delete mode 100644 src/support/strerror.c
diff --git a/configure.ac b/configure.ac
index b85c4c1..75e74ac 100644
--- a/configure.ac
+++ b/configure.ac
@@ -61,11 +61,9 @@ done
AM_PATH_PYTHON(2.4.0,, :)
AC_PROG_RANLIB
-### we need to know the byte order for unicode conversions
-AC_C_BIGENDIAN
-
### Check for a C++ compiler
LYX_PROG_CXX
+AC_LANG(C++)
### Objective-C compiler
AC_PROG_OBJC
@@ -97,6 +95,9 @@ AC_CHECK_LIB(gdi32, main)
LYX_USE_INCLUDED_BOOST
LYX_USE_INCLUDED_MYTHES
+### we need to know the byte order for unicode conversions
+AC_C_BIGENDIAN
+
# Needed for our char_type
AC_CHECK_SIZEOF(wchar_t)
@@ -181,7 +182,7 @@ AC_FUNC_SELECT_ARGTYPES
LYX_CHECK_SPELL_ENGINES
lyx_client_subdir=true
-AC_LANG_PUSH(C)
+dnl AC_LANG_PUSH(C)
dnl LIBS already contains some X extra libs that may interfere.
save_LIBS="$LIBS"
LIBS=
@@ -194,7 +195,7 @@ AC_CHECK_FUNCS(fcntl,
AC_SUBST(SOCKET_LIBS,$LIBS)
LIBS="$save_LIBS"
AM_CONDITIONAL(BUILD_CLIENT_SUBDIR, $lyx_client_subdir)
-AC_LANG_POP(C)
+dnl AC_LANG_POP(C)
lyx_win_res=false;
case ${host} in
diff --git a/src/support/atexit.c b/src/support/atexit.c
deleted file mode 100644
index 4331809..000
--- a/src/support/atexit.c
+++ /dev/null
@@ -1,18 +0,0 @@
-/**
- * \file atexit.c
- * Wrapper to implement ANSI C's atexit using SunOS's on_exit.
- *
- * This function is in the public domain. --Mike Stump.
- */
-
-#include
-
-#ifndef NEED_on_exit
-int atexit(void (*f)())
-{
- /* If the system doesn't provide a definition for atexit, use on_exit
- if the system provides that. */
- on_exit (f, 0);
- return 0;
-}
-#endif
diff --git a/src/support/strerror.c b/src/support/strerror.c
deleted file mode 100644
index b678dda..000
--- a/src/support/strerror.c
+++ /dev/null
@@ -1,24 +0,0 @@
-/*
- * provides strerror()
- * author Stephan Witt
- */
-
-#include
-
-/* $Id: strerror.c,v 1.3 2000/08/04 13:12:30 lasgouttes Exp $ */
-
-#if !defined(lint) && !defined(WITH_WARNINGS)
-static char vcid[] = "$Id: strerror.c,v 1.3 2000/08/04 13:12:30 lasgouttes Exp $";
-#endif /* lint */
-
-extern int sys_nerr ;
-extern char * sys_errlist [] ;
-
-char * strerror (int errnum)
-{
- static char * errtext = "unknown errno" ;
-
- if ( errnum < sys_nerr )
- return sys_errlist [errnum] ;
- return errtext ;
-}
--
1.7.0.4
Re: Lyx2.1.0beta2 issues ?
Vincent van Ravesteijn wrote: > Please reply if you know about any issues so that I won't be able to > overlook them. #8783
Re: Lyx2.1.0beta2 issues ?
Am 28.08.2013 um 22:45 schrieb Vincent van Ravesteijn : > Hi all, > > I think it's time to start thinking about what is needed before we can > release beta2. > > Please reply if you know about any issues so that I won't be able to overlook > them. > > Vincentl I'd like to migrate the lyx user directory contents automatically. This happens if enabled at configure time and if with-version-suffiy is used. On Mac this is very desirable - on Windows I don't know. The patch I've attached. Stephan 0007-configure-migrate.diff Description: Binary data
Re: Lyx2.1.0beta2 issues ?
Am Mittwoch, 28. August 2013 um 22:45:26, schrieb Vincent van Ravesteijn > Hi all, > > I think it's time to start thinking about what is needed before we can > release beta2. > > Please reply if you know about any issues so that I won't be able to > overlook them. > > Vincentl I'd like to move the man pages in cmake build from the lyx-subdirectory into independent place (default /usr/local/man) like autotools does. Kornel signature.asc Description: This is a digitally signed message part.
