Re: A Cygwin related patch
On Fri, Mar 24, 2006 at 02:04:53PM +0100, Georg Baum wrote:
No, it does not need posix style paths: It simply cannot cope with
backslashes for obvious reasons. AFAIK miktex can handle paths like
C:/temp/test.tex. Not a big difference, but not exactly posix either.
This path style is called mixed style in the cygwin documentation.
The test performed by the configure script about use_cygwin_path_fix is
exactly based on that. It consists in creating a latex file containing
something like \input{C:/temp/test.tex}, launching latex on the file,
and then looking for an error. The cygwin tetex gives an error, indeed,
whereas miktex does not.
If everything works: Fine, but from reading the code I suspect that there
might be problems when calling external programs or handling paths to the
filedialog. Of course we should not fix anything when we are not sure where
the problem lies.
As I said, cygwin can cope with both win and posix style paths, so a
problem is unlikely to occur but not impossible (see bug 2344). However,
I am getting more and more convinced that the way the Use Cygwin-style
paths checkbox is currently used is wrong. The configure script already
takes care of appropriately setting the use_cygwin_path_fix switch, and
the fact that such setting can be subverted through that checkbox, can
only be harmful to the casual user which has not clear what its
implications are.
I don't know if the idea of who introduced such checkbox was to let
the user decide what kind of path style LyX would use/show in its GUI,
but I think that its purpose should be exactly this one. Therefore it
should be made orthogonal to the use_cygwin_path_fix switch.
Building upon your clever intuition about the need of an os::latex_path
function, I devised the attached patch and I am currently testing it.
Until now everything works and the behavior seems more consistent to me.
Essentially, my idea is that when Use Cygwin-style paths is unchecked,
LyX/Cygwin should look and feel as a native windows application and the
files it produces should be completely interoperable with the native
LyX/Win version, as I think this is a good thing.
I still have to reach that goal. For the moment only the paths in
preferences are converted to the desired format.
Anyway, I am posting the patch as it is now, in order to get some
feedback.
--
Enrico
Index: src/lyxfunc.C
===
--- src/lyxfunc.C (revision 13496)
+++ src/lyxfunc.C (working copy)
@@ -1977,6 +1977,11 @@ void actOnUpdatedPrefs(LyXRC const lyx
namespace os = lyx::support::os;
os::cygwin_path_fix(lyxrc_new.cygwin_path_fix);
}
+ case LyXRC::RC_CYGWIN_STYLE_PATHS:
+ if (lyxrc_orig.cygwin_style_paths !=
lyxrc_new.cygwin_style_paths) {
+ namespace os = lyx::support::os;
+ os::cygwin_style_paths(lyxrc_new.cygwin_style_paths);
+ }
case LyXRC::RC_DATE_INSERT_FORMAT:
case LyXRC::RC_DEFAULT_LANGUAGE:
case LyXRC::RC_DEFAULT_PAPERSIZE:
Index: src/frontends/qt2/QPrefs.C
===
--- src/frontends/qt2/QPrefs.C (revision 13496)
+++ src/frontends/qt2/QPrefs.C (working copy)
@@ -148,6 +148,11 @@ string const internal_path(QString const
return lyx::support::os::internal_path(fromqstr(input));
}
+string const internal_path_list(QString const input)
+{
+ return lyx::support::os::internal_path_list(fromqstr(input));
+}
+
}
@@ -199,7 +204,7 @@ void QPrefs::apply()
#if defined(__CYGWIN__) || defined(__CYGWIN32__)
QPrefCygwinPathModule * cygwinmod(dialog_-cygwinpathModule);
- rc.cygwin_path_fix = cygwinmod-pathCB-isChecked();
+ rc.cygwin_style_paths = cygwinmod-pathCB-isChecked();
#endif
QPrefLatexModule * latexmod(dialog_-latexModule);
@@ -253,7 +258,7 @@ void QPrefs::apply()
rc.template_path = internal_path(pathsmod-templateDirED-text());
rc.backupdir_path = internal_path(pathsmod-backupDirED-text());
rc.tempdir_path = internal_path(pathsmod-tempDirED-text());
- rc.path_prefix = fromqstr(pathsmod-pathPrefixED-text());
+ rc.path_prefix = internal_path_list(pathsmod-pathPrefixED-text());
// FIXME: should be a checkbox only
rc.lyxpipes = internal_path(pathsmod-lyxserverDirED-text());
@@ -467,6 +472,11 @@ QString const external_path(string const
return toqstr(lyx::support::os::external_path(input));
}
+QString const external_path_list(string const input)
+{
+ return toqstr(lyx::support::os::external_path_list(input));
+}
+
}
@@ -536,7 +546,7 @@ void QPrefs::update_contents()
#if defined(__CYGWIN__) || defined(__CYGWIN32__)
QPrefCygwinPathModule * cygwinmod(dialog_-cygwinpathModule);
- cygwinmod-pathCB-setChecked(rc.cygwin_path_fix);
+
Re: A Cygwin related patch
On Fri, Mar 24, 2006 at 02:04:53PM +0100, Georg Baum wrote:
> No, it does not need posix style paths: It simply cannot cope with
> backslashes for obvious reasons. AFAIK miktex can handle paths like
> "C:/temp/test.tex". Not a big difference, but not exactly posix either.
This path style is called mixed style in the cygwin documentation.
The test performed by the configure script about use_cygwin_path_fix is
exactly based on that. It consists in creating a latex file containing
something like "\input{C:/temp/test.tex}", launching latex on the file,
and then looking for an error. The cygwin tetex gives an error, indeed,
whereas miktex does not.
> If everything works: Fine, but from reading the code I suspect that there
> might be problems when calling external programs or handling paths to the
> filedialog. Of course we should not fix anything when we are not sure where
> the problem lies.
As I said, cygwin can cope with both win and posix style paths, so a
problem is unlikely to occur but not impossible (see bug 2344). However,
I am getting more and more convinced that the way the "Use Cygwin-style
paths" checkbox is currently used is wrong. The configure script already
takes care of appropriately setting the use_cygwin_path_fix switch, and
the fact that such setting can be subverted through that checkbox, can
only be harmful to the casual user which has not clear what its
implications are.
I don't know if the idea of who introduced such checkbox was to let
the user decide what kind of path style LyX would use/show in its GUI,
but I think that its purpose should be exactly this one. Therefore it
should be made orthogonal to the use_cygwin_path_fix switch.
Building upon your clever intuition about the need of an os::latex_path
function, I devised the attached patch and I am currently testing it.
Until now everything works and the behavior seems more consistent to me.
Essentially, my idea is that when "Use Cygwin-style paths" is unchecked,
LyX/Cygwin should look and feel as a native windows application and the
files it produces should be completely interoperable with the native
LyX/Win version, as I think this is a good thing.
I still have to reach that goal. For the moment only the paths in
preferences are converted to the desired format.
Anyway, I am posting the patch as it is now, in order to get some
feedback.
--
Enrico
Index: src/lyxfunc.C
===
--- src/lyxfunc.C (revision 13496)
+++ src/lyxfunc.C (working copy)
@@ -1977,6 +1977,11 @@ void actOnUpdatedPrefs(LyXRC const & lyx
namespace os = lyx::support::os;
os::cygwin_path_fix(lyxrc_new.cygwin_path_fix);
}
+ case LyXRC::RC_CYGWIN_STYLE_PATHS:
+ if (lyxrc_orig.cygwin_style_paths !=
lyxrc_new.cygwin_style_paths) {
+ namespace os = lyx::support::os;
+ os::cygwin_style_paths(lyxrc_new.cygwin_style_paths);
+ }
case LyXRC::RC_DATE_INSERT_FORMAT:
case LyXRC::RC_DEFAULT_LANGUAGE:
case LyXRC::RC_DEFAULT_PAPERSIZE:
Index: src/frontends/qt2/QPrefs.C
===
--- src/frontends/qt2/QPrefs.C (revision 13496)
+++ src/frontends/qt2/QPrefs.C (working copy)
@@ -148,6 +148,11 @@ string const internal_path(QString const
return lyx::support::os::internal_path(fromqstr(input));
}
+string const internal_path_list(QString const & input)
+{
+ return lyx::support::os::internal_path_list(fromqstr(input));
+}
+
}
@@ -199,7 +204,7 @@ void QPrefs::apply()
#if defined(__CYGWIN__) || defined(__CYGWIN32__)
QPrefCygwinPathModule * cygwinmod(dialog_->cygwinpathModule);
- rc.cygwin_path_fix = cygwinmod->pathCB->isChecked();
+ rc.cygwin_style_paths = cygwinmod->pathCB->isChecked();
#endif
QPrefLatexModule * latexmod(dialog_->latexModule);
@@ -253,7 +258,7 @@ void QPrefs::apply()
rc.template_path = internal_path(pathsmod->templateDirED->text());
rc.backupdir_path = internal_path(pathsmod->backupDirED->text());
rc.tempdir_path = internal_path(pathsmod->tempDirED->text());
- rc.path_prefix = fromqstr(pathsmod->pathPrefixED->text());
+ rc.path_prefix = internal_path_list(pathsmod->pathPrefixED->text());
// FIXME: should be a checkbox only
rc.lyxpipes = internal_path(pathsmod->lyxserverDirED->text());
@@ -467,6 +472,11 @@ QString const external_path(string const
return toqstr(lyx::support::os::external_path(input));
}
+QString const external_path_list(string const & input)
+{
+ return toqstr(lyx::support::os::external_path_list(input));
+}
+
}
@@ -536,7 +546,7 @@ void QPrefs::update_contents()
#if defined(__CYGWIN__) || defined(__CYGWIN32__)
QPrefCygwinPathModule * cygwinmod(dialog_->cygwinpathModule);
-
Re: A Cygwin related patch
Georg == Georg Baum [EMAIL PROTECTED] writes: Georg Here you basically revert external_path on non-cygwin windows. Georg This is a bit ugly, so I moved this into a new function Georg lyx::support::os::latex_path(). This is a noop except on Georg cygwin, where it is identical to external_path(). Shall I put Georg this in? I thought the idea of cygwin_path_fix was that it is only needed for latex. I am not so sure anymore. JMarc
Re: A Cygwin related patch
Jean-Marc Lasgouttes wrote: I thought the idea of cygwin_path_fix was that it is only needed for latex. I am not so sure anymore. The lines //No backslashes in LaTeX files dos_path = subst(dos_path,'\\','/'); would support the argument that external_path() is intended for latex on cygwin. This might also explain the need for patching qt on cygwin, since external_path() is used for paths in file dialogs, for example. Anyway, the patch is somewehat orthogonal to the question whether cygwin_path_fix should be used in external_path or not. We would need to change the implementation of latex_path() if that should be true, but the external usage would stay the same. So what do you think about the patch? Should it go in, or should we first research the use cygwin_path_fix in external_path or not issue? Georg
Re: A Cygwin related patch
Jean-Marc Lasgouttes wrote:
I think I agree with it. Could you repost it please? I am lost in the
thread :)
Sure, here it is.
GeorgIndex: src/support/os_unix.C
===
--- src/support/os_unix.C (Revision 13467)
+++ src/support/os_unix.C (Arbeitskopie)
@@ -63,6 +63,12 @@ string internal_path(string const p)
}
+string latex_path(string const p)
+{
+ return p;
+}
+
+
bool is_absolute_path(string const p)
{
return !p.empty() p[0] == '/';
Index: src/support/os.h
===
--- src/support/os.h (Revision 13467)
+++ src/support/os.h (Arbeitskopie)
@@ -47,6 +47,14 @@ std::string external_path(std::string co
/// Converts a host OS style path to unix style.
std::string internal_path(std::string const p);
+/**
+ * Converts a unix style path into a form suitable for inclusion in a LaTeX
+ * document.
+ * Caution: This function handles only the OS specific part of that task.
+ * Never use it directly, use lyx::support::latex_path instead.
+ */
+std::string latex_path(std::string const p);
+
/// Is the path absolute?
bool is_absolute_path(std::string const p);
Index: src/support/filetools.C
===
--- src/support/filetools.C (Revision 13467)
+++ src/support/filetools.C (Arbeitskopie)
@@ -87,6 +87,8 @@ string const latex_path(string const o
latex_path_dots dots)
{
string path = subst(original_path, \\, /);
+ // On cygwin, we may need windows or posix style paths.
+ path = os::latex_path(path);
path = subst(path, ~, \\string~);
if (path.find(' ') != string::npos) {
// We can't use '' because is sometimes active (e.g. if
Index: src/support/os_win32.C
===
--- src/support/os_win32.C (Revision 13467)
+++ src/support/os_win32.C (Arbeitskopie)
@@ -216,6 +216,12 @@ string internal_path(string const p)
}
+string latex_path(string const p)
+{
+ return p;
+}
+
+
// (Claus H.) On Win32 both Unix and Win32/DOS pathnames are used.
// Therefore an absolute path could be either a pathname starting
// with a slash (Unix) or a pathname starting with a drive letter
Index: src/support/os_cygwin.C
===
--- src/support/os_cygwin.C (Revision 13467)
+++ src/support/os_cygwin.C (Arbeitskopie)
@@ -102,6 +102,15 @@ string internal_path(string const p)
}
+string latex_path(string const p)
+{
+ // We may need a posix style path or a windows style path (depending
+ // on cygwin_path_fix_), but we use always forward slashes, since it
+ // gets written into a .tex file.
+ return external_path(p);
+}
+
+
bool is_absolute_path(string const p)
{
if (p.empty())
Re: A Cygwin related patch
Georg == Georg Baum [EMAIL PROTECTED] writes: Georg Jean-Marc Lasgouttes wrote: I think I agree with it. Could you repost it please? I am lost in the thread :) Georg Sure, here it is. Yes, that's the one I saw. Apply it to trunk and branch. JMarc
Re: A Cygwin related patch
On Fri, Mar 24, 2006 at 11:09:00AM +0100, Georg Baum wrote: Jean-Marc Lasgouttes wrote: I thought the idea of cygwin_path_fix was that it is only needed for latex. I am not so sure anymore. Why do you say so? The lines //No backslashes in LaTeX files dos_path = subst(dos_path,'\\','/'); would support the argument that external_path() is intended for latex on cygwin. Indeed. This might also explain the need for patching qt on cygwin, since external_path() is used for paths in file dialogs, for example. I really faced the opposite problem, as Qt was being passed posix paths, which is right, but when using the native graphics version, calls to the Windows API were being made and a conversion to win-style paths was needed. If I remember correctly, I patched the Qt sources such that both win-style and posix-style paths were ok. I also remember that I had to do something for UNC paths (//machine/xxx), which otherwise would have not worked. That is because I was so wary about the unconditional call to external_path. I am sure that my Qt port can cope with whatever path-style it is presented, but I cannot be sure for other ports. Maybe Kayvan can shed some light? I think that the current Qt version distributed with cygwin cannot be used for compiling LyX as it is multi-threaded and there are problems with the cygwin1.dll, as explained here: http://www.mail-archive.com/cygwin@cygwin.com/msg65681.html Anyway, the patch is somewehat orthogonal to the question whether cygwin_path_fix should be used in external_path or not. We would need to change the implementation of latex_path() if that should be true, but the external usage would stay the same. Well, I have been compiling by myself LyX and Qt/X11 since LyX 1.3.4 and until 1.3.5 (then I have been using the Angus port) and never noticed a problem. Even this latex_path issue was escaping me. But I am not sure if this was because until a certain time I have been using the cygwin tetex before switching to miktex. So what do you think about the patch? Should it go in, or should we first research the use cygwin_path_fix in external_path or not issue? In my experience, if a cygwin application does not try to manipulate a path by itself, i.e., if it simply use the pathname for loading, or saving, or whatever, it can even be passed a true win-style path with '\'. A notable exception is latex, which for some reason needs posix-style paths in .tex files. So, given that things work as they stand now, why changing? Anyway, I have no problem in maintaining my own patches to the official sources. To tell you the truth, there is almost no free software I use that I do not patch for some reason or another. Sometimes it is because the patches are really peculiar to me, but sometimes it is because I am scared by the time you have to invest to have them accepted (I don't mean you in this case, really). -- Enrico
Re: A Cygwin related patch
On Fri, Mar 24, 2006 at 12:51:17PM +0100, Georg Baum wrote: Jean-Marc Lasgouttes wrote: I think I agree with it. Could you repost it please? I am lost in the thread :) Sure, here it is. I have a comment on this patch. I see that you get rid of the rtrim call in latex_path. This has the following effect: in all cases the argument of [EMAIL PROTECTED] will have two '/' appended, except on cygwin when cygwin_path_fix_ is true, in which case only one '/' is appended. This is because external_path() will trim the trailing '/' when cygwin_path_fix_ is true. -- Enrico
Re: A Cygwin related patch
Enrico Forestieri wrote: I have a comment on this patch. I see that you get rid of the rtrim call in latex_path. This has the following effect: in all cases the argument of [EMAIL PROTECTED] will have two '/' appended, except on cygwin when cygwin_path_fix_ is true, in which case only one '/' is appended. This is because external_path() will trim the trailing '/' when cygwin_path_fix_ is true. Why does it do that? It does not make any sense to me. I'll commit the patch as-is rught now. It is a monotonic improvement over the current situation, we can fix this issue when we know why it happens. Georg
Re: A Cygwin related patch
Enrico Forestieri wrote: On Fri, Mar 24, 2006 at 11:09:00AM +0100, Georg Baum wrote: So what do you think about the patch? Should it go in, or should we first research the use cygwin_path_fix in external_path or not issue? In my experience, if a cygwin application does not try to manipulate a path by itself, i.e., if it simply use the pathname for loading, or saving, or whatever, it can even be passed a true win-style path with '\'. A notable exception is latex, which for some reason needs posix-style paths in .tex files. No, it does not need posix style paths: It simply cannot cope with backslashes for obvious reasons. AFAIK miktex can handle paths like C:/temp/test.tex. Not a big difference, but not exactly posix either. So, given that things work as they stand now, why changing? If everything works: Fine, but from reading the code I suspect that there might be problems when calling external programs or handling paths to the filedialog. Of course we should not fix anything when we are not sure where the problem lies. Anyway, I have no problem in maintaining my own patches to the official sources. To tell you the truth, there is almost no free software I use that I do not patch for some reason or another. Sometimes it is because the patches are really peculiar to me, but sometimes it is because I am scared by the time you have to invest to have them accepted (I don't mean you in this case, really). Similar here. If I discover a bug I normally report it through the bug tracker or mailing list. If they don't act on that it is not my problem. Georg
Re: A Cygwin related patch
On Fri, Mar 24, 2006 at 01:43:55PM +0100, Georg Baum wrote: Enrico Forestieri wrote: I have a comment on this patch. I see that you get rid of the rtrim call in latex_path. This has the following effect: in all cases the argument of [EMAIL PROTECTED] will have two '/' appended, except on cygwin when cygwin_path_fix_ is true, in which case only one '/' is appended. This is because external_path() will trim the trailing '/' when cygwin_path_fix_ is true. Why does it do that? It does not make any sense to me. This is the way the cygwin call which convert paths works... I'll commit the patch as-is rught now. It is a monotonic improvement over the current situation, we can fix this issue when we know why it happens. I was only being pedantic. I put a lot of attention on those small things because you never know to what a small difference can lead. Anyway, the argument of [EMAIL PROTECTED] should have a single shash appended and the rtrim would guarantee that. -- Enrico PS: Sorry that I cannot reply promptly, but I have also to do some work.
Re: A Cygwin related patch
> "Georg" == Georg Baum <[EMAIL PROTECTED]> writes: Georg> Here you basically revert external_path on non-cygwin windows. Georg> This is a bit ugly, so I moved this into a new function Georg> lyx::support::os::latex_path(). This is a noop except on Georg> cygwin, where it is identical to external_path(). Shall I put Georg> this in? I thought the idea of cygwin_path_fix was that it is only needed for latex. I am not so sure anymore. JMarc
Re: A Cygwin related patch
Jean-Marc Lasgouttes wrote: > I thought the idea of cygwin_path_fix was that it is only needed for > latex. I am not so sure anymore. The lines //No backslashes in LaTeX files dos_path = subst(dos_path,'\\','/'); would support the argument that external_path() is intended for latex on cygwin. This might also explain the need for patching qt on cygwin, since external_path() is used for paths in file dialogs, for example. Anyway, the patch is somewehat orthogonal to the question whether cygwin_path_fix should be used in external_path or not. We would need to change the implementation of latex_path() if that should be true, but the external usage would stay the same. So what do you think about the patch? Should it go in, or should we first research the "use cygwin_path_fix in external_path or not" issue? Georg
Re: A Cygwin related patch
Jean-Marc Lasgouttes wrote:
> I think I agree with it. Could you repost it please? I am lost in the
> thread :)
Sure, here it is.
GeorgIndex: src/support/os_unix.C
===
--- src/support/os_unix.C (Revision 13467)
+++ src/support/os_unix.C (Arbeitskopie)
@@ -63,6 +63,12 @@ string internal_path(string const & p)
}
+string latex_path(string const & p)
+{
+ return p;
+}
+
+
bool is_absolute_path(string const & p)
{
return !p.empty() && p[0] == '/';
Index: src/support/os.h
===
--- src/support/os.h (Revision 13467)
+++ src/support/os.h (Arbeitskopie)
@@ -47,6 +47,14 @@ std::string external_path(std::string co
/// Converts a host OS style path to unix style.
std::string internal_path(std::string const & p);
+/**
+ * Converts a unix style path into a form suitable for inclusion in a LaTeX
+ * document.
+ * Caution: This function handles only the OS specific part of that task.
+ * Never use it directly, use lyx::support::latex_path instead.
+ */
+std::string latex_path(std::string const & p);
+
/// Is the path absolute?
bool is_absolute_path(std::string const & p);
Index: src/support/filetools.C
===
--- src/support/filetools.C (Revision 13467)
+++ src/support/filetools.C (Arbeitskopie)
@@ -87,6 +87,8 @@ string const latex_path(string const & o
latex_path_dots dots)
{
string path = subst(original_path, "\\", "/");
+ // On cygwin, we may need windows or posix style paths.
+ path = os::latex_path(path);
path = subst(path, "~", "\\string~");
if (path.find(' ') != string::npos) {
// We can't use '"' because " is sometimes active (e.g. if
Index: src/support/os_win32.C
===
--- src/support/os_win32.C (Revision 13467)
+++ src/support/os_win32.C (Arbeitskopie)
@@ -216,6 +216,12 @@ string internal_path(string const & p)
}
+string latex_path(string const & p)
+{
+ return p;
+}
+
+
// (Claus H.) On Win32 both Unix and Win32/DOS pathnames are used.
// Therefore an absolute path could be either a pathname starting
// with a slash (Unix) or a pathname starting with a drive letter
Index: src/support/os_cygwin.C
===
--- src/support/os_cygwin.C (Revision 13467)
+++ src/support/os_cygwin.C (Arbeitskopie)
@@ -102,6 +102,15 @@ string internal_path(string const & p)
}
+string latex_path(string const & p)
+{
+ // We may need a posix style path or a windows style path (depending
+ // on cygwin_path_fix_), but we use always forward slashes, since it
+ // gets written into a .tex file.
+ return external_path(p);
+}
+
+
bool is_absolute_path(string const & p)
{
if (p.empty())
Re: A Cygwin related patch
> "Georg" == Georg Baum <[EMAIL PROTECTED]> writes: Georg> Jean-Marc Lasgouttes wrote: >> I think I agree with it. Could you repost it please? I am lost in >> the thread :) Georg> Sure, here it is. Yes, that's the one I saw. Apply it to trunk and branch. JMarc
Re: A Cygwin related patch
On Fri, Mar 24, 2006 at 11:09:00AM +0100, Georg Baum wrote: > Jean-Marc Lasgouttes wrote: > > > I thought the idea of cygwin_path_fix was that it is only needed for > > latex. I am not so sure anymore. Why do you say so? > The lines > > //No backslashes in LaTeX files > dos_path = subst(dos_path,'\\','/'); > > would support the argument that external_path() is intended for latex on > cygwin. Indeed. > This might also explain the need for patching qt on cygwin, since > external_path() is used for paths in file dialogs, for example. I really faced the opposite problem, as Qt was being passed posix paths, which is right, but when using the native graphics version, calls to the Windows API were being made and a conversion to win-style paths was needed. If I remember correctly, I patched the Qt sources such that both win-style and posix-style paths were ok. I also remember that I had to do something for UNC paths (//machine/xxx), which otherwise would have not worked. That is because I was so wary about the unconditional call to external_path. I am sure that my Qt port can cope with whatever path-style it is presented, but I cannot be sure for other ports. Maybe Kayvan can shed some light? I think that the current Qt version distributed with cygwin cannot be used for compiling LyX as it is multi-threaded and there are problems with the cygwin1.dll, as explained here: http://www.mail-archive.com/cygwin@cygwin.com/msg65681.html > Anyway, the patch is somewehat orthogonal to the question whether > cygwin_path_fix should be used in external_path or not. We would need to > change the implementation of latex_path() if that should be true, but the > external usage would stay the same. Well, I have been compiling by myself LyX and Qt/X11 since LyX 1.3.4 and until 1.3.5 (then I have been using the Angus port) and never noticed a problem. Even this latex_path issue was escaping me. But I am not sure if this was because until a certain time I have been using the cygwin tetex before switching to miktex. > So what do you think about the patch? Should it go in, or should we first > research the "use cygwin_path_fix in external_path or not" issue? In my experience, if a cygwin application does not try to manipulate a path by itself, i.e., if it simply use the pathname for loading, or saving, or whatever, it can even be passed a true win-style path with '\'. A notable exception is latex, which for some reason needs posix-style paths in .tex files. So, given that things work as they stand now, why changing? Anyway, I have no problem in maintaining my own patches to the official sources. To tell you the truth, there is almost no free software I use that I do not patch for some reason or another. Sometimes it is because the patches are really peculiar to me, but sometimes it is because I am scared by the time you have to invest to have them accepted (I don't mean you in this case, really). -- Enrico
Re: A Cygwin related patch
On Fri, Mar 24, 2006 at 12:51:17PM +0100, Georg Baum wrote: > Jean-Marc Lasgouttes wrote: > > > I think I agree with it. Could you repost it please? I am lost in the > > thread :) > > Sure, here it is. I have a comment on this patch. I see that you get rid of the rtrim call in latex_path. This has the following effect: in all cases the argument of [EMAIL PROTECTED] will have two '/' appended, except on cygwin when cygwin_path_fix_ is true, in which case only one '/' is appended. This is because external_path() will trim the trailing '/' when cygwin_path_fix_ is true. -- Enrico
Re: A Cygwin related patch
Enrico Forestieri wrote: > I have a comment on this patch. I see that you get rid of the rtrim call > in latex_path. This has the following effect: in all cases the argument > of [EMAIL PROTECTED] will have two '/' appended, except on cygwin when > cygwin_path_fix_ is true, in which case only one '/' is appended. > This is because external_path() will trim the trailing '/' when > cygwin_path_fix_ is true. Why does it do that? It does not make any sense to me. I'll commit the patch as-is rught now. It is a monotonic improvement over the current situation, we can fix this issue when we know why it happens. Georg
Re: A Cygwin related patch
Enrico Forestieri wrote: > On Fri, Mar 24, 2006 at 11:09:00AM +0100, Georg Baum wrote: >> So what do you think about the patch? Should it go in, or should we first >> research the "use cygwin_path_fix in external_path or not" issue? > > In my experience, if a cygwin application does not try to manipulate a > path by itself, i.e., if it simply use the pathname for loading, or > saving, or whatever, it can even be passed a true win-style path with '\'. > A notable exception is latex, which for some reason needs posix-style > paths in .tex files. No, it does not need posix style paths: It simply cannot cope with backslashes for obvious reasons. AFAIK miktex can handle paths like "C:/temp/test.tex". Not a big difference, but not exactly posix either. > So, given that things work as they stand now, why > changing? If everything works: Fine, but from reading the code I suspect that there might be problems when calling external programs or handling paths to the filedialog. Of course we should not fix anything when we are not sure where the problem lies. > Anyway, I have no problem in maintaining my own patches to the official > sources. To tell you the truth, there is almost no free software I use > that I do not patch for some reason or another. Sometimes it is because > the patches are really peculiar to me, but sometimes it is because I am > scared by the time you have to invest to have them accepted (I don't mean > you in this case, really). Similar here. If I discover a bug I normally report it through the bug tracker or mailing list. If they don't act on that it is not my problem. Georg
Re: A Cygwin related patch
On Fri, Mar 24, 2006 at 01:43:55PM +0100, Georg Baum wrote: > Enrico Forestieri wrote: > > > I have a comment on this patch. I see that you get rid of the rtrim call > > in latex_path. This has the following effect: in all cases the argument > > of [EMAIL PROTECTED] will have two '/' appended, except on cygwin when > > cygwin_path_fix_ is true, in which case only one '/' is appended. > > This is because external_path() will trim the trailing '/' when > > cygwin_path_fix_ is true. > > Why does it do that? It does not make any sense to me. This is the way the cygwin call which convert paths works... > I'll commit the patch as-is rught now. It is a monotonic improvement over > the current situation, we can fix this issue when we know why it happens. I was only being pedantic. I put a lot of attention on those small things because you never know to what a small difference can lead. Anyway, the argument of [EMAIL PROTECTED] should have a single shash appended and the rtrim would guarantee that. -- Enrico PS: Sorry that I cannot reply promptly, but I have also to do some work.
Re: A Cygwin related patch
Enrico Forestieri wrote: The attached patch fixes the bug reported here: http://thread.gmane.org/gmane.editors.lyx.general/29227 In my intention this is the first of a series of patches aimed at polishing the Cygwin target, which I think is lagging behind. Please, tell me if this ok with you. I have no problem in maintaining the Cygwin target but my time is limited so I cannot guarantee a continuous commitment. I will do it time permitting. There should be no big problems, as I think the Cygwin users will not be more than those I can count with only one of my hands ;-) This patch looks good. I don't like the other one, because the code in filetools.C is more complicated, and in this one it is more clear because of cygwin_path_fix(). Do you prefer that I file this one on bugzilla? I think it is there already: http://bugzilla.lyx.org/show_bug.cgi?id=2409 Or do you fix something else? Georg
Re: A Cygwin related patch
Martin Vermeer [EMAIL PROTECTED] writes: On Wed, Mar 22, 2006 at 09:29:35PM +, Enrico Forestieri wrote: The attached patch fixes the bug reported here: http://thread.gmane.org/gmane.editors.lyx.general/29227 In my intention this is the first of a series of patches aimed at polishing the Cygwin target, which I think is lagging behind. Please, tell me if this ok with you. I have no problem in maintaining the Cygwin target but my time is limited so I cannot guarantee a continuous commitment. I will do it time permitting. There should be no big problems, as I think the Cygwin users will not be more than those I can count with only one of my hands You would be amazed by the true number of LyX/Windows users... I have no doubt, but somehow I have the idea that LyX/Cygwin users are a niche. However, I maybe wrong. Unfortunately I am unable / cannot be bothered to work on that, but I am really happy somebody does. LyX adoption benefits greatly by having the option of cross-platform collaborative authoring. And some people are stuck on Windows. BTW some of us are hit by symptoms like compulsive hand washing after touching Windows. Another reason for admiring you I am not any different... I can bear Windows only because of cygwin. With it, my working environment is not any different from some *nix flavor ;-) -- Enrico
Re: A Cygwin related patch
Georg Baum [EMAIL PROTECTED] writes: Enrico Forestieri wrote: The attached patch fixes the bug reported here: http://thread.gmane.org/gmane.editors.lyx.general/29227 In my intention this is the first of a series of patches aimed at polishing the Cygwin target, which I think is lagging behind. Please, tell me if this ok with you. I have no problem in maintaining the Cygwin target but my time is limited so I cannot guarantee a continuous commitment. I will do it time permitting. There should be no big problems, as I think the Cygwin users will not be more than those I can count with only one of my hands This patch looks good. I don't like the other one, because the code in filetools.C is more complicated, and in this one it is more clear because of cygwin_path_fix(). Really? I had thought the second one was more cleaner. Actually I had to figure out that when latex_path() is called for generating the argument of [EMAIL PROTECTED] it is assumed that a trailing / is present, and the second patch makes this clear not only for cygwin. On the other hand, external_path() is almost a no-op for the other OSes, so I don't it is such an overhead. So, what shall I use? Do you prefer that I file this one on bugzilla? I think it is there already: http://bugzilla.lyx.org/show_bug.cgi?id=2409 Or do you fix something else? Funny, it was not there when I started working on the bug ;-) -- Enrico
Re: A Cygwin related patch
Enrico == Enrico Forestieri [EMAIL PROTECTED] writes: Enrico Really? I had thought the second one was more cleaner. Enrico Actually I had to figure out that when latex_path() is called Enrico for generating the argument of [EMAIL PROTECTED] it is assumed that Enrico a trailing / is present, and the second patch makes this clear Enrico not only for cygwin. Why shall we assume that? I do not really understand this explicit check for a trailing slash. I think it should be elsewhere. JMarc
Re: A Cygwin related patch
Enrico Forestieri wrote: Really? I had thought the second one was more cleaner. Actually I had to figure out that when latex_path() is called for generating the argument of [EMAIL PROTECTED] it is assumed that a trailing / is present, and the second patch makes this clear not only for cygwin. [EMAIL PROTECTED] must always be an absolute path (otherwise it would not work TeX-wise), but why should that matter in LyX? On the other hand, external_path() is almost a no-op for the other OSes, so I don't it is such an overhead. So, what shall I use? The overhead does not matter at all here. We have had a lot of path problems on the different windows flavors. Angus worked hard to resolve these, and implemented the external/internal path distiction. If we want to avoid more problems we must keep this distinction and be always explicit whether a path is external or intrnal. In the long run we should not juggle with paths as strings, but always use a path class with external() and internal() methods. This will make the distinction more clear. I am not really sure how things work on cygwin. Can we always assume that all external paths are either windows or posix style? Or do we need posix paths in some cases and windows paths in others? Funny, it was not there when I started working on the bug ;-) It should have been there a long time ago, but finally somebody did not only complain on lyx-users and added it. Georg
Re: A Cygwin related patch
Jean-Marc Lasgouttes [EMAIL PROTECTED] writes: Enrico == Enrico Forestieri [EMAIL PROTECTED] writes: Enrico Really? I had thought the second one was more cleaner. Enrico Actually I had to figure out that when latex_path() is called Enrico for generating the argument of \input at path it is assumed that Enrico a trailing / is present, and the second patch makes this clear Enrico not only for cygwin. Why shall we assume that? I do not really understand this explicit check for a trailing slash. I think it should be elsewhere. I saw that the [EMAIL PROTECTED] argument always has a double / appended. In my eyes this was not wrong because I remember that in kpathsea a double / signals to also search for subdirectories. Your post rang an alarm bell for me and I checked the [EMAIL PROTECTED] syntax and indeed it is not the same as for kpathsea. A single / should be appended to the path, but it seems that two do not hurt. Now, as a / was later added to the path, in my mind a / should have already been in the path. And indeed this is the case: whenever latex_path() is called to generate the argument of [EMAIL PROTECTED] the path which is passed always has a / appended. As I think that one should not fix what is not broken, in my patch I used that to distinguish when latex_path should generate the argument of [EMAIL PROTECTED] such that to act on this case only. Not only, as in cygwin external_path() can remove the trailing slash, I also paid attention to readd it... So I now know that the trailing slash is not there on purpose. However, it is a good flag for signaling latex_path() that a path for [EMAIL PROTECTED] is to be generated. Now I think I have two choices: 1) Find in the sources where latex_path() is called for [EMAIL PROTECTED], in order to enforce the trailing slash as a flag for this case, and then modify the patch such as to later remove it in latex_path(). 2) Check if the path passed to latex_path() is a directory. If it is, assume that this is the [EMAIL PROTECTED] case and act accordingly. What do you think is the better choice? -- Enrico
Re: A Cygwin related patch
Georg Baum [EMAIL PROTECTED] writes: Enrico Forestieri wrote: Really? I had thought the second one was more cleaner. Actually I had to figure out that when latex_path() is called for generating the argument of \input at path it is assumed that a trailing / is present, and the second patch makes this clear not only for cygwin. \input at path must always be an absolute path (otherwise it would not work TeX-wise), but why should that matter in LyX? Trailing, not heading /. But it seems I was misunderstanding the code. See my other post. On the other hand, external_path() is almost a no-op for the other OSes, so I don't it is such an overhead. So, what shall I use? The overhead does not matter at all here. We have had a lot of path problems on the different windows flavors. Angus worked hard to resolve these, and implemented the external/internal path distiction. If we want to avoid more problems we must keep this distinction and be always explicit whether a path is external or intrnal. In the long run we should not juggle with paths as strings, but always use a path class with external() and internal() methods. This will make the distinction more clear. Ok I am not really sure how things work on cygwin. Can we always assume that all external paths are either windows or posix style? Or do we need posix paths in some cases and windows paths in others? The latter. On cygwin, depending on a checkbox status, external_path() generates posix or windows style paths. This is because if you use MikTeX you need win-style paths. The configure script checks what kind of paths are needed and then sets the boolean \cygwin_path_fix_needed in lyxrc.defaults. But then you can override it by a checkbox (which, btw, is misnamed as it does the contrary of what its name implies). -- Enrico
Re: A Cygwin related patch
Enrico Forestieri wrote: Trailing, not heading /. But it seems I was misunderstanding the code. And I was misunderstanding your post :-( I am not really sure how things work on cygwin. Can we always assume that all external paths are either windows or posix style? Or do we need posix paths in some cases and windows paths in others? The latter. On cygwin, depending on a checkbox status, external_path() generates posix or windows style paths. This is because if you use MikTeX you need win-style paths. The configure script checks what kind of paths are needed and then sets the boolean \cygwin_path_fix_needed in lyxrc.defaults. But then you can override it by a checkbox (which, btw, is misnamed as it does the contrary of what its name implies). What I meant was: Do we need for one setting of cygwin_path_fix_needed both windos/posix styles, or does cygwin_path_fix_needed simply switch external_path() between the windows and posix version? Georg
Re: A Cygwin related patch
Georg Baum [EMAIL PROTECTED] writes: | Enrico Forestieri wrote: | | The attached patch fixes the bug reported here: | http://thread.gmane.org/gmane.editors.lyx.general/29227 | | In my intention this is the first of a series of patches aimed at | polishing the Cygwin target, which I think is lagging behind. Please, tell | me if this ok with you. I have no problem in maintaining the Cygwin target | but my time is limited so I cannot guarantee a continuous commitment. I | will do it time permitting. There should be no big problems, as I think | the Cygwin users will not be more than those I can count with only one of | my hands ;-) | | This patch looks good. I don't like the other one, because the code in | filetools.C is more complicated, and in this one it is more clear because | of cygwin_path_fix(). I feel the exact opposite. the cygwin_path_fix pollutes the portable code, pollutes the not cygwin files The second patch is the nice one IMO. -- Lgb
Re: A Cygwin related patch
Lars Gullik Bjønnes wrote: I feel the exact opposite. the cygwin_path_fix pollutes the portable code, pollutes the not cygwin files But it makes it explicit why external_path() is needed. The second patch is the nice one IMO. Now I don't like either anymore. The problem of both fixes is that they use a trailing '/' as flag whether external_path() is needed or not. This will strike back sooner or later. Georg
Re: A Cygwin related patch
Georg == Georg Baum [EMAIL PROTECTED] writes: Georg Now I don't like either anymore. The problem of both fixes is Georg that they use a trailing '/' as flag whether external_path() is Georg needed or not. This will strike back sooner or later. Sure, this is not good. Is the problem that latex_path() needs different syntax depending on whether the file name is used inside a .tex file or as command argument? What is the problem exactly? JMarc
Re: A Cygwin related patch
Georg Baum [EMAIL PROTECTED] writes: What I meant was: Do we need for one setting of cygwin_path_fix_needed both windos/posix styles, or does cygwin_path_fix_needed simply switch external_path() between the windows and posix version? As it is now, cygwin_path_fix_needed simply switch external_path() between the windows and posix version. However, if regarded as an OS, cygwin is really posix, so in some cases one must always use internal_path(). This is a singular case with respect to the other supported OSes. It may seem complicated, but apart the need for some polish, it does work. -- Enrico
Re: A Cygwin related patch
Jean-Marc Lasgouttes [EMAIL PROTECTED] writes:
Georg == Georg Baum [EMAIL PROTECTED] writes:
Georg Now I don't like either anymore. The problem of both fixes is
Georg that they use a trailing '/' as flag whether external_path() is
Georg needed or not. This will strike back sooner or later.
Sure, this is not good. Is the problem that latex_path() needs
different syntax depending on whether the file name is used inside a
.tex file or as command argument?
What is the problem exactly?
The problem is that I have not a full grasp of the overall code and don't
want introduce changes without fully knowing the consequences. I repeat,
my idea is of only fixing what is broken, making changes that I fully
understand. As a problem arised with [EMAIL PROTECTED], I tried to fix only
that case.
As regards the trailing '/', I think this is not a problem, as there is
only one place in the code where latex_path() is called for generating
the argument of [EMAIL PROTECTED] It is in src/buffer.C, so a patch in which
I am confident is along these lines:
--- src/buffer.C(revision 13450)
+++ src/buffer.C(working copy)
@@ -875,7 +876,9 @@ void Buffer::makeLaTeXFile(ostream os,
texrow().newline();
}
if (!original_path.empty()) {
- string const inputpath = latex_path(original_path);
+ // A trailing slash is used as a flag for latex_path
+ // which will return an unslashed path.
+ string const inputpath = latex_path(original_path + /)
;
os \\makeatletter\n
[EMAIL PROTECTED]
inputpath /}}\n
and then in latex_path(), either:
--- src/support/filetools.C (revision 13450)
+++ src/support/filetools.C (working copy)
@@ -87,6 +87,11 @@ string const latex_path(string const o
latex_path_dots dots)
{
string path = subst(original_path, \\, /);
+ if (suffixIs(path, '/')) {
+ path = rtrim(path, /);
+ if (os::cygwin_path_fix())
+ path = os::external_path(path);
+ }
path = subst(path, ~, \\string~);
if (path.find(' ') != string::npos) {
// We can't use '' because is sometimes active (e.g. if
alongthe lines of the first patch, or:
--- src/support/filetools.C (revision 13450)
+++ src/support/filetools.C (working copy)
@@ -86,7 +86,15 @@ string const latex_path(string const o
latex_path_extension extension,
latex_path_dots dots)
{
- string path = subst(original_path, \\, /);
+ string path;
+
+ if (suffixIs(original_path, '/')) {
+ path = rtrim(path, /);
+ // The call to os::external_path is needed for Cygwin.
+ path = subst(os::external_path(original_path), \\, /);
+ } else
+ path = subst(original_path, \\, /);
+
path = subst(path, ~, \\string~);
if (path.find(' ') != string::npos) {
// We can't use '' because is sometimes active (e.g. if
along the lines of the second patch. Both approaches work and are robust
because the trailing '/' is anyway added. Please, tell me what to do.
I do not feel confortable in unconditionally call external_path (I don't
want to fix what is not broken).
--
Enrico
Re: A Cygwin related patch
Enrico Forestieri wrote:
The problem is that I have not a full grasp of the overall code and don't
want introduce changes without fully knowing the consequences. I repeat,
my idea is of only fixing what is broken, making changes that I fully
understand. As a problem arised with [EMAIL PROTECTED], I tried to fix only
that case.
I do fully agree. My point however is that using a trailing '/' as flag will
sooner or later strike back. If it is really necessary to tell latex_path()
whether it should call external_path() or not, add an additional parameter
that tells exactly that.
--- src/support/filetools.C (revision 13450)
+++ src/support/filetools.C (working copy)
@@ -86,7 +86,15 @@ string const latex_path(string const o
latex_path_extension extension,
latex_path_dots dots)
{
- string path = subst(original_path, \\, /);
+ string path;
+
+ if (suffixIs(original_path, '/')) {
+ path = rtrim(path, /);
+ // The call to os::external_path is needed for Cygwin.
+ path = subst(os::external_path(original_path), \\, /);
+ } else
+ path = subst(original_path, \\, /);
+
path = subst(path, ~, \\string~);
if (path.find(' ') != string::npos) {
// We can't use '' because is sometimes active (e.g. if
along the lines of the second patch. Both approaches work and are robust
because the trailing '/' is anyway added. Please, tell me what to do.
This version is preferrable since it does not need the cygwin switch.
I do not feel confortable in unconditionally call external_path (I don't
want to fix what is not broken).
I think that you can safely call it unconditionally. I looked it up, and
latex_path() is only called on filenames that are written to .tex files. I
would be very surprised if a TeX compiler exists on windows that needs win
style paths in [EMAIL PROTECTED] but posix style paths in \includegraphics or
\input.
To be sure you could make a simple test: Call external_path()
unconditionally, and try to view a document that contains a graphics inset,
and two include insets, one with \input and one with \include. All files
should be entered with absolute names.
If my theory is right then running latex on the exported .tex file will work
with the unconditional fix and fail with the conditional one.
The absolute names are necessary to get slashes in the name, and running
latex on the exported .tex file is necessary because LyX uses always
relative paths for running latex internally.
Georg
PS: You might want to have a look at the cygwin version of external_path().
It contains unreachable code.
Re: A Cygwin related patch
Georg Baum [EMAIL PROTECTED] writes:
I think that you can safely call it unconditionally. I looked it up, and
latex_path() is only called on filenames that are written to .tex files. I
would be very surprised if a TeX compiler exists on windows that needs win
style paths in \inputa at path but posix style paths in \includegraphics or
\input.
To be sure you could make a simple test: Call external_path()
unconditionally, and try to view a document that contains a graphics inset,
and two include insets, one with \input and one with \include. All files
should be entered with absolute names.
If my theory is right then running latex on the exported .tex file will work
with the unconditional fix and fail with the conditional one.
The absolute names are necessary to get slashes in the name, and running
latex on the exported .tex file is necessary because LyX uses always
relative paths for running latex internally.
Your theory is right and I wasn't fearless enough, not having the time
to study the code. So, the attached is apparently the right fix. While
testing this patch I run up against another bug.
1) File-New
2) Insert-File-Child Document (say we input ~/test.tex)
3) File-Export-LaTeX
LyX pops up a dialog telling me that ~/test.tex already exists and
asking for overwriting it. I say yes, overwrite it, but newfile1.lyx
is normally created and ~/test.tex is not overwritten. This is
independent from the fact that the buffer has been given a name or not.
This also happens on Solaris and I could not find a similar bug
reported in bugzilla.
PS: You might want to have a look at the cygwin version of external_path().
It contains unreachable code.
Uhm, I don't think so. The code after else return p; gets executed when
the previous if() clause is satisfied.
Many thanks for your helpful and appreciated comments.
--
Enrico
Index: src/support/filetools.C
===
--- src/support/filetools.C (revision 13463)
+++ src/support/filetools.C (working copy)
@@ -86,7 +86,9 @@ string const latex_path(string const o
latex_path_extension extension,
latex_path_dots dots)
{
- string path = subst(original_path, \\, /);
+ // The call to os::external_path is only needed for Cygwin.
+ string path = subst(os::external_path(original_path), \\, /);
+ path = rtrim(path, /);
path = subst(path, ~, \\string~);
if (path.find(' ') != string::npos) {
// We can't use '' because is sometimes active (e.g. if
Re: A Cygwin related patch
Am Donnerstag, 23. März 2006 21:26 schrieb Enrico Forestieri:
Your theory is right and I wasn't fearless enough, not having the time
to study the code. So, the attached is apparently the right fix. While
testing this patch I run up against another bug.
1) File-New
2) Insert-File-Child Document (say we input ~/test.tex)
3) File-Export-LaTeX
LyX pops up a dialog telling me that ~/test.tex already exists and
asking for overwriting it. I say yes, overwrite it, but newfile1.lyx
is normally created and ~/test.tex is not overwritten. This is
independent from the fact that the buffer has been given a name or not.
This also happens on Solaris and I could not find a similar bug
reported in bugzilla.
I will have a look.
Uhm, I don't think so. The code after else return p; gets executed
when
the previous if() clause is satisfied.
Oh yes, you are right.
Many thanks for your helpful and appreciated comments.
I hope they are not too disappointing :-)
Unfortunately I have another one:
Index: src/support/filetools.C
===
--- src/support/filetools.C (revision 13463)
+++ src/support/filetools.C (working copy)
@@ -86,7 +86,9 @@ string const latex_path(string const o
latex_path_extension extension,
latex_path_dots dots)
{
- string path = subst(original_path, \\, /);
+ // The call to os::external_path is only needed for Cygwin.
+ string path = subst(os::external_path(original_path), \\, /);
Here you basically revert external_path on non-cygwin windows. This is a
bit ugly, so I moved this into a new function
lyx::support::os::latex_path(). This is a noop except on cygwin, where it
is identical to external_path().
Shall I put this in?
Georg
Index: src/support/os_unix.C
===
--- src/support/os_unix.C (Revision 13467)
+++ src/support/os_unix.C (Arbeitskopie)
@@ -63,6 +63,12 @@ string internal_path(string const p)
}
+string latex_path(string const p)
+{
+ return p;
+}
+
+
bool is_absolute_path(string const p)
{
return !p.empty() p[0] == '/';
Index: src/support/os.h
===
--- src/support/os.h (Revision 13467)
+++ src/support/os.h (Arbeitskopie)
@@ -47,6 +47,14 @@ std::string external_path(std::string co
/// Converts a host OS style path to unix style.
std::string internal_path(std::string const p);
+/**
+ * Converts a unix style path into a form suitable for inclusion in a LaTeX
+ * document.
+ * Caution: This function handles only the OS specific part of that task.
+ * Never use it directly, use lyx::support::latex_path instead.
+ */
+std::string latex_path(std::string const p);
+
/// Is the path absolute?
bool is_absolute_path(std::string const p);
Index: src/support/filetools.C
===
--- src/support/filetools.C (Revision 13467)
+++ src/support/filetools.C (Arbeitskopie)
@@ -87,6 +87,8 @@ string const latex_path(string const o
latex_path_dots dots)
{
string path = subst(original_path, \\, /);
+ // On cygwin, we may need windows or posix style paths.
+ path = os::latex_path(path);
path = subst(path, ~, \\string~);
if (path.find(' ') != string::npos) {
// We can't use '' because is sometimes active (e.g. if
Index: src/support/os_win32.C
===
--- src/support/os_win32.C (Revision 13467)
+++ src/support/os_win32.C (Arbeitskopie)
@@ -216,6 +216,12 @@ string internal_path(string const p)
}
+string latex_path(string const p)
+{
+ return p;
+}
+
+
// (Claus H.) On Win32 both Unix and Win32/DOS pathnames are used.
// Therefore an absolute path could be either a pathname starting
// with a slash (Unix) or a pathname starting with a drive letter
Index: src/support/os_cygwin.C
===
--- src/support/os_cygwin.C (Revision 13467)
+++ src/support/os_cygwin.C (Arbeitskopie)
@@ -102,6 +102,15 @@ string internal_path(string const p)
}
+string latex_path(string const p)
+{
+ // We may need a posix style path or a windows style path (depending
+ // on cygwin_path_fix_), but we use always forward slashes, since it
+ // gets written into a .tex file.
+ return external_path(p);
+}
+
+
bool is_absolute_path(string const p)
{
if (p.empty())
Re: A Cygwin related patch
Georg Baum [EMAIL PROTECTED] writes: Many thanks for your helpful and appreciated comments. I hope they are not too disappointing Oh, not at all... Here you basically revert external_path on non-cygwin windows. This is a bit ugly, so I moved this into a new function lyx::support::os::latex_path(). This is a noop except on cygwin, where it is identical to external_path(). Shall I put this in? It's very elegant. Please do. -- Enrico
Re: A Cygwin related patch
Enrico Forestieri wrote: > The attached patch fixes the bug reported here: > http://thread.gmane.org/gmane.editors.lyx.general/29227 > > In my intention this is the first of a series of patches aimed at > polishing the Cygwin target, which I think is lagging behind. Please, tell > me if this ok with you. I have no problem in maintaining the Cygwin target > but my time is limited so I cannot guarantee a continuous commitment. I > will do it time permitting. There should be no big problems, as I think > the Cygwin users will not be more than those I can count with only one of > my hands ;-) This patch looks good. I don't like the other one, because the code in filetools.C is more complicated, and in this one it is more clear because of cygwin_path_fix(). > Do you prefer that I file this one on bugzilla? I think it is there already: http://bugzilla.lyx.org/show_bug.cgi?id=2409 Or do you fix something else? Georg
Re: A Cygwin related patch
Martin Vermeer <[EMAIL PROTECTED]> writes: > > On Wed, Mar 22, 2006 at 09:29:35PM +, Enrico Forestieri wrote: > > The attached patch fixes the bug reported here: > > http://thread.gmane.org/gmane.editors.lyx.general/29227 > > > > In my intention this is the first of a series of patches aimed at polishing > > the Cygwin target, which I think is lagging behind. Please, tell me if this > > ok with you. I have no problem in maintaining the Cygwin target but my time > > is limited so I cannot guarantee a continuous commitment. I will do it time > > permitting. There should be no big problems, as I think the Cygwin users > > will not be more than those I can count with only one of my hands > > You would be amazed by the true number of LyX/Windows users... I have no doubt, but somehow I have the idea that LyX/Cygwin users are a niche. However, I maybe wrong. > Unfortunately I am unable / cannot be bothered to work on that, but I am > really happy somebody does. LyX adoption benefits greatly by having the > option of cross-platform collaborative authoring. And some people are > stuck on Windows. > > BTW some of us are hit by symptoms like compulsive hand washing after > touching Windows. Another reason for admiring you I am not any different... I can bear Windows only because of cygwin. With it, my working environment is not any different from some *nix flavor ;-) -- Enrico
Re: A Cygwin related patch
Georg Baum <[EMAIL PROTECTED]> writes: > > Enrico Forestieri wrote: > > > The attached patch fixes the bug reported here: > > http://thread.gmane.org/gmane.editors.lyx.general/29227 > > > > In my intention this is the first of a series of patches aimed at > > polishing the Cygwin target, which I think is lagging behind. Please, tell > > me if this ok with you. I have no problem in maintaining the Cygwin target > > but my time is limited so I cannot guarantee a continuous commitment. I > > will do it time permitting. There should be no big problems, as I think > > the Cygwin users will not be more than those I can count with only one of > > my hands > > This patch looks good. I don't like the other one, because the code in > filetools.C is more complicated, and in this one it is more clear because > of cygwin_path_fix(). Really? I had thought the second one was more cleaner. Actually I had to figure out that when latex_path() is called for generating the argument of [EMAIL PROTECTED] it is assumed that a trailing / is present, and the second patch makes this clear not only for cygwin. On the other hand, external_path() is almost a no-op for the other OSes, so I don't it is such an overhead. So, what shall I use? > > Do you prefer that I file this one on bugzilla? > > I think it is there already: http://bugzilla.lyx.org/show_bug.cgi?id=2409 Or > do you fix something else? Funny, it was not there when I started working on the bug ;-) -- Enrico
Re: A Cygwin related patch
> "Enrico" == Enrico Forestieri <[EMAIL PROTECTED]> writes: Enrico> Really? I had thought the second one was more cleaner. Enrico> Actually I had to figure out that when latex_path() is called Enrico> for generating the argument of [EMAIL PROTECTED] it is assumed that Enrico> a trailing / is present, and the second patch makes this clear Enrico> not only for cygwin. Why shall we assume that? I do not really understand this explicit check for a trailing slash. I think it should be elsewhere. JMarc
Re: A Cygwin related patch
Enrico Forestieri wrote: > Really? I had thought the second one was more cleaner. Actually I had to > figure out that when latex_path() is called for generating the argument > of [EMAIL PROTECTED] it is assumed that a trailing / is present, and the > second > patch makes this clear not only for cygwin. [EMAIL PROTECTED] must always be an absolute path (otherwise it would not work TeX-wise), but why should that matter in LyX? > On the other hand, > external_path() is almost a no-op for the other OSes, so I don't it is > such an overhead. So, what shall I use? The overhead does not matter at all here. We have had a lot of path problems on the different windows flavors. Angus worked hard to resolve these, and implemented the external/internal path distiction. If we want to avoid more problems we must keep this distinction and be always explicit whether a path is external or intrnal. In the long run we should not juggle with paths as strings, but always use a path class with external() and internal() methods. This will make the distinction more clear. I am not really sure how things work on cygwin. Can we always assume that all external paths are either windows or posix style? Or do we need posix paths in some cases and windows paths in others? > Funny, it was not there when I started working on the bug ;-) It should have been there a long time ago, but finally somebody did not only complain on lyx-users and added it. Georg
Re: A Cygwin related patch
Jean-Marc Lasgouttes <[EMAIL PROTECTED]> writes: > > > "Enrico" == Enrico Forestieri <[EMAIL PROTECTED]> writes: > > Enrico> Really? I had thought the second one was more cleaner. > Enrico> Actually I had to figure out that when latex_path() is called > Enrico> for generating the argument of \input path it is assumed that > Enrico> a trailing / is present, and the second patch makes this clear > Enrico> not only for cygwin. > > Why shall we assume that? I do not really understand this explicit > check for a trailing slash. I think it should be elsewhere. I saw that the [EMAIL PROTECTED] argument always has a double / appended. In my eyes this was not wrong because I remember that in kpathsea a double / signals to also search for subdirectories. Your post rang an alarm bell for me and I checked the [EMAIL PROTECTED] syntax and indeed it is not the same as for kpathsea. A single / should be appended to the path, but it seems that two do not hurt. Now, as a / was later added to the path, in my mind a / should have already been in the path. And indeed this is the case: whenever latex_path() is called to generate the argument of [EMAIL PROTECTED] the path which is passed always has a / appended. As I think that one should not fix what is not broken, in my patch I used that to distinguish when latex_path should generate the argument of [EMAIL PROTECTED] such that to act on this case only. Not only, as in cygwin external_path() can remove the trailing slash, I also paid attention to readd it... So I now know that the trailing slash is not there on purpose. However, it is a good flag for signaling latex_path() that a path for [EMAIL PROTECTED] is to be generated. Now I think I have two choices: 1) Find in the sources where latex_path() is called for [EMAIL PROTECTED], in order to enforce the trailing slash as a flag for this case, and then modify the patch such as to later remove it in latex_path(). 2) Check if the path passed to latex_path() is a directory. If it is, assume that this is the [EMAIL PROTECTED] case and act accordingly. What do you think is the better choice? -- Enrico
Re: A Cygwin related patch
Georg Baum <[EMAIL PROTECTED]> writes: > > Enrico Forestieri wrote: > > > Really? I had thought the second one was more cleaner. Actually I had to > > figure out that when latex_path() is called for generating the argument > > of \input path it is assumed that a trailing / is present, and the > > second patch makes this clear not only for cygwin. > > \input path must always be an absolute path (otherwise it would not > work TeX-wise), but why should that matter in LyX? Trailing, not heading /. But it seems I was misunderstanding the code. See my other post. > > On the other hand, > > external_path() is almost a no-op for the other OSes, so I don't it is > > such an overhead. So, what shall I use? > > The overhead does not matter at all here. We have had a lot of path problems > on the different windows flavors. Angus worked hard to resolve these, and > implemented the external/internal path distiction. > If we want to avoid more problems we must keep this distinction and be > always explicit whether a path is external or intrnal. > In the long run we should not juggle with paths as strings, but always use a > path class with external() and internal() methods. This will make the > distinction more clear. Ok > I am not really sure how things work on cygwin. Can we always assume that > all external paths are either windows or posix style? Or do we need posix > paths in some cases and windows paths in others? The latter. On cygwin, depending on a checkbox status, external_path() generates posix or windows style paths. This is because if you use MikTeX you need win-style paths. The configure script checks what kind of paths are needed and then sets the boolean \cygwin_path_fix_needed in lyxrc.defaults. But then you can override it by a checkbox (which, btw, is misnamed as it does the contrary of what its name implies). -- Enrico
Re: A Cygwin related patch
Enrico Forestieri wrote: > Trailing, not heading /. But it seems I was misunderstanding the code. And I was misunderstanding your post :-( >> I am not really sure how things work on cygwin. Can we always assume that >> all external paths are either windows or posix style? Or do we need posix >> paths in some cases and windows paths in others? > > The latter. On cygwin, depending on a checkbox status, external_path() > generates posix or windows style paths. This is because if you use > MikTeX you need win-style paths. The configure script checks what kind > of paths are needed and then sets the boolean \cygwin_path_fix_needed > in lyxrc.defaults. But then you can override it by a checkbox (which, > btw, is misnamed as it does the contrary of what its name implies). What I meant was: Do we need for one setting of cygwin_path_fix_needed both windos/posix styles, or does cygwin_path_fix_needed simply switch external_path() between the windows and posix version? Georg
Re: A Cygwin related patch
Georg Baum <[EMAIL PROTECTED]> writes: | Enrico Forestieri wrote: | | > The attached patch fixes the bug reported here: | > http://thread.gmane.org/gmane.editors.lyx.general/29227 | > | > In my intention this is the first of a series of patches aimed at | > polishing the Cygwin target, which I think is lagging behind. Please, tell | > me if this ok with you. I have no problem in maintaining the Cygwin target | > but my time is limited so I cannot guarantee a continuous commitment. I | > will do it time permitting. There should be no big problems, as I think | > the Cygwin users will not be more than those I can count with only one of | > my hands ;-) | | This patch looks good. I don't like the other one, because the code in | filetools.C is more complicated, and in this one it is more clear because | of cygwin_path_fix(). I feel the exact opposite. the cygwin_path_fix pollutes the "portable" code, pollutes the "not cygwin files" The second patch is the nice one IMO. -- Lgb
Re: A Cygwin related patch
Lars Gullik Bjønnes wrote: > I feel the exact opposite. the cygwin_path_fix pollutes the "portable" > code, pollutes the "not cygwin files" But it makes it explicit why external_path() is needed. > The second patch is the nice one IMO. Now I don't like either anymore. The problem of both fixes is that they use a trailing '/' as flag whether external_path() is needed or not. This will strike back sooner or later. Georg
Re: A Cygwin related patch
> "Georg" == Georg Baum <[EMAIL PROTECTED]> writes: Georg> Now I don't like either anymore. The problem of both fixes is Georg> that they use a trailing '/' as flag whether external_path() is Georg> needed or not. This will strike back sooner or later. Sure, this is not good. Is the problem that latex_path() needs different syntax depending on whether the file name is used inside a .tex file or as command argument? What is the problem exactly? JMarc
Re: A Cygwin related patch
Georg Baum <[EMAIL PROTECTED]> writes: > What I meant was: Do we need for one setting of cygwin_path_fix_needed both > windos/posix styles, or does cygwin_path_fix_needed simply switch > external_path() between the windows and posix version? As it is now, cygwin_path_fix_needed simply switch external_path() between the windows and posix version. However, if regarded as an OS, cygwin is really posix, so in some cases one must always use internal_path(). This is a singular case with respect to the other supported OSes. It may seem complicated, but apart the need for some polish, it does work. -- Enrico
Re: A Cygwin related patch
Jean-Marc Lasgouttes <[EMAIL PROTECTED]> writes:
>
> > "Georg" == Georg Baum <[EMAIL PROTECTED]> writes:
>
> Georg> Now I don't like either anymore. The problem of both fixes is
> Georg> that they use a trailing '/' as flag whether external_path() is
> Georg> needed or not. This will strike back sooner or later.
>
> Sure, this is not good. Is the problem that latex_path() needs
> different syntax depending on whether the file name is used inside a
> .tex file or as command argument?
>
> What is the problem exactly?
The problem is that I have not a full grasp of the overall code and don't
want introduce changes without fully knowing the consequences. I repeat,
my idea is of only fixing what is broken, making changes that I fully
understand. As a problem arised with [EMAIL PROTECTED], I tried to fix only
that case.
As regards the trailing '/', I think this is not a problem, as there is
only one place in the code where latex_path() is called for generating
the argument of [EMAIL PROTECTED] It is in src/buffer.C, so a patch in which
I am confident is along these lines:
--- src/buffer.C(revision 13450)
+++ src/buffer.C(working copy)
@@ -875,7 +876,9 @@ void Buffer::makeLaTeXFile(ostream & os,
texrow().newline();
}
if (!original_path.empty()) {
- string const inputpath = latex_path(original_path);
+ // A trailing slash is used as a flag for latex_path
+ // which will return an unslashed path.
+ string const inputpath = latex_path(original_path + "/")
;
os << "\\makeatletter\n"
<< "[EMAIL PROTECTED]"
<< inputpath << "/}}\n"
and then in latex_path(), either:
--- src/support/filetools.C (revision 13450)
+++ src/support/filetools.C (working copy)
@@ -87,6 +87,11 @@ string const latex_path(string const & o
latex_path_dots dots)
{
string path = subst(original_path, "\\", "/");
+ if (suffixIs(path, '/')) {
+ path = rtrim(path, "/");
+ if (os::cygwin_path_fix())
+ path = os::external_path(path);
+ }
path = subst(path, "~", "\\string~");
if (path.find(' ') != string::npos) {
// We can't use '"' because " is sometimes active (e.g. if
alongthe lines of the first patch, or:
--- src/support/filetools.C (revision 13450)
+++ src/support/filetools.C (working copy)
@@ -86,7 +86,15 @@ string const latex_path(string const & o
latex_path_extension extension,
latex_path_dots dots)
{
- string path = subst(original_path, "\\", "/");
+ string path;
+
+ if (suffixIs(original_path, '/')) {
+ path = rtrim(path, "/");
+ // The call to os::external_path is needed for Cygwin.
+ path = subst(os::external_path(original_path), "\\", "/");
+ } else
+ path = subst(original_path, "\\", "/");
+
path = subst(path, "~", "\\string~");
if (path.find(' ') != string::npos) {
// We can't use '"' because " is sometimes active (e.g. if
along the lines of the second patch. Both approaches work and are robust
because the trailing '/' is anyway added. Please, tell me what to do.
I do not feel confortable in unconditionally call external_path (I don't
want to fix what is not broken).
--
Enrico
Re: A Cygwin related patch
Enrico Forestieri wrote:
> The problem is that I have not a full grasp of the overall code and don't
> want introduce changes without fully knowing the consequences. I repeat,
> my idea is of only fixing what is broken, making changes that I fully
> understand. As a problem arised with [EMAIL PROTECTED], I tried to fix only
> that case.
I do fully agree. My point however is that using a trailing '/' as flag will
sooner or later strike back. If it is really necessary to tell latex_path()
whether it should call external_path() or not, add an additional parameter
that tells exactly that.
> --- src/support/filetools.C (revision 13450)
> +++ src/support/filetools.C (working copy)
> @@ -86,7 +86,15 @@ string const latex_path(string const & o
> latex_path_extension extension,
> latex_path_dots dots)
> {
> - string path = subst(original_path, "\\", "/");
> + string path;
> +
> + if (suffixIs(original_path, '/')) {
> + path = rtrim(path, "/");
> + // The call to os::external_path is needed for Cygwin.
> + path = subst(os::external_path(original_path), "\\", "/");
> + } else
> + path = subst(original_path, "\\", "/");
> +
> path = subst(path, "~", "\\string~");
> if (path.find(' ') != string::npos) {
> // We can't use '"' because " is sometimes active (e.g. if
>
>
> along the lines of the second patch. Both approaches work and are robust
> because the trailing '/' is anyway added. Please, tell me what to do.
This version is preferrable since it does not need the cygwin switch.
> I do not feel confortable in unconditionally call external_path (I don't
> want to fix what is not broken).
I think that you can safely call it unconditionally. I looked it up, and
latex_path() is only called on filenames that are written to .tex files. I
would be very surprised if a TeX compiler exists on windows that needs win
style paths in [EMAIL PROTECTED] but posix style paths in \includegraphics or
\input.
To be sure you could make a simple test: Call external_path()
unconditionally, and try to view a document that contains a graphics inset,
and two include insets, one with \input and one with \include. All files
should be entered with absolute names.
If my theory is right then running latex on the exported .tex file will work
with the unconditional fix and fail with the conditional one.
The absolute names are necessary to get slashes in the name, and running
latex on the exported .tex file is necessary because LyX uses always
relative paths for running latex internally.
Georg
PS: You might want to have a look at the cygwin version of external_path().
It contains unreachable code.
Re: A Cygwin related patch
Georg Baum <[EMAIL PROTECTED]> writes:
> I think that you can safely call it unconditionally. I looked it up, and
> latex_path() is only called on filenames that are written to .tex files. I
> would be very surprised if a TeX compiler exists on windows that needs win
> style paths in \inputa path but posix style paths in \includegraphics or
> \input.
> To be sure you could make a simple test: Call external_path()
> unconditionally, and try to view a document that contains a graphics inset,
> and two include insets, one with \input and one with \include. All files
> should be entered with absolute names.
> If my theory is right then running latex on the exported .tex file will work
> with the unconditional fix and fail with the conditional one.
> The absolute names are necessary to get slashes in the name, and running
> latex on the exported .tex file is necessary because LyX uses always
> relative paths for running latex internally.
Your theory is right and I wasn't fearless enough, not having the time
to study the code. So, the attached is apparently the right fix. While
testing this patch I run up against another bug.
1) File->New
2) Insert->File->Child Document (say we input ~/test.tex)
3) File->Export->LaTeX
LyX pops up a dialog telling me that ~/test.tex already exists and
asking for overwriting it. I say yes, overwrite it, but newfile1.lyx
is normally created and ~/test.tex is not overwritten. This is
independent from the fact that the buffer has been given a name or not.
This also happens on Solaris and I could not find a similar bug
reported in bugzilla.
> PS: You might want to have a look at the cygwin version of external_path().
> It contains unreachable code.
Uhm, I don't think so. The code after "else return p;" gets executed when
the previous if() clause is satisfied.
Many thanks for your helpful and appreciated comments.
--
Enrico
Index: src/support/filetools.C
===
--- src/support/filetools.C (revision 13463)
+++ src/support/filetools.C (working copy)
@@ -86,7 +86,9 @@ string const latex_path(string const & o
latex_path_extension extension,
latex_path_dots dots)
{
- string path = subst(original_path, "\\", "/");
+ // The call to os::external_path is only needed for Cygwin.
+ string path = subst(os::external_path(original_path), "\\", "/");
+ path = rtrim(path, "/");
path = subst(path, "~", "\\string~");
if (path.find(' ') != string::npos) {
// We can't use '"' because " is sometimes active (e.g. if
Re: A Cygwin related patch
Am Donnerstag, 23. März 2006 21:26 schrieb Enrico Forestieri:
> Your theory is right and I wasn't fearless enough, not having the time
> to study the code. So, the attached is apparently the right fix. While
> testing this patch I run up against another bug.
>
> 1) File->New
> 2) Insert->File->Child Document (say we input ~/test.tex)
> 3) File->Export->LaTeX
>
> LyX pops up a dialog telling me that ~/test.tex already exists and
> asking for overwriting it. I say yes, overwrite it, but newfile1.lyx
> is normally created and ~/test.tex is not overwritten. This is
> independent from the fact that the buffer has been given a name or not.
> This also happens on Solaris and I could not find a similar bug
> reported in bugzilla.
I will have a look.
> Uhm, I don't think so. The code after "else return p;" gets executed
when
> the previous if() clause is satisfied.
Oh yes, you are right.
> Many thanks for your helpful and appreciated comments.
I hope they are not too disappointing :-)
Unfortunately I have another one:
> Index: src/support/filetools.C
> ===
> --- src/support/filetools.C (revision 13463)
> +++ src/support/filetools.C (working copy)
> @@ -86,7 +86,9 @@ string const latex_path(string const & o
> latex_path_extension extension,
> latex_path_dots dots)
> {
> - string path = subst(original_path, "\\", "/");
> + // The call to os::external_path is only needed for Cygwin.
> + string path = subst(os::external_path(original_path), "\\", "/");
Here you basically revert external_path on non-cygwin windows. This is a
bit ugly, so I moved this into a new function
lyx::support::os::latex_path(). This is a noop except on cygwin, where it
is identical to external_path().
Shall I put this in?
Georg
Index: src/support/os_unix.C
===
--- src/support/os_unix.C (Revision 13467)
+++ src/support/os_unix.C (Arbeitskopie)
@@ -63,6 +63,12 @@ string internal_path(string const & p)
}
+string latex_path(string const & p)
+{
+ return p;
+}
+
+
bool is_absolute_path(string const & p)
{
return !p.empty() && p[0] == '/';
Index: src/support/os.h
===
--- src/support/os.h (Revision 13467)
+++ src/support/os.h (Arbeitskopie)
@@ -47,6 +47,14 @@ std::string external_path(std::string co
/// Converts a host OS style path to unix style.
std::string internal_path(std::string const & p);
+/**
+ * Converts a unix style path into a form suitable for inclusion in a LaTeX
+ * document.
+ * Caution: This function handles only the OS specific part of that task.
+ * Never use it directly, use lyx::support::latex_path instead.
+ */
+std::string latex_path(std::string const & p);
+
/// Is the path absolute?
bool is_absolute_path(std::string const & p);
Index: src/support/filetools.C
===
--- src/support/filetools.C (Revision 13467)
+++ src/support/filetools.C (Arbeitskopie)
@@ -87,6 +87,8 @@ string const latex_path(string const & o
latex_path_dots dots)
{
string path = subst(original_path, "\\", "/");
+ // On cygwin, we may need windows or posix style paths.
+ path = os::latex_path(path);
path = subst(path, "~", "\\string~");
if (path.find(' ') != string::npos) {
// We can't use '"' because " is sometimes active (e.g. if
Index: src/support/os_win32.C
===
--- src/support/os_win32.C (Revision 13467)
+++ src/support/os_win32.C (Arbeitskopie)
@@ -216,6 +216,12 @@ string internal_path(string const & p)
}
+string latex_path(string const & p)
+{
+ return p;
+}
+
+
// (Claus H.) On Win32 both Unix and Win32/DOS pathnames are used.
// Therefore an absolute path could be either a pathname starting
// with a slash (Unix) or a pathname starting with a drive letter
Index: src/support/os_cygwin.C
===
--- src/support/os_cygwin.C (Revision 13467)
+++ src/support/os_cygwin.C (Arbeitskopie)
@@ -102,6 +102,15 @@ string internal_path(string const & p)
}
+string latex_path(string const & p)
+{
+ // We may need a posix style path or a windows style path (depending
+ // on cygwin_path_fix_), but we use always forward slashes, since it
+ // gets written into a .tex file.
+ return external_path(p);
+}
+
+
bool is_absolute_path(string const & p)
{
if (p.empty())
Re: A Cygwin related patch
Georg Baum <[EMAIL PROTECTED]> writes: > > Many thanks for your helpful and appreciated comments. > > I hope they are not too disappointing Oh, not at all... > Here you basically revert external_path on non-cygwin windows. This is a > bit ugly, so I moved this into a new function > lyx::support::os::latex_path(). This is a noop except on cygwin, where it > is identical to external_path(). > Shall I put this in? It's very elegant. Please do. -- Enrico
A Cygwin related patch
The attached patch fixes the bug reported here:
http://thread.gmane.org/gmane.editors.lyx.general/29227
In my intention this is the first of a series of patches aimed at polishing
the Cygwin target, which I think is lagging behind. Please, tell me if this
ok with you. I have no problem in maintaining the Cygwin target but my time
is limited so I cannot guarantee a continuous commitment. I will do it time
permitting. There should be no big problems, as I think the Cygwin users
will not be more than those I can count with only one of my hands ;-)
Do you prefer that I file this one on bugzilla?
--
Enrico
Index: src/support/os_unix.C
===
--- src/support/os_unix.C (revision 13450)
+++ src/support/os_unix.C (working copy)
@@ -97,6 +97,12 @@ char path_separator()
void cygwin_path_fix(bool)
{}
+
+bool cygwin_path_fix()
+{
+ return false;
+}
+
} // namespace os
} // namespace support
} // namespace lyx
Index: src/support/os.h
===
--- src/support/os.h(revision 13450)
+++ src/support/os.h(working copy)
@@ -65,6 +65,7 @@ char path_separator();
* under Cygwin.
*/
void cygwin_path_fix(bool use_cygwin_paths);
+bool cygwin_path_fix();
} // namespace os
} // namespace support
Index: src/support/filetools.C
===
--- src/support/filetools.C (revision 13450)
+++ src/support/filetools.C (working copy)
@@ -86,7 +86,13 @@ string const latex_path(string const o
latex_path_extension extension,
latex_path_dots dots)
{
- string path = subst(original_path, \\, /);
+ string path;
+
+ if (suffixIs(original_path, '/') os::cygwin_path_fix())
+ path = os::external_path(original_path) + /;
+ else
+ path = subst(original_path, \\, /);
+
path = subst(path, ~, \\string~);
if (path.find(' ') != string::npos) {
// We can't use '' because is sometimes active (e.g. if
Index: src/support/os_win32.C
===
--- src/support/os_win32.C (revision 13450)
+++ src/support/os_win32.C (working copy)
@@ -265,6 +265,12 @@ void cygwin_path_fix(bool)
{}
+bool cygwin_path_fix()
+{
+ return false;
+}
+
+
namespace {
void bail_out()
Index: src/support/os_cygwin.C
===
--- src/support/os_cygwin.C (revision 13450)
+++ src/support/os_cygwin.C (working copy)
@@ -146,6 +146,12 @@ void cygwin_path_fix(bool use_cygwin_pat
cygwin_path_fix_ = use_cygwin_paths;
}
+
+bool cygwin_path_fix()
+{
+ return cygwin_path_fix_;
+}
+
} // namespace os
} // namespace support
} // namespace lyx
Index: src/support/os_os2.C
===
--- src/support/os_os2.C(revision 13450)
+++ src/support/os_os2.C(working copy)
@@ -212,6 +212,12 @@ char path_separator()
void cygwin_path_fix(bool)
{}
+
+bool cygwin_path_fix()
+{
+ return false;
+}
+
} // namespace os
} // namespace support
} // namespace lyx
Re: A Cygwin related patch
Enrico Forestieri [EMAIL PROTECTED] writes:
The attached patch fixes the bug reported here:
http://thread.gmane.org/gmane.editors.lyx.general/29227
In my intention this is the first of a series of patches aimed at polishing
the Cygwin target, which I think is lagging behind. Please, tell me if this
ok with you. I have no problem in maintaining the Cygwin target but my time
is limited so I cannot guarantee a continuous commitment. I will do it time
permitting. There should be no big problems, as I think the Cygwin users
will not be more than those I can count with only one of my hands
Do you prefer that I file this one on bugzilla?
Here is a perhaps less obtrusive patch.
--
Enrico
Index: src/support/filetools.C
===
--- src/support/filetools.C (revision 13450)
+++ src/support/filetools.C (working copy)
@@ -86,7 +86,16 @@ string const latex_path(string const o
latex_path_extension extension,
latex_path_dots dots)
{
- string path = subst(original_path, \\, /);
+ string path;
+
+ if (suffixIs(original_path, '/')) {
+ // The call to os::external_path is needed for Cygwin.
+ path = subst(os::external_path(original_path), \\, /);
+ if (!suffixIs(path, '/'))
+ path += /;
+ } else
+ path = subst(original_path, \\, /);
+
path = subst(path, ~, \\string~);
if (path.find(' ') != string::npos) {
// We can't use '' because is sometimes active (e.g. if
Re: A Cygwin related patch
On Wed, Mar 22, 2006 at 09:29:35PM +, Enrico Forestieri wrote: The attached patch fixes the bug reported here: http://thread.gmane.org/gmane.editors.lyx.general/29227 In my intention this is the first of a series of patches aimed at polishing the Cygwin target, which I think is lagging behind. Please, tell me if this ok with you. I have no problem in maintaining the Cygwin target but my time is limited so I cannot guarantee a continuous commitment. I will do it time permitting. There should be no big problems, as I think the Cygwin users will not be more than those I can count with only one of my hands ;-) You would be amazed by the true number of LyX/Windows users... Unfortunately I am unable / cannot be bothered to work on that, but I am really happy somebody does. LyX adoption benefits greatly by having the option of cross-platform collaborative authoring. And some people are stuck on Windows. BTW some of us are hit by symptoms like compulsive hand washing after touching Windows. Another reason for admiring you ;-) - Martin pgphtPh5qNEUY.pgp Description: PGP signature
A Cygwin related patch
The attached patch fixes the bug reported here:
http://thread.gmane.org/gmane.editors.lyx.general/29227
In my intention this is the first of a series of patches aimed at polishing
the Cygwin target, which I think is lagging behind. Please, tell me if this
ok with you. I have no problem in maintaining the Cygwin target but my time
is limited so I cannot guarantee a continuous commitment. I will do it time
permitting. There should be no big problems, as I think the Cygwin users
will not be more than those I can count with only one of my hands ;-)
Do you prefer that I file this one on bugzilla?
--
Enrico
Index: src/support/os_unix.C
===
--- src/support/os_unix.C (revision 13450)
+++ src/support/os_unix.C (working copy)
@@ -97,6 +97,12 @@ char path_separator()
void cygwin_path_fix(bool)
{}
+
+bool cygwin_path_fix()
+{
+ return false;
+}
+
} // namespace os
} // namespace support
} // namespace lyx
Index: src/support/os.h
===
--- src/support/os.h(revision 13450)
+++ src/support/os.h(working copy)
@@ -65,6 +65,7 @@ char path_separator();
* under Cygwin.
*/
void cygwin_path_fix(bool use_cygwin_paths);
+bool cygwin_path_fix();
} // namespace os
} // namespace support
Index: src/support/filetools.C
===
--- src/support/filetools.C (revision 13450)
+++ src/support/filetools.C (working copy)
@@ -86,7 +86,13 @@ string const latex_path(string const & o
latex_path_extension extension,
latex_path_dots dots)
{
- string path = subst(original_path, "\\", "/");
+ string path;
+
+ if (suffixIs(original_path, '/') && os::cygwin_path_fix())
+ path = os::external_path(original_path) + "/";
+ else
+ path = subst(original_path, "\\", "/");
+
path = subst(path, "~", "\\string~");
if (path.find(' ') != string::npos) {
// We can't use '"' because " is sometimes active (e.g. if
Index: src/support/os_win32.C
===
--- src/support/os_win32.C (revision 13450)
+++ src/support/os_win32.C (working copy)
@@ -265,6 +265,12 @@ void cygwin_path_fix(bool)
{}
+bool cygwin_path_fix()
+{
+ return false;
+}
+
+
namespace {
void bail_out()
Index: src/support/os_cygwin.C
===
--- src/support/os_cygwin.C (revision 13450)
+++ src/support/os_cygwin.C (working copy)
@@ -146,6 +146,12 @@ void cygwin_path_fix(bool use_cygwin_pat
cygwin_path_fix_ = use_cygwin_paths;
}
+
+bool cygwin_path_fix()
+{
+ return cygwin_path_fix_;
+}
+
} // namespace os
} // namespace support
} // namespace lyx
Index: src/support/os_os2.C
===
--- src/support/os_os2.C(revision 13450)
+++ src/support/os_os2.C(working copy)
@@ -212,6 +212,12 @@ char path_separator()
void cygwin_path_fix(bool)
{}
+
+bool cygwin_path_fix()
+{
+ return false;
+}
+
} // namespace os
} // namespace support
} // namespace lyx
Re: A Cygwin related patch
Enrico Forestieri <[EMAIL PROTECTED]> writes:
>
> The attached patch fixes the bug reported here:
> http://thread.gmane.org/gmane.editors.lyx.general/29227
>
> In my intention this is the first of a series of patches aimed at polishing
> the Cygwin target, which I think is lagging behind. Please, tell me if this
> ok with you. I have no problem in maintaining the Cygwin target but my time
> is limited so I cannot guarantee a continuous commitment. I will do it time
> permitting. There should be no big problems, as I think the Cygwin users
> will not be more than those I can count with only one of my hands
>
> Do you prefer that I file this one on bugzilla?
Here is a perhaps less obtrusive patch.
--
Enrico
Index: src/support/filetools.C
===
--- src/support/filetools.C (revision 13450)
+++ src/support/filetools.C (working copy)
@@ -86,7 +86,16 @@ string const latex_path(string const & o
latex_path_extension extension,
latex_path_dots dots)
{
- string path = subst(original_path, "\\", "/");
+ string path;
+
+ if (suffixIs(original_path, '/')) {
+ // The call to os::external_path is needed for Cygwin.
+ path = subst(os::external_path(original_path), "\\", "/");
+ if (!suffixIs(path, '/'))
+ path += "/";
+ } else
+ path = subst(original_path, "\\", "/");
+
path = subst(path, "~", "\\string~");
if (path.find(' ') != string::npos) {
// We can't use '"' because " is sometimes active (e.g. if
Re: A Cygwin related patch
On Wed, Mar 22, 2006 at 09:29:35PM +, Enrico Forestieri wrote: > The attached patch fixes the bug reported here: > http://thread.gmane.org/gmane.editors.lyx.general/29227 > > In my intention this is the first of a series of patches aimed at polishing > the Cygwin target, which I think is lagging behind. Please, tell me if this > ok with you. I have no problem in maintaining the Cygwin target but my time > is limited so I cannot guarantee a continuous commitment. I will do it time > permitting. There should be no big problems, as I think the Cygwin users > will not be more than those I can count with only one of my hands ;-) You would be amazed by the true number of LyX/Windows users... Unfortunately I am unable / cannot be bothered to work on that, but I am really happy somebody does. LyX adoption benefits greatly by having the option of cross-platform collaborative authoring. And some people are stuck on Windows. BTW some of us are hit by symptoms like compulsive hand washing after touching Windows. Another reason for admiring you ;-) - Martin pgphtPh5qNEUY.pgp Description: PGP signature
