Re: [Geany-devel] Split Window Patches - Scintilla C++0x
On 03/31/11 10:34, Enrico Tröger wrote: Ok, then we could update our Scintilla copy soon, either with the current release and patch Matthew's patches in or wait for the next release which includes the patches. FWIW, I finally got Geany compiling on Windows XP. Even without the patches, using GTK+ 2.22, there is no problem with Split Window plugin here[1]. I guess patches applied would be good for older GTK+ versions. [1] http://www.codebrainz.ca/images/screens/geany-splitwindow-xp.png Cheers, Matthew Brush ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Split Window Patches - Scintilla C++0x
On Thu, 31 Mar 2011 09:17:26 +1100 Lex Trotman ele...@gmail.com wrote: Currently there is the problem with dropped support for old C++ compilers which may not affect us directly but more over the introduction of new C++0x code. For details see http://groups.google.com/group/scintilla-interest/browse_thread/thread/8f9c4aae5f5c5368/6ecc660f51cecefc. Though Scintilla's release notes state that the Borland C++ support was removed, I'm not sure if the new C++0x code is already in. Nick, Lex, do you know more about the current status? Good question, thanks Enrico for asking it :) Actually there is another patch that was applied to upstream Sinctilla I would like to see in our copy (related to Vala highlighting), so I wondered if we didn't wanted to update our scintilla copy at some point... but there is the C++0x thing. Well Scintilla 2.25 does not use the std=c++0x option and it does use -Wall which includes a c++0x warning so I would say its reasonably safe. Yes, and I'd expect Neil to mention it in the release notes if the requirements change. Now that C++0x has been finalised as C++ 2011 this might encourage him :-/ Regards, Nick ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Split Window Patches - Scintilla C++0x
On Thu, 31 Mar 2011 16:56:44 +0100, Nick wrote: On Thu, 31 Mar 2011 09:17:26 +1100 Lex Trotman ele...@gmail.com wrote: Currently there is the problem with dropped support for old C++ compilers which may not affect us directly but more over the introduction of new C++0x code. For details see http://groups.google.com/group/scintilla-interest/browse_thread/thread/8f9c4aae5f5c5368/6ecc660f51cecefc. Though Scintilla's release notes state that the Borland C++ support was removed, I'm not sure if the new C++0x code is already in. Nick, Lex, do you know more about the current status? Good question, thanks Enrico for asking it :) Actually there is another patch that was applied to upstream Sinctilla I would like to see in our copy (related to Vala highlighting), so I wondered if we didn't wanted to update our scintilla copy at some point... but there is the C++0x thing. Well Scintilla 2.25 does not use the std=c++0x option and it does use -Wall which includes a c++0x warning so I would say its reasonably safe. Yes, and I'd expect Neil to mention it in the release notes if the requirements change. Now that C++0x has been finalised as C++ 2011 this might encourage him :-/ Ok, then we could update our Scintilla copy soon, either with the current release and patch Matthew's patches in or wait for the next release which includes the patches. Regards, Enrico -- Get my GPG key from http://www.uvena.de/pub.asc pgps794wuBiMm.pgp Description: PGP signature ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Split Window Patches
On Tue, 29 Mar 2011 18:39:37 -0700 Matthew Brush mbr...@codebrainz.ca wrote: The patches were committed[1][2]. So now, can we patch our copy of Scintilla, or do we need to wait until their next release and then until we update to that version? We can patch our copy now. I'm pretty confident those patches will apply cleanly on versions between ours and what they just got committed to, though I don't really understand Scintilla versions and how/when we update. We update soon after a Scintilla release unless it's a major Scintilla release that may introduce bugs. Regards, Nick ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Split Window Patches
On Wed, 30 Mar 2011 13:03:33 +0100, Nick wrote: On Tue, 29 Mar 2011 18:39:37 -0700 Matthew Brush mbr...@codebrainz.ca wrote: The patches were committed[1][2]. Cool! (btw, I also didn't get the mails from the SF tracker although I'm monitoring it, also not all of these mails didn't arrive on the scintilla list, so I'd say also it's a problem at SF) So now, can we patch our copy of Scintilla, or do we need to wait until their next release and then until we update to that version? We can patch our copy now. I'm pretty confident those patches will apply cleanly on versions between ours and what they just got committed to, though I don't really understand Scintilla versions and how/when we update. We update soon after a Scintilla release unless it's a major Scintilla release that may introduce bugs. Currently there is the problem with dropped support for old C++ compilers which may not affect us directly but more over the introduction of new C++0x code. For details see http://groups.google.com/group/scintilla-interest/browse_thread/thread/8f9c4aae5f5c5368/6ecc660f51cecefc. Though Scintilla's release notes state that the Borland C++ support was removed, I'm not sure if the new C++0x code is already in. Nick, Lex, do you know more about the current status? Regards, Enrico -- Get my GPG key from http://www.uvena.de/pub.asc pgp57v49xj5At.pgp Description: PGP signature ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Split Window Patches
Le 30/03/2011 22:59, Enrico Tröger a écrit : On Wed, 30 Mar 2011 13:03:33 +0100, Nick wrote: [...] We update soon after a Scintilla release unless it's a major Scintilla release that may introduce bugs. Currently there is the problem with dropped support for old C++ compilers which may not affect us directly but more over the introduction of new C++0x code. For details see http://groups.google.com/group/scintilla-interest/browse_thread/thread/8f9c4aae5f5c5368/6ecc660f51cecefc. Though Scintilla's release notes state that the Borland C++ support was removed, I'm not sure if the new C++0x code is already in. Nick, Lex, do you know more about the current status? Good question, thanks Enrico for asking it :) Actually there is another patch that was applied to upstream Sinctilla I would like to see in our copy (related to Vala highlighting), so I wondered if we didn't wanted to update our scintilla copy at some point... but there is the C++0x thing. Cheers, Colomban ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Split Window Patches
Currently there is the problem with dropped support for old C++ compilers which may not affect us directly but more over the introduction of new C++0x code. For details see http://groups.google.com/group/scintilla-interest/browse_thread/thread/8f9c4aae5f5c5368/6ecc660f51cecefc. Though Scintilla's release notes state that the Borland C++ support was removed, I'm not sure if the new C++0x code is already in. Nick, Lex, do you know more about the current status? Good question, thanks Enrico for asking it :) Actually there is another patch that was applied to upstream Sinctilla I would like to see in our copy (related to Vala highlighting), so I wondered if we didn't wanted to update our scintilla copy at some point... but there is the C++0x thing. Well Scintilla 2.25 does not use the std=c++0x option and it does use -Wall which includes a c++0x warning so I would say its reasonably safe. Cheers Lex ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Split Window Patches
On 03/30/11 05:03, Nick Treleaven wrote: On Tue, 29 Mar 2011 18:39:37 -0700 Matthew Brushmbr...@codebrainz.ca wrote: The patches were committed[1][2]. So now, can we patch our copy of Scintilla, or do we need to wait until their next release and then until we update to that version? We can patch our copy now. Now that we have all our ducks in a row, I've attached two patches to re-add Windows support to the Split Window plugin. #0001 Applies the patches merged upstream and puts copies of the two patches in the scintilla/ directory (not sure if this is needed). #0002 The patch that was applied and removed previously on SVN, but I recommitted on my local (up to date) Git copy and re-worded the commit message a bit. Probably any of the older versions of this patch will work just the same. In the meantime, I'll keep testing...at least in Linux. Cheers, Matthew Brush From 4711211ba80a6d88f5bf8a4be8b2216df275d1b2 Mon Sep 17 00:00:00 2001 From: Matthew Brush matthewbr...@gmail.com Date: Wed, 30 Mar 2011 19:48:02 -0700 Subject: [PATCH 1/2] Apply patches needed to support re-releazing Scintilla widgets. Patches needed to support unrealizing and re-realizing Scintilla widgets in the Split Window plugin. These changes were merged upstream and will not be required in the future. --- .../0001-Fix-PRIMARY-selection-on-realize.patch| 70 scintilla/0002-Fix-cursors-on-realize.patch| 40 +++ scintilla/gtk/ScintillaGTK.cxx | 36 -- 3 files changed, 138 insertions(+), 8 deletions(-) create mode 100644 scintilla/0001-Fix-PRIMARY-selection-on-realize.patch create mode 100644 scintilla/0002-Fix-cursors-on-realize.patch diff --git a/scintilla/0001-Fix-PRIMARY-selection-on-realize.patch b/scintilla/0001-Fix-PRIMARY-selection-on-realize.patch new file mode 100644 index 000..fe8a1e3 --- /dev/null +++ b/scintilla/0001-Fix-PRIMARY-selection-on-realize.patch @@ -0,0 +1,70 @@ +# HG changeset patch +# User Matthew Brush matthewbr...@gmail.com +# Date 1301362777 25200 +# Node ID f4b6785eea540df855dda717f1886d59758d1cc3 +# Parent b4b219312338521cd5afd3f92c2cb314cd498727 +Fix X PRIMARY selection issue when Scintilla widget is unrealized/re-realized. + +When the widget is unrealized (for ex. with gtk_container_remove), the +GdkWindows used in the widget are destroyed and when the widget is realized +again (for ex. with gtk_container_add), the GdkWindows are re-created. This +commit moves the gtk_selection_add_targets() calls into the RealizeThis +function, so that when the widget is realized again, the selection targets +are re-added on the new GdkWindow. + +Also add gtk_selection_clear_targets() into UnRealizeThis to remove the +registered targets before the GdkWindow is destroyed. + +References: +http://mail.gnome.org/archives/gtk-devel-list/2002-March/msg00078.html +http://mail.gnome.org/archives/anjuta-devel-list/2002-March/msg00066.html +http://git.geany.org/geany/tree/plugins/splitwindow.c#n310 + +diff -r b4b219312338 -r f4b6785eea54 gtk/ScintillaGTK.cxx +--- a/gtk/ScintillaGTK.cxx Sun Mar 27 12:57:55 2011 +1100 b/gtk/ScintillaGTK.cxx Mon Mar 28 18:39:37 2011 -0700 +@@ -420,6 +420,14 @@ + gtk_widget_realize(widtxt); + gtk_widget_realize(PWidget(scrollbarv)); + gtk_widget_realize(PWidget(scrollbarh)); ++ ++ gtk_selection_add_targets(widget, GDK_SELECTION_PRIMARY, ++ clipboardCopyTargets, nClipboardCopyTargets); ++ ++#ifndef USE_GTK_CLIPBOARD ++ gtk_selection_add_targets(widget, atomClipboard, ++ clipboardPasteTargets, nClipboardPasteTargets); ++#endif + } + + void ScintillaGTK::Realize(GtkWidget *widget) { +@@ -429,6 +437,14 @@ + + void ScintillaGTK::UnRealizeThis(GtkWidget *widget) { + try { ++ ++ gtk_selection_clear_targets(widget, GDK_SELECTION_PRIMARY); ++ ++#ifndef USE_GTK_CLIPBOARD ++ gtk_selection_clear_targets(widget, atomClipboard); ++#endif ++ ++ + if (IS_WIDGET_MAPPED(widget)) { + gtk_widget_unmap(widget); + } +@@ -670,14 +686,6 @@ + + gtk_widget_grab_focus(PWidget(wMain)); + +- gtk_selection_add_targets(GTK_WIDGET(PWidget(wMain)), GDK_SELECTION_PRIMARY, +- clipboardCopyTargets, nClipboardCopyTargets); +- +-#ifndef USE_GTK_CLIPBOARD +- gtk_selection_add_targets(GTK_WIDGET(PWidget(wMain)), atomClipboard, +- clipboardPasteTargets, nClipboardPasteTargets); +-#endif +- + gtk_drag_dest_set(GTK_WIDGET(PWidget(wMain)), + GTK_DEST_DEFAULT_ALL, clipboardPasteTargets, nClipboardPasteTargets, + static_castGdkDragAction(GDK_ACTION_COPY | GDK_ACTION_MOVE)); diff --git a/scintilla/0002-Fix-cursors-on-realize.patch b/scintilla/0002-Fix-cursors-on-realize.patch new file mode 100644 index 000..1502484 --- /dev/null +++ b/scintilla/0002-Fix-cursors-on-realize.patch @@ -0,0 +1,40 @@ +# HG changeset patch +# User Matthew Brush matthewbr...@gmail.com +# Date 1301379167
Re: [Geany-devel] Split Window Patches
On 03/28/11 11:49, Enrico Tröger wrote: On Mon, 28 Mar 2011 21:38:22 +1100, Lex wrote: I havn't had a chance to try it, but perhaps you should report the problem to Scintilla and submit the two patches to Neil. In general Geany tries to use an unmodified version of Scintilla so that changes don't have to be made each time its updated. Since this is to fix a plugin problem not a core problem (albeit a core plugin) I would suggest waiting for the response from Scintilla rather than including patches in the Geany Scintilla. What do others think? The same. First, the Scintilla patches should be shared with upstream, ideally they apply them. Then we can patch Geany. Done. I re-did the patches proper against Scintilla's Mercurial repository and posted a bug report on Source Forge[1]. [1] https://sourceforge.net/tracker/?func=detailaid=3256153group_id=2439atid=102439 Cheers, Matthew Brush ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Split Window Patches
On Mon, 28 Mar 2011 23:44:42 -0700 Matthew Brush mbr...@codebrainz.ca wrote: First, the Scintilla patches should be shared with upstream, ideally they apply them. Then we can patch Geany. Done. I re-did the patches proper against Scintilla's Mercurial repository and posted a bug report on Source Forge[1]. [1] https://sourceforge.net/tracker/?func=detailaid=3256153group_id=2439atid=102439 Great, thanks. I added a comment to explain the plugin a bit more. Regards, Nick ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Split Window Patches
On 03/29/11 06:03, Nick Treleaven wrote: On Mon, 28 Mar 2011 23:44:42 -0700 Matthew Brushmbr...@codebrainz.ca wrote: First, the Scintilla patches should be shared with upstream, ideally they apply them. Then we can patch Geany. Done. I re-did the patches proper against Scintilla's Mercurial repository and posted a bug report on Source Forge[1]. [1] https://sourceforge.net/tracker/?func=detailaid=3256153group_id=2439atid=102439 Great, thanks. I added a comment to explain the plugin a bit more. The patches were committed[1][2]. So now, can we patch our copy of Scintilla, or do we need to wait until their next release and then until we update to that version? I'm pretty confident those patches will apply cleanly on versions between ours and what they just got committed to, though I don't really understand Scintilla versions and how/when we update. [1] http://scintilla.hg.sourceforge.net/hgweb/scintilla/scintilla/rev/e88a8a880c96 [2] http://scintilla.hg.sourceforge.net/hgweb/scintilla/scintilla/rev/874b84aa77b3 Cheers, Matthew Brush ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Split Window Patches
On 03/27/11 09:11, Colomban Wendling wrote: We need a better fix then. Maybe you can try to find out why the X clipboard get broken on Scintilla (and fix it :D). If it get fixed, we will probably can re-apply the patch. When the ScintillaObject gets reparented (unrealized/re-realized), it doesn't re-setup the selection targets again (this is only done in ScintillaGTK::Initialize()). The first patch #0001 is the same as the previous #0003 patch, which removes the reparenting stuff, to make Split Window work on Windows. The second patch #0002 fixes the primary selection issue by moving gtk_selection_add_targets() and friends into the ScintillaGTK::RealizeThis() function, and also adds its counter-parts to the ScintillaGTK::UnRealizeThis() function. The third patch #0003 fixes the issue where the I-Beam cursor is displayed even for the scrollbars when the widget is unrealized/re-realized. I tried a few different ways to do this, including trying to use Scintilla's SetCursor() function, but nothing seemed to work. Everything *seems* to work now, and it's not hacky, IMO - except maybe #0003 a little bit ;). Cheers, Matthew Brush From 1eb6bc349514ef4446e03b61d9661e33c7cf9499 Mon Sep 17 00:00:00 2001 From: Matthew Brush matthewbr...@gmail.com Date: Wed, 16 Mar 2011 14:33:40 -0700 Subject: [PATCH 3/5] Remove widget reparenting in Split Window plugin. Instead of reparenting the documents notebook full of ScintillaObjects, just ref it, remove it from the old parent, add it to the new parent, and then unref it. This might fix issues with Split Window plugin on Windows and seems to have no issues on in Linux. --- plugins/splitwindow.c | 21 ++--- 1 files changed, 10 insertions(+), 11 deletions(-) diff --git a/plugins/splitwindow.c b/plugins/splitwindow.c index e357198..9117bf4 100644 --- a/plugins/splitwindow.c +++ b/plugins/splitwindow.c @@ -298,7 +298,6 @@ static GtkWidget *create_toolbar(void) return toolbar; } - static void split_view(gboolean horizontal) { GtkWidget *notebook = geany_data-main_widgets-notebook; @@ -313,14 +312,14 @@ static void split_view(gboolean horizontal) set_state(horizontal ? STATE_SPLIT_HORIZONTAL : STATE_SPLIT_VERTICAL); - /* temporarily put document notebook in main vbox (scintilla widgets must stay - * in a visible parent window, otherwise there are X selection and scrollbar issues) */ - gtk_widget_reparent(notebook, - ui_lookup_widget(geany-main_widgets-window, vbox1)); + gtk_widget_ref(notebook); + gtk_container_remove(GTK_CONTAINER(parent), notebook); pane = horizontal ? gtk_hpaned_new() : gtk_vpaned_new(); gtk_container_add(GTK_CONTAINER(parent), pane); - gtk_widget_reparent(notebook, pane); + + gtk_container_add(GTK_CONTAINER(pane), notebook); + gtk_widget_unref(notebook); box = gtk_vbox_new(FALSE, 0); toolbar = create_toolbar(); @@ -364,10 +363,8 @@ static void on_unsplit(GtkMenuItem *menuitem, gpointer user_data) g_return_if_fail(edit_window.editor); - /* temporarily put document notebook in main vbox (scintilla widgets must stay - * in a visible parent window, otherwise there are X selection and scrollbar issues) */ - gtk_widget_reparent(notebook, - ui_lookup_widget(geany-main_widgets-window, vbox1)); + gtk_widget_ref(notebook); + gtk_container_remove(GTK_CONTAINER(pane), notebook); if (edit_window.sci != NULL edit_window.handler_id 0) { @@ -378,7 +375,9 @@ static void on_unsplit(GtkMenuItem *menuitem, gpointer user_data) gtk_widget_destroy(pane); edit_window.editor = NULL; edit_window.sci = NULL; - gtk_widget_reparent(notebook, parent); + + gtk_container_add(GTK_CONTAINER(parent), notebook); + gtk_widget_unref(notebook); } -- 1.7.1 diff --git a/scintilla/gtk/ScintillaGTK.cxx b/scintilla/gtk/ScintillaGTK.cxx index c288488..1773e61 100644 --- a/scintilla/gtk/ScintillaGTK.cxx +++ b/scintilla/gtk/ScintillaGTK.cxx @@ -416,6 +416,18 @@ void ScintillaGTK::RealizeThis(GtkWidget *widget) { gtk_widget_realize(widtxt); gtk_widget_realize(PWidget(scrollbarv)); gtk_widget_realize(PWidget(scrollbarh)); + + gtk_selection_add_targets(widget, GDK_SELECTION_PRIMARY, + clipboardCopyTargets, nClipboardCopyTargets); + +#ifndef USE_GTK_CLIPBOARD + gtk_selection_add_targets(widget, atomClipboard, + clipboardPasteTargets, nClipboardPasteTargets); +#endif + + gtk_drag_dest_set(widget, + GTK_DEST_DEFAULT_ALL, clipboardPasteTargets, nClipboardPasteTargets, + static_castGdkDragAction(GDK_ACTION_COPY | GDK_ACTION_MOVE)); } void ScintillaGTK::Realize(GtkWidget *widget) { @@ -425,6 +437,15 @@ void ScintillaGTK::Realize(GtkWidget *widget) { void ScintillaGTK::UnRealizeThis(GtkWidget *widget) { try { + + gtk_selection_clear_targets(widget, GDK_SELECTION_PRIMARY); + +#ifndef USE_GTK_CLIPBOARD + gtk_selection_clear_targets(widget, atomClipboard); +#endif + + gtk_drag_dest_unset(widget); + if
Re: [Geany-devel] Split Window Patches
On Mon, 28 Mar 2011 21:38:22 +1100, Lex wrote: On 28 March 2011 17:39, Matthew Brush mbr...@codebrainz.ca wrote: On 03/27/11 09:11, Colomban Wendling wrote: We need a better fix then. Maybe you can try to find out why the X clipboard get broken on Scintilla (and fix it :D). If it get fixed, we will probably can re-apply the patch. When the ScintillaObject gets reparented (unrealized/re-realized), it doesn't re-setup the selection targets again (this is only done in ScintillaGTK::Initialize()). The first patch #0001 is the same as the previous #0003 patch, which removes the reparenting stuff, to make Split Window work on Windows. The second patch #0002 fixes the primary selection issue by moving gtk_selection_add_targets() and friends into the ScintillaGTK::RealizeThis() function, and also adds its counter-parts to the ScintillaGTK::UnRealizeThis() function. The third patch #0003 fixes the issue where the I-Beam cursor is displayed even for the scrollbars when the widget is unrealized/re-realized. I tried a few different ways to do this, including trying to use Scintilla's SetCursor() function, but nothing seemed to work. Everything *seems* to work now, and it's not hacky, IMO - except maybe #0003 a little bit ;). Cheers, Matthew Brush Hi Matthew, I havn't had a chance to try it, but perhaps you should report the problem to Scintilla and submit the two patches to Neil. In general Geany tries to use an unmodified version of Scintilla so that changes don't have to be made each time its updated. Since this is to fix a plugin problem not a core problem (albeit a core plugin) I would suggest waiting for the response from Scintilla rather than including patches in the Geany Scintilla. What do others think? The same. First, the Scintilla patches should be shared with upstream, ideally they apply them. Then we can patch Geany. Regards, Enrico -- Get my GPG key from http://www.uvena.de/pub.asc pgpY7UlUYKHtc.pgp Description: PGP signature ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Split Window Patches
On Sat, 26 Mar 2011 22:26:42 -0700, Matthew wrote: On 03/26/11 17:31, Colomban Wendling wrote: #0002 Enable Scintilla's default popup editor menu on the split window editor so that there's a way to copy/paste/undo/redo/etc. Closes bug #2983145[2] although the keybindings and main menu items still don't work in the split window. Not sure about this one since the menu is neither translatable nor look like other Geany's menus. Maybe manually implementing a basic menu would be better? Should a bug/feature request also be reported to Scintilla? #0003 Remove the widget reparenting in an attempt to make the plugin work on Windows again. Although I can't test on Windows, I think there's a good chance this will fix the issues and it seems to work fine on Linux as well. I base this on an old GTK+ FAQ entry[3]. If someone can confirm, it will close bug #2725342[4] and the plugin can be re-enabled on the Windows build. Not sure about this, what was the issues before (the one the code talked about)? I'd prefer one of the original authors to review this one if possible... Nick (or Enrico, seems you also worked on this), could you check this/tell what was exactly the issue? See r3161 and r3163. also the bug report for this and FAQ entry linked from original message. I think it's best to have original authors (especially someone who is able to build on windows) to check this also. I guess that means me :). I tested the patch (#0003) on Windows and it works fine, at least I couldn't find any wrong behaviour. The problems referenced in the code were about broken scrollbars, I don't remember exactly whether in the normal editor widget or in the splitted. And there were issues with the PRIMARY selection which behaved wrong. None of these I could reproduce with the patch #0003, neither on Windows nor Linux. Before the patch, on Windows the splitted window wasn't reparented probably and so was moved in the upper left corner of the Geany root window, this is also fixed with the patch. I think we should commit it and see how it works for other people. Regards, Enrico -- Get my GPG key from http://www.uvena.de/pub.asc pgpZfg62fyokB.pgp Description: PGP signature ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Split Window Patches
On Sun, 27 Mar 2011 16:41:40 +0200, Enrico wrote: On Sat, 26 Mar 2011 22:26:42 -0700, Matthew wrote: On 03/26/11 17:31, Colomban Wendling wrote: #0002 Enable Scintilla's default popup editor menu on the split window editor so that there's a way to copy/paste/undo/redo/etc. Closes bug #2983145[2] although the keybindings and main menu items still don't work in the split window. Not sure about this one since the menu is neither translatable nor look like other Geany's menus. Maybe manually implementing a basic menu would be better? Should a bug/feature request also be reported to Scintilla? #0003 Remove the widget reparenting in an attempt to make the plugin work on Windows again. Although I can't test on Windows, I think there's a good chance this will fix the issues and it seems to work fine on Linux as well. I base this on an old GTK+ FAQ entry[3]. If someone can confirm, it will close bug #2725342[4] and the plugin can be re-enabled on the Windows build. Not sure about this, what was the issues before (the one the code talked about)? I'd prefer one of the original authors to review this one if possible... Nick (or Enrico, seems you also worked on this), could you check this/tell what was exactly the issue? See r3161 and r3163. also the bug report for this and FAQ entry linked from original message. I think it's best to have original authors (especially someone who is able to build on windows) to check this also. I guess that means me :). I tested the patch (#0003) on Windows and it works fine, at least I couldn't find any wrong behaviour. The problems referenced in the code were about broken scrollbars, I don't remember exactly whether in the normal editor widget or in the splitted. And there were issues with the PRIMARY selection which behaved wrong. None of these I could reproduce with the patch #0003, neither on Windows nor Linux. They still exist, at least the broken PRIMARY selection problem. It's easy to reproduce: select some text in the normal editor widget after you splitted it. You cannot paste the selected anymore by a middle click. Colomban already reverted the two related commits. Thanks. I think we should commit it and see how it works for other people. Er, let's say I changed my mind...:D. Regards, Enrico -- Get my GPG key from http://www.uvena.de/pub.asc pgpUh1jshqEEl.pgp Description: PGP signature ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Split Window Patches
Le 27/03/2011 16:41, Enrico Tröger a écrit : On Sat, 26 Mar 2011 22:26:42 -0700, Matthew wrote: #0003 Remove the widget reparenting in an attempt to make the plugin work on Windows again. Although I can't test on Windows, I think there's a good chance this will fix the issues and it seems to work fine on Linux as well. I base this on an old GTK+ FAQ entry[3]. If someone can confirm, it will close bug #2725342[4] and the plugin can be re-enabled on the Windows build. Not sure about this, what was the issues before (the one the code talked about)? I'd prefer one of the original authors to review this one if possible... Nick (or Enrico, seems you also worked on this), could you check this/tell what was exactly the issue? See r3161 and r3163. also the bug report for this and FAQ entry linked from original message. I think it's best to have original authors (especially someone who is able to build on windows) to check this also. I guess that means me :). I tested the patch (#0003) on Windows and it works fine, at least I couldn't find any wrong behaviour. The problems referenced in the code were about broken scrollbars, I don't remember exactly whether in the normal editor widget or in the splitted. And there were issues with the PRIMARY selection which behaved wrong. None of these I could reproduce with the patch #0003, neither on Windows nor Linux. Before the patch, on Windows the splitted window wasn't reparented probably and so was moved in the upper left corner of the Geany root window, this is also fixed with the patch. I think we should commit it and see how it works for other people. Well, I committed it (r5636) but... reverted it as r5640. It initially look good, but breaks the X PRIMARY clipboard (aka the mouse one) in the normal Geany editor (even after unsplitting), so it's not acceptable. We need a better fix then. Maybe you can try to find out why the X clipboard get broken on Scintilla (and fix it :D). If it get fixed, we will probably can re-apply the patch. Regards, Colomban ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Split Window Patches
On 28 March 2011 11:31, Matthew Brush mbr...@codebrainz.ca wrote: On 03/27/11 09:11, Colomban Wendling wrote: We need a better fix then. Maybe you can try to find out why the X clipboard get broken on Scintilla (and fix it :D). If it get fixed, we will probably can re-apply the patch. I spent a lot of time studying the Scintilla source and trying various things there and in splitwindow.c. I don't see anywhere that Scintilla is breaking the PRIMARY selection. It seems to become the owner of the PRIMARY selection in ScintillaGTK::ClaimSelection() whenever text is selected, but I can't figure for life of me why it it's not working. It seems like it should own the PRIMARY selection in each widget where you're selecting text. Even setting the owner of the primary selection manually to the original scintilla, it still locks into the split scintilla. So after spending way too much time on this, and getting nowhere, I give up. I'm attaching a patch that no dev is going to like because it behaves differently on Windows than it does on non-Windows to work around a know issue in GTK+ that is not documented to affect Windows differently than X windows. Either someone else needs to find the root of the problem deep inside GTK+ (ie. why it reparenting only breaks on Windows), we figure out what's happening in Scintilla, we use the slightly hacky approach of behaving differently on Windows, or we just leave the whole plugin stay disable and let the Windows users not have it, even though we *can* make it working. Since it's just a plugin, completely separate from the core code, which is already kinda hacky with what it's doing, my opinion is just to use the patch. It's not my call, but the patch is here if you want it. I only had a short time at lunch to look, but I noticed that Scintilla GTK selection code depends on USE_GTK_CLIPBOARD defined or not. I can't find where its defined so I presume its not. Maybe try with it defined? Cheers Lex Cheers, Matthew Brush ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Split Window Patches
On 28.03.2011 02:31, Matthew Brush wrote: So after spending way too much time on this, and getting nowhere, I give up. I'm attaching a patch that no dev is going to like because it behaves differently on Windows than it does on non-Windows to work around a know issue in GTK+ that is not documented to affect Windows differently than X windows. Either someone else needs to find the root of the problem deep inside GTK+ (ie. why it reparenting only breaks on Windows), we figure out what's happening in Scintilla, we use the slightly hacky approach of behaving differently on Windows, or we just leave the whole plugin stay disable and let the Windows users not have it, even though we *can* make it working. Since it's just a plugin, completely separate from the core code, which is already kinda hacky with what it's doing, my opinion is just to use the patch. It's not my call, but the patch is here if you want it. I agree here. Your patch is a lot better than not working at all, isn't it? Best regards. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Split Window Patches
On 03/27/11 18:19, Lex Trotman wrote: I only had a short time at lunch to look, but I noticed that Scintilla GTK selection code depends on USE_GTK_CLIPBOARD defined or not. I can't find where its defined so I presume its not. Maybe try with it defined? https://github.com/codebrainz/geany/blob/master/scintilla/gtk/ScintillaGTK.cxx#L85 Seems to be defined for GTK+ = 2.6. Cheers, Matthew Brush ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Split Window Patches
On 28 March 2011 13:35, Matthew Brush mbr...@codebrainz.ca wrote: On 03/27/11 18:19, Lex Trotman wrote: I only had a short time at lunch to look, but I noticed that Scintilla GTK selection code depends on USE_GTK_CLIPBOARD defined or not. I can't find where its defined so I presume its not. Maybe try with it defined? https://github.com/codebrainz/geany/blob/master/scintilla/gtk/ScintillaGTK.cxx#L85 Seems to be defined for GTK+ = 2.6. I was in too much of a rush, missed it. In that case I can't add anything new except to note that watching the lists, GTK on windows seems a bit doubtful as to its maturity. Like Thomas I'd say so long as the plugin continues to work on Linux and doesn't do anything *bad* on Windows then make it available with whatever differences on Windows documented, its better than not available at all. Cheers Lex Cheers, Matthew Brush ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Split Window Patches
On 03/26/11 17:31, Colomban Wendling wrote: #0002 Enable Scintilla's default popup editor menu on the split window editor so that there's a way to copy/paste/undo/redo/etc. Closes bug #2983145[2] although the keybindings and main menu items still don't work in the split window. Not sure about this one since the menu is neither translatable nor look like other Geany's menus. Maybe manually implementing a basic menu would be better? Should a bug/feature request also be reported to Scintilla? #0003 Remove the widget reparenting in an attempt to make the plugin work on Windows again. Although I can't test on Windows, I think there's a good chance this will fix the issues and it seems to work fine on Linux as well. I base this on an old GTK+ FAQ entry[3]. If someone can confirm, it will close bug #2725342[4] and the plugin can be re-enabled on the Windows build. Not sure about this, what was the issues before (the one the code talked about)? I'd prefer one of the original authors to review this one if possible... Nick (or Enrico, seems you also worked on this), could you check this/tell what was exactly the issue? See r3161 and r3163. also the bug report for this and FAQ entry linked from original message. I think it's best to have original authors (especially someone who is able to build on windows) to check this also. Cheers, Matthew Brush ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Split Window Patches
Not sure about this one since the menu is neither translatable nor look like other Geany's menus. Maybe manually implementing a basic menu would be better? Should a bug/feature request also be reported to Scintilla? Its not really Scintilla's problem that Geany menus look different :-) And Scintilla/Scite has its own translation system which we havn't integrated, see http://code.google.com/p/scite-files/wiki/Translations Cheers Lex ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Split Window Patches
On 03/26/11 22:44, Lex Trotman wrote: Not sure about this one since the menu is neither translatable nor look like other Geany's menus. Maybe manually implementing a basic menu would be better? Should a bug/feature request also be reported to Scintilla? Its not really Scintilla's problem that Geany menus look different :-) Well, I didn't mean because the appearance :) And Scintilla/Scite has its own translation system which we havn't integrated, see http://code.google.com/p/scite-files/wiki/Translations Ah ok. I'm not very knowledgeable about translation stuff. Cheers, Matthew Brush ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel