-
Notifications
You must be signed in to change notification settings - Fork 270
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Various small WebHelper improvements #1295
Conversation
b4n
commented
Nov 22, 2023
- Improve documentation
- Add support for webkit2gtk-4.1 (no changes)
- Add a key binding & menu item for loading the current file into the web view (for e.g. static HTML pages)
This is the same as webkit2gtk-4.0 but using libsoup3 instead of libsoup2, which is irrelevant for WebHelper's usage.
This looks pretty and helps a bit with discovery... although this item currently has no default binding so the user has to have set one, meaning they already probably know about it. But it looks nice and consistent.
This is a somewhat popular request, which is easy enough to implement and can be handy when working on static HTML files.
Documentation looks ok, rest went zooming over my head. |
Switching from webkit2gtk 4.0 to 4.1 is important. Fedora has stopped 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.
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. |
Maintainers are:
Edit: Google says libsoup2 and libsoup3 have API incompatibilities, so it is unknown how much needs to change in Updatechecker and Geniuspaste. |
I don't use this plugin, but can test it to help facilitate its update if provided a list of things to check. |
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. |
@jbicha could you make a PR with your Markdown changes, assuming they are not too Ubunut/Debian-specific? |
@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 🤞 |
|
Maybe should update the requirements section of README as well. |
@elextr I just did the bare minimum, see update |
webhelper/README
Outdated
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Geany 0.20 or 2.0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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? 😁
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 😁
And you had already made changes to README here so it seemed pretty simple to add.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Damned 😄
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What was the first version of Geany supporting GTK 2.x? Is it necessary to mention Geany version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably
GTK, GLib, GIO, Gdk-Pixbuf, WebKit, all those are actually minimal dependencies of the code (although arguably GIO is always at the same version than GLib, so having something different there is somewhat of a lie). Geany is merely based on the PLUGIN_VERSION_CHECK(), which might or might not be appropriate.
And voila, I got plagued with another round of cleanups :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGBI except for one query.