Skip to content
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

Merged
merged 9 commits into from
Apr 27, 2024
Merged

Various small WebHelper improvements #1295

merged 9 commits into from
Apr 27, 2024

Conversation

b4n
Copy link
Member

@b4n 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)

b4n added 4 commits November 21, 2023 22:33
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.
@b4n b4n added this to the 2.1.0 milestone Nov 22, 2023
@elextr
Copy link
Member

elextr commented Nov 22, 2023

Documentation looks ok, rest went zooming over my head.

@jbicha
Copy link

jbicha commented Jan 23, 2024

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.

  • 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.

@elextr
Copy link
Member

elextr commented Jan 23, 2024

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.

Edit: Google says libsoup2 and libsoup3 have API incompatibilities, so it is unknown how much needs to change in Updatechecker and Geniuspaste.

@xiota
Copy link
Contributor

xiota commented Apr 23, 2024

I don't use this plugin, but can test it to help facilitate its update if provided a list of things to check.

@elextr
Copy link
Member

elextr commented Apr 24, 2024

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.

@b4n
Copy link
Member Author

b4n commented Apr 24, 2024

@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.

@b4n
Copy link
Member Author

b4n commented Apr 24, 2024

@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 🤞

@xiota
Copy link
Contributor

xiota commented Apr 24, 2024

  • 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.

@elextr
Copy link
Member

elextr commented Apr 24, 2024

Maybe should update the requirements section of README as well.

@b4n
Copy link
Member Author

b4n commented Apr 24, 2024

@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.
Copy link
Member

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?

Copy link
Member Author

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.

Copy link
Member

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.

Copy link
Member Author

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?

Copy link
Member

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.

Copy link
Member Author

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.

Copy link
Contributor

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?

Copy link
Member

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?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably

b4n added 5 commits April 25, 2024 22:38
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.
@b4n b4n force-pushed the webhelper/stuff branch from 8ee8a92 to 8b17184 Compare April 25, 2024 20:39
@b4n
Copy link
Member Author

b4n commented Apr 25, 2024

And voila, I got plagued with another round of cleanups :)

Copy link
Member

@elextr elextr left a 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.

webhelper/src/gwh-browser.c Show resolved Hide resolved
@b4n b4n merged commit 8da1471 into geany:master Apr 27, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants