Problems with Copy and line endings on GTK/Win32
Enrico Tröger
an user of Geany mentioned an interesting problem:
when Scintilla is built with GTK on Windows(PLAT_GTK and
PLAT_GTK_WIN32) and you have a file opened with usual CR/LF line
endings, then select some text and copy it to the clipboard, paste it
somewhere else, e.g. in Windows Notepad and then save the file with
Notepad, it contains wrong line endings: 0x0d 0x0d 0x0a. The correct
sequence would be 0x0d 0x0a.
I tried this also with SciTE built on Windows using GTK and it happens
also with SciTE.
It works well when the copied text has Unix line endings(only \n).
In ScintillaGTK.cxx's GetSelection() the first #ifdef block seems to be
the related code for that. What seems strange to me, the comment says
that native Windows applications expect '\n', i.e. the common Unix line
endings. Is this really correct? Don't they expect \r\n line endings as
it is the default on Windows?
For testing purposes, I changed the line ending format in the following
TransformLineEnds() call from SC_EOL_LF to SC_EOL_CRLF and this seems
to do the trick. At least, with that change I can copy \n and \r\n
ended files to Notepad and they are always saved correctly with \r\n.
I'm not completely sure about any side-effects or other applications
which might don't work with that change. I only tested it with Notepad.
Regards,
Enrico
--
Get my GPG key from http://www.uvena.de/pub.key
Neil Hodgson
> In ScintillaGTK.cxx's GetSelection() the first #ifdef block seems to be
> the related code for that. What seems strange to me, the comment says
> that native Windows applications expect '\n', i.e. the common Unix line
> endings. Is this really correct? Don't they expect \r\n line endings as
> it is the default on Windows?
It is quite deliberate as is apparent from this message:
http://www.mail-archive.com/scintilla...@lyra.org/msg01624.html
I was hoping John Ehresman or another GTK+ on Windows developer
would comment as I don't work on this configuration myself.
Neil
Enrico Tröger
<nyama...@gmail.com> wrote:
>
> Enrico Tröger:
>
> > In ScintillaGTK.cxx's GetSelection() the first #ifdef block seems to be
> > the related code for that. What seems strange to me, the comment says
> > that native Windows applications expect '\n', i.e. the common Unix line
> > endings. Is this really correct? Don't they expect \r\n line endings as
> > it is the default on Windows?
>
> It is quite deliberate as is apparent from this message:
> http://www.mail-archive.com/scintilla...@lyra.org/msg01624.html
> I was hoping John Ehresman or another GTK+ on Windows developer
I hope so, too.
It seems very strange to me, I can't see a reason why converting the
line endings to \n instead of \r\n.
jpe
>>> In ScintillaGTK.cxx's GetSelection() the first #ifdef block seems to be
>>> the related code for that. What seems strange to me, the comment says
>>> that native Windows applications expect '\n', i.e. the common Unix line
>>> endings. Is this really correct? Don't they expect \r\n line endings as
>>> it is the default on Windows?
>> It is quite deliberate as is apparent from this message:
>> http://www.mail-archive.com/scintilla...@lyra.org/msg01624.html
>> I was hoping John Ehresman or another GTK+ on Windows developer
> I hope so, too.
> It seems very strange to me, I can't see a reason why converting the
> line endings to \n instead of \r\n.
into \r\n so the Scintilla code converts to \n to compensate for
this.
If the Scintilla code is removed, \r\r\n is put on the clipboard for
each \r\n eol. The gtk code that does the \n -> \r\n expansion is in
gdkproperty-win32.c's gdk_property_change function.
This means that the comment in ScintillaGTK.cxx's GetSelection() is
wrong. It's gtk, not native win32 programs, that needs the eol to be
\n. I suspect I wrote the comment before I fully understood what the
problem was.
Cheers,
John
Enrico Tröger
>
> Enrico Tröger wrote:
> >>> In ScintillaGTK.cxx's GetSelection() the first #ifdef block seems to be
> >>> the related code for that. What seems strange to me, the comment says
> >>> that native Windows applications expect '\n', i.e. the common Unix line
> >>> endings. Is this really correct? Don't they expect \r\n line endings as
> >>> it is the default on Windows?
> >> It is quite deliberate as is apparent from this message:
> >> http://www.mail-archive.com/scintilla...@lyra.org/msg01624.html
> >> I was hoping John Ehresman or another GTK+ on Windows developer
> > I hope so, too.
> > It seems very strange to me, I can't see a reason why converting the
> > line endings to \n instead of \r\n.
>
> Sorry for the delay in responding. It looks like gtk expands any \n
No problem.
> into \r\n so the Scintilla code converts to \n to compensate for
> this.
> If the Scintilla code is removed, \r\r\n is put on the clipboard for
> each \r\n eol. The gtk code that does the \n -> \r\n expansion is in
> gdkproperty-win32.c's gdk_property_change function.
Yes, after some more testing, I agree. I'm not sure why I thought
changing the code to converting to SC_EOL_CRLF helps, it doesn't make
any sense. But I found the real cause for my problem:
When building Scintilla on Windows with GTK and with gcc(mingw),
PLAT_GTK_WIN32 is set to 0. But it needs to be set to 1.
There are two possible solutions I think:
a) in include/Platform.h where PLAT_GTK_WIN32 initially is set to 0, add
an #ifndef to only set it to 0, if it isn't already defined, something
like this:
#ifndef PLAT_GTK_WIN32
#definePLAT_GTK_WIN32 0
#endif
In this way, developers who know they are building on Win32, GTK and
gcc they can set PLAT_GTK_WIN32 to 1 by themselves before including
Scintilla.h.
b) in include/Platform.h extend the check
#ifdef _MSC_VER
#undef PLAT_GTK_WIN32
#define PLAT_GTK_WIN32 1
#endif
to not only check for MSVC but also for mingw-gcc. I'm not sure at the
moment how to check for ming-gcc, but I think there is a macro to test
it.
What do you think?
> This means that the comment in ScintillaGTK.cxx's GetSelection() is
> wrong. It's gtk, not native win32 programs, that needs the eol to be
> \n. I suspect I wrote the comment before I fully understood what the
So, it would be cool if the comment should be fixed ;-).
Enrico Tröger
<enrico....@uvena.de> wrote:
> On Mon, 4 Feb 2008 08:38:55 -0800 (PST), jpe <j...@wingware.com> wrote:
> There are two possible solutions I think:
> a) in include/Platform.h where PLAT_GTK_WIN32 initially is set to 0, add
> an #ifndef to only set it to 0, if it isn't already defined, something
> like this:
> #ifndef PLAT_GTK_WIN32
> #definePLAT_GTK_WIN32 0
> #endif
> In this way, developers who know they are building on Win32, GTK and
> gcc they can set PLAT_GTK_WIN32 to 1 by themselves before including
> Scintilla.h.
> b) in include/Platform.h extend the check
> #ifdef _MSC_VER
> #undef PLAT_GTK_WIN32
> #define PLAT_GTK_WIN32 1
> #endif
> to not only check for MSVC but also for mingw-gcc. I'm not sure at the
> moment how to check for ming-gcc, but I think there is a macro to test
> it.
Maybe the better solution than checking for MSVC or mingw-gcc, check
for G_OS_WIN32 which is set by Glib/GTK on Win32 platforms:
#ifdef G_OS_WIN32
#undef PLAT_GTK_WIN32
#define PLAT_GTK_WIN32 1
#endif
But this requires that gtk.h or glib.h is always included before
Platform.h. This is true for ScintillaGTK.cxx but I didn't check other
files.
John Ehresman
> Maybe the better solution than checking for MSVC or mingw-gcc, check
> for G_OS_WIN32 which is set by Glib/GTK on Win32 platforms:
> #ifdef G_OS_WIN32
> #undef PLAT_GTK_WIN32
> #define PLAT_GTK_WIN32 1
> #endif
> But this requires that gtk.h or glib.h is always included before
> Platform.h. This is true for ScintillaGTK.cxx but I didn't check other
> files.
This would mean that PLAT_GTK_WIN32 would be 0 in files where glib.h
wasn't included, which is probably not the best solution even if it did
work. Could G_OS_WIN32 be used everywhere PLAT_GTK_WIN32 currently is used?
This could be a problem with gtk 1.x; I don't remember if G_OS_WIN32 was
defined or not. I don't think there's a need to support gtk 1.x on
win32 so the question would be whether changing this would affect gtk
1.x support on non-win32 platforms.
Cheers,
John
Enrico Tröger
wrote:
>
> Enrico Tröger wrote:
> > Maybe the better solution than checking for MSVC or mingw-gcc, check
> > for G_OS_WIN32 which is set by Glib/GTK on Win32 platforms:
> > #ifdef G_OS_WIN32
> > #undef PLAT_GTK_WIN32
> > #define PLAT_GTK_WIN32 1
> > #endif
> > But this requires that gtk.h or glib.h is always included before
> > Platform.h. This is true for ScintillaGTK.cxx but I didn't check other
> > files.
>
> This would mean that PLAT_GTK_WIN32 would be 0 in files where glib.h
> wasn't included, which is probably not the best solution even if it did
> work. Could G_OS_WIN32 be used everywhere PLAT_GTK_WIN32 currently is used?
After grepping the sources, I noticed PLAT_GTK_WIN32 is only used in
ScintillaGTK.cxx. In Platform.h it is only defined, not used. So, the
inclusion of gtk.h/glib.h wouldn't be a problem because these are
included in ScintilalGTK.cxx. I'm not sure about the Gtk 1.x issue.
Maybe solution a) is the best one to not break anything because it
requires direct work of the developer who uses Scintilla.
John Ehresman
>> This would mean that PLAT_GTK_WIN32 would be 0 in files where glib.h
>> wasn't included, which is probably not the best solution even if it did
>> work. Could G_OS_WIN32 be used everywhere PLAT_GTK_WIN32 currently is used?
>
> After grepping the sources, I noticed PLAT_GTK_WIN32 is only used in
> ScintillaGTK.cxx. In Platform.h it is only defined, not used. So, the
> inclusion of gtk.h/glib.h wouldn't be a problem because these are
> included in ScintilalGTK.cxx. I'm not sure about the Gtk 1.x issue.
> Maybe solution a) is the best one to not break anything because it
> requires direct work of the developer who uses Scintilla.
I'm not sure what solution a) is, but having PLAT_GTK_WIN32 be 1 in
ScintillaGTK.cxx and 0 or 1 elsewhere (depending on the order of include
files) could very easily cause problems when someone uses the symbol in
another file. Using G_OS_WIN32 or coming up with a better way to set
PLAT_GTK_WIN32 seems like better options to me.
Cheers,
John
Neil Hodgson
> But this requires that gtk.h or glib.h is always included before
> Platform.h. This is true for ScintillaGTK.cxx but I didn't check other
> files.
As far as possible, code is protected from system headers like
gtk.h to avoid unintentional dependencies, spurious matching of
symbols, and lengthy compile times. If you can extend the #ifdef
_MSC_VER to include mingw then that is easy. Only files in
scintilla/gtk should need to distinguish these cases.
Neil
John Ehresman
> <nyama...@gmail.com> wrote:
>> As far as possible, code is protected from system headers like
>> gtk.h to avoid unintentional dependencies, spurious matching of
>> symbols, and lengthy compile times. If you can extend the #ifdef
>> _MSC_VER to include mingw then that is easy. Only files in
>> scintilla/gtk should need to distinguish these cases.
> I didn't find anything mingw specific, but
> #if __WIN32__ || _MSC_VER
> seems to be enough. __WIN32__ is defined by the mingw-gcc and I guess
> also by the MSVC(I don't have any MSVC version so I couldn't verify it).
You may want to use #if defined(__WIN32__) || defined(_MSC_VER) to
handle the case where __WIN32__ is not defined, or maybe not. I must
admit the #if NAME form of macros often confuse me.
Do you have a patch for this?
Thanks,
John
Enrico Tröger
wrote:
>
Yes, this is indeed better and cleaner.
> Do you have a patch for this?
For this single line change?
John Ehresman
>> Do you have a patch for this?
> For this single line change?
It's up to Neil since he's the maintainer, but in general patches make
it a bit easier for maintainers to know exactly what changes you want
made and the less work there is for the maintainer, the more likely your
contribution will be accepted. Alternately, you could clearly state
what the modified line should be and where it is located.
Cheers,
John
Enrico Tröger
wrote:
>
> Enrico Tröger wrote:
> >> Do you have a patch for this?
> > For this single line change?
>
> It's up to Neil since he's the maintainer, but in general patches make
> it a bit easier for maintainers to know exactly what changes you want
> made and the less work there is for the maintainer, the more likely your
Ok, here we go.
Neil Hodgson
> Ok, here we go.
OK, committed.
Neil
Enrico Tröger
Cool, thanks.
Maybe you could also correct John's comment in gtk/ScintillaGTK.cxx
about the line ending conversion for copy on Win32/GTK to avoid further
confusion (see attached patch as a suggestion).
Neil Hodgson
> Maybe you could also correct John's comment in gtk/ScintillaGTK.cxx
> about the line ending conversion for copy on Win32/GTK to avoid further
> confusion (see attached patch as a suggestion).
OK, committed.
Neil