[Github-comments] Re: [geany/geany-plugins] Various small WebHelper improvements (PR #1295)
Merged #1295 into master. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1295#event-12632694851 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Various small WebHelper improvements (PR #1295)
@elextr commented on this pull request. > @@ -950,6 +1005,23 @@ gwh_browser_set_uri (GwhBrowser *self, g_free (real_uri); } +gboolean +gwh_browser_set_uri_from_document (GwhBrowser*self, + GeanyDocument *doc) +{ + gchar *uri; + + /* document must exist on disk */ + if (! doc || ! doc->real_path) +return FALSE; + + uri = g_strconcat ("file://", doc->file_name, NULL); Ok, I forgot the inconsistency that sometimes paths are called paths and sometimes filenames in C land, ... sigh @elextr goes back to his nice consistent C++ `std::path` :grin: -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1295#discussion_r1581662031 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Various small WebHelper improvements (PR #1295)
@b4n commented on this pull request. > @@ -950,6 +1005,23 @@ gwh_browser_set_uri (GwhBrowser *self, g_free (real_uri); } +gboolean +gwh_browser_set_uri_from_document (GwhBrowser*self, + GeanyDocument *doc) +{ + gchar *uri; + + /* document must exist on disk */ + if (! doc || ! doc->real_path) +return FALSE; + + uri = g_strconcat ("file://", doc->file_name, NULL); No, `doc->real_path` is used to verify the document is actually saved to a file, but it's in locale encoding, whereas `doc->file_name` is the expected UTF-8. Admittedly, it might not behave properly if the locale is non-UTF-8 and the file path contains non-ASCII characters; but at least the worse that would happen would be the file not being found. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1295#discussion_r1581062630 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Various small WebHelper improvements (PR #1295)
@elextr approved this pull request. LGBI except for one query. > @@ -950,6 +1005,23 @@ gwh_browser_set_uri (GwhBrowser *self, g_free (real_uri); } +gboolean +gwh_browser_set_uri_from_document (GwhBrowser*self, + GeanyDocument *doc) +{ + gchar *uri; + + /* document must exist on disk */ + if (! doc || ! doc->real_path) +return FALSE; + + uri = g_strconcat ("file://", doc->file_name, NULL); Why test doc->real_path above and use untested doc->file_name here? Shouldn't this use the path? -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1295#pullrequestreview-2023836790 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Various small WebHelper improvements (PR #1295)
And voila, I got plagued with another round of cleanups :) -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1295#issuecomment-2078136827 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Various small WebHelper improvements (PR #1295)
@b4n commented on this pull request. > @@ -26,9 +26,9 @@ Prominent features Requirements -This plugin requires GTK+ (>= 2.16), GLib (>= 2.16), GIO (>= 2.18), -GdkPixbuf (>= 2.0), WebKitGTK (>= 1.1.18), and obviously Geany (>= 0.20) to -work. If you intend to build it yourself, you will need to get the development +This plugin requires GTK3, GLib (>= 2.22), GIO (>= 2.18), GdkPixbuf (>= 2.0), +WebKit2GTK (either API 4.0 or 4.1), and obviously Geany (>= 0.20) to work. Probably -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1295#discussion_r1580097533 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Various small WebHelper improvements (PR #1295)
@b4n pushed 5 commits. 52ab3ebefc8fdf805b081bac74efaec442c82529 webhelper: Use modern GObject private data 9f420e09447cc2f8a57e4880dfa99bebc67734e7 webhelper: Replace use of deprecated GtkVBox with GtkBox 3edea3f891cdbe241d63d2b4b37ef4c5eea035c4 webhelper: Remove use of deprecated g_thread_init() 9eb5d4630a1449d8c4ea13926631f23f447f85fe webhelper: Replace deprecated g_value_set_char() 8b17184e6a2eecfd598367d9f9783e9abbcd984f webhelper: Update minimal requirements -- View it on GitHub: https://github.com/geany/geany-plugins/pull/1295/files/8ee8a92a9eb7276ae210e376e0b80d271f885162..8b17184e6a2eecfd598367d9f9783e9abbcd984f You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Various small WebHelper improvements (PR #1295)
@elextr commented on this pull request. > @@ -26,9 +26,9 @@ Prominent features Requirements -This plugin requires GTK+ (>= 2.16), GLib (>= 2.16), GIO (>= 2.18), -GdkPixbuf (>= 2.0), WebKitGTK (>= 1.1.18), and obviously Geany (>= 0.20) to -work. If you intend to build it yourself, you will need to get the development +This plugin requires GTK3, GLib (>= 2.22), GIO (>= 2.18), GdkPixbuf (>= 2.0), +WebKit2GTK (either API 4.0 or 4.1), and obviously Geany (>= 0.20) to work. Well, do we need to list all those things individually? I would expect a new enough webkit2gtk will require newer versions of Glib, GTK3, etc than the plugin. So wherever a builder gets their webkit2gtk-dev from should have the right parts automatically ... won't it? -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1295#discussion_r1579598013 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Various small WebHelper improvements (PR #1295)
@xiota commented on this pull request. > @@ -26,9 +26,9 @@ Prominent features Requirements -This plugin requires GTK+ (>= 2.16), GLib (>= 2.16), GIO (>= 2.18), -GdkPixbuf (>= 2.0), WebKitGTK (>= 1.1.18), and obviously Geany (>= 0.20) to -work. If you intend to build it yourself, you will need to get the development +This plugin requires GTK3, GLib (>= 2.22), GIO (>= 2.18), GdkPixbuf (>= 2.0), +WebKit2GTK (either API 4.0 or 4.1), and obviously Geany (>= 0.20) to work. What was the first version of Geany supporting GTK 2.x? Is it necessary to mention Geany version? -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1295#discussion_r1579595352 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Various small WebHelper improvements (PR #1295)
@b4n commented on this pull request. > @@ -26,9 +26,9 @@ Prominent features Requirements -This plugin requires GTK+ (>= 2.16), GLib (>= 2.16), GIO (>= 2.18), -GdkPixbuf (>= 2.0), WebKitGTK (>= 1.1.18), and obviously Geany (>= 0.20) to -work. If you intend to build it yourself, you will need to get the development +This plugin requires GTK3, GLib (>= 2.22), GIO (>= 2.18), GdkPixbuf (>= 2.0), +WebKit2GTK (either API 4.0 or 4.1), and obviously Geany (>= 0.20) to work. Damned :smile: Simple to add, yes, but simple to figure out, not so much. I sometimes wish there was a generic tool that can *reliably* scan the code an tell me "ok, you need libfoo >= 1.2.3 (because of $reason), libbar 42.0 (because of $othereason), …", but well. I had a script that did something like that ad-hoc grepping GTK documentation and the code to try and get an idea, but not sure if it still works, and it never was truly reliable. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1295#discussion_r1579574079 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Various small WebHelper improvements (PR #1295)
@elextr commented on this pull request. > @@ -26,9 +26,9 @@ Prominent features Requirements -This plugin requires GTK+ (>= 2.16), GLib (>= 2.16), GIO (>= 2.18), -GdkPixbuf (>= 2.0), WebKitGTK (>= 1.1.18), and obviously Geany (>= 0.20) to -work. If you intend to build it yourself, you will need to get the development +This plugin requires GTK3, GLib (>= 2.22), GIO (>= 2.18), GdkPixbuf (>= 2.0), +WebKit2GTK (either API 4.0 or 4.1), and obviously Geany (>= 0.20) to work. Well, you updated requirements in README as part of #1336, which is what prompted me to mention it here and on the markdown PR, so its all your own fault :grin: And you had already made changes to README here so it seemed pretty simple to add. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1295#discussion_r1579477539 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Various small WebHelper improvements (PR #1295)
@b4n commented on this pull request. > @@ -26,9 +26,9 @@ Prominent features Requirements -This plugin requires GTK+ (>= 2.16), GLib (>= 2.16), GIO (>= 2.18), -GdkPixbuf (>= 2.0), WebKitGTK (>= 1.1.18), and obviously Geany (>= 0.20) to -work. If you intend to build it yourself, you will need to get the development +This plugin requires GTK3, GLib (>= 2.22), GIO (>= 2.18), GdkPixbuf (>= 2.0), +WebKit2GTK (either API 4.0 or 4.1), and obviously Geany (>= 0.20) to work. > but pretending it will work with any Geany pre-GTK3 is wrong. Good point. Well, what about you opening a "update plugins requirements" PR after having checked and updated them all? :grin: Though, if I find the time (maybe tonight? who knows) I'll try and do that for this one. But I don't love seeing this endlessly delayed for basically unrelated stuff… although I laid that on myself given the PR title, didn't I? -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1295#discussion_r1579248405 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Various small WebHelper improvements (PR #1295)
@elextr commented on this pull request. > @@ -26,9 +26,9 @@ Prominent features Requirements -This plugin requires GTK+ (>= 2.16), GLib (>= 2.16), GIO (>= 2.18), -GdkPixbuf (>= 2.0), WebKitGTK (>= 1.1.18), and obviously Geany (>= 0.20) to -work. If you intend to build it yourself, you will need to get the development +This plugin requires GTK3, GLib (>= 2.22), GIO (>= 2.18), GdkPixbuf (>= 2.0), +WebKit2GTK (either API 4.0 or 4.1), and obviously Geany (>= 0.20) to work. I thought it might have been a typo 0.2 vs 2.0 ;-) Don't forget that the plugin READMEs go to the plugins.geany.org site, probably doesn't have to be 2.0 but pretending it will work with any Geany pre-GTK3 is wrong. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1295#discussion_r1579233070 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Various small WebHelper improvements (PR #1295)
@b4n commented on this pull request. > @@ -26,9 +26,9 @@ Prominent features Requirements -This plugin requires GTK+ (>= 2.16), GLib (>= 2.16), GIO (>= 2.18), -GdkPixbuf (>= 2.0), WebKitGTK (>= 1.1.18), and obviously Geany (>= 0.20) to -work. If you intend to build it yourself, you will need to get the development +This plugin requires GTK3, GLib (>= 2.22), GIO (>= 2.18), GdkPixbuf (>= 2.0), +WebKit2GTK (either API 4.0 or 4.1), and obviously Geany (>= 0.20) to work. Who knows? As mentioned, I didn't properly review those numbers just yet. Geany dep is likely to be closer to 0.20 than 2.0 though. Others like GTK3 or GLib might be higher. But I don't plan in fiddling with that for the moment, because it's less trivial than what I feel it gains, and can be done at a later point. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1295#discussion_r1579217448 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Various small WebHelper improvements (PR #1295)
@elextr commented on this pull request. > @@ -26,9 +26,9 @@ Prominent features Requirements -This plugin requires GTK+ (>= 2.16), GLib (>= 2.16), GIO (>= 2.18), -GdkPixbuf (>= 2.0), WebKitGTK (>= 1.1.18), and obviously Geany (>= 0.20) to -work. If you intend to build it yourself, you will need to get the development +This plugin requires GTK3, GLib (>= 2.22), GIO (>= 2.18), GdkPixbuf (>= 2.0), +WebKit2GTK (either API 4.0 or 4.1), and obviously Geany (>= 0.20) to work. Geany 0.20 or 2.0? -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1295#pullrequestreview-2020969159 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Various small WebHelper improvements (PR #1295)
@elextr I just did the bare minimum, see update -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1295#issuecomment-2075710224 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Various small WebHelper improvements (PR #1295)
@b4n pushed 1 commit. 8ee8a92a9eb7276ae210e376e0b80d271f885162 webhelper: Update dependencies in README -- View it on GitHub: https://github.com/geany/geany-plugins/pull/1295/files/d19aee697472477c0e53c12a75303121e47f5b64..8ee8a92a9eb7276ae210e376e0b80d271f885162 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Various small WebHelper improvements (PR #1295)
Maybe should update the requirements section of README as well. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1295#issuecomment-2074716996 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Various small WebHelper improvements (PR #1295)
* Loads and seems to function when built with either webkit2gtk 4.0 or 4.1. * Key bindings all work when conditions for their function are met. * The reload on save function works. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1295#issuecomment-2074474371 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Various small WebHelper improvements (PR #1295)
@xiota if you'd like to try, just look at the PR's commit messages. Basically, try building against WebKit2GTK 4.1, enable the plugin and try loading pages, editing an HTML displayed there, etc. But all should be good in theory :crossed_fingers: -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1295#issuecomment-2074347674 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Various small WebHelper improvements (PR #1295)
@jbicha could you make a PR with your Markdown changes, assuming they are not too Ubunut/Debian-specific? @elextr @jbicha I'll merge this (which at least *allows* building webhelper with 4.1, although not forcing it), and try to update the rest to match, as indeed it will cause issues if various plugins load various versions of those libraries. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1295#issuecomment-2074343157 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Various small WebHelper improvements (PR #1295)
Which plugin did you mean? There are four listed in the previous two comments. Since I would think that it is not possible to load old and new versions of webkit2gtk and libsoup at the same time probably the version needs to be selected once and builds of all four need to disable them if the new version is found. Then they can be switched to re-enable as they are modified to use the new libraries, ping @b4n as the plugin build system expert. Webhelper is done here by itself and if the comments are correct will select webkit2gtk-4.1 if its available, so @xiota if you build it with this PR applied it should build with the new version if its on your system. Then just try it to see that it doesn't crash when you use it a bit. Then the other plugins need to be adapted to the new libraries by "somebody", since the maintainers seem to have departed. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1295#issuecomment-2073809698 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Various small WebHelper improvements (PR #1295)
I don't use this plugin, but can test it to help facilitate its update if provided a list of things to check. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1295#issuecomment-2073071561 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Various small WebHelper improvements (PR #1295)
Maintainers are: - Webhelper - @b4n, this PR - Markdown - in theory @codebrainz, but hasn't contributed to it for 8 years, the markdown plugin died for a while until others ported to GTK3, maybe @b4n could port the (small) webkit4.1 change to markdown, or it can die again until a new maintainer or contributor steps up - Updatechecker - @frlan - Geniuspaste - in theory @Enrix835, but hasn't contributed to it for 12 years, maybe if the libsoup3 changes are small enough @frlan may port them to Geniuspaste, or wait for a maintainer or contributor. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1295#issuecomment-1907080484 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Various small WebHelper improvements (PR #1295)
Switching from webkit2gtk 4.0 to 4.1 is important. Fedora has [stopped](https://fedoraproject.org/wiki/Changes/Remove_webkit2gtk-4.0_API_Version) building the 4.0 API in preparation for their Fedora 40 release in a few months. Debian and Ubuntu are preparing to do the same thing. Ideally, all the plugins would be ported at the same time. - markdown (uses webkitgtk 4.0 - webhelper (uses webkitgtk 4.0 - geniuspaste (uses libsoup2) - updatechecker (uses libsoup2) However, it is not possible for a process to load both libsoup2 and libsoup3 at the same time. I think this could be possible if an app uses plugins. I tested this with the markdown plugin (which I switched to use webkitgtk 4.1). With the markdown plugin enabled and a Markdown document open, many times (but not every time), I was unable to use the _Check for Updates_ button or _Paste It!_ without Geany hanging. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1295#issuecomment-1906999722 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Various small WebHelper improvements (PR #1295)
Documentation looks ok, rest went zooming over my head. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1295#issuecomment-1821933103 You are receiving this because you are subscribed to this thread. Message ID: