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

[WIP] commander: fixed deprecated gtk3 calls #865

Closed
wants to merge 1 commit into from

Conversation

lpaulsen93
Copy link
Contributor

Fixed deprecated gtk3 calls. The PR unfortunately includes an ugly list of all the GTK stockitems as I did not see a different solution.

@lpaulsen93 lpaulsen93 changed the title commander: fixed deprecated gtk3 calls [WIP] commander: fixed deprecated gtk3 calls May 18, 2019
@lpaulsen93
Copy link
Contributor Author

@b4n: please fell free to reject this PR is it's to ugly for you 😀

All the functions in the if-condition the code part below seem to not have any replacement functions which colud be of help.

      if (GTK_IS_IMAGE_MENU_ITEM (node->data) &&
          gtk_image_menu_item_get_use_stock (node->data) &&
          gtk_stock_lookup (gtk_menu_item_get_label (node->data), &item)) {
        item_label = g_strdup (item.label);
        use_underline = TRUE;
      } else {
        item_label = g_strdup (gtk_menu_item_get_label (node->data));
        use_underline = gtk_menu_item_get_use_underline (node->data);
      }

So I have written a function which returns the required info. Why am I getting that stock-id's at all under GTK3? Is the geany core still using them on GTK3?

Also a little question regarding the plugin API: as the utils lib is not a registered plugin - how can I get a callback on closing of geany to free the memory allocated for the utils lib?

@elextr
Copy link
Member

elextr commented May 18, 2019

Also a little question regarding the plugin API: as the utils lib is not a registered plugin - how can I get a callback on closing of geany to free the memory allocated for the utils lib?

There is a "geany-startup-complete" signal, but I don't think there is an equivalent "geany-starting-quitting" one when quitting.

[Edit: so it doesn't matter you are not a plugin because there is nothing to catch anyway]

@elextr
Copy link
Member

elextr commented May 18, 2019

Is the geany core still using them on GTK3?

All over the place, its only deprecated, not removed in GTK3 so it still works. Probably won't with GTK4 but its also probably not the only thing that will break.

@lpaulsen93
Copy link
Contributor Author

lpaulsen93 commented May 18, 2019

All over the place, its only deprecated, not removed in GTK3 so it still works.

Ok, that's why I see the stock-id's being returned as labels on GTK3. Then the function I wrote is maybe the only solution to have some interworking with code that uses the gtk2-stock-items and code that doesn't to prevent the deprecation warnings. (But if there is a better solution I would be happy to use it)

I know they still work - just all this warnings all over the place in geany-plugins on gtk3 - hard to see the important ones.

@elextr
Copy link
Member

elextr commented May 18, 2019

All over the place, its only deprecated, not removed in GTK3 so it still works.

And Geany itself is compiled with deprecation warnings off so you don't see it.

@lpaulsen93
Copy link
Contributor Author

I would really like to fix it for geany-plugins at least. Sometimes there is just a warning and you can run geany and the plugins - but that doesn't mean that the GUI works as it should under gtk3.

@elextr
Copy link
Member

elextr commented May 18, 2019

On the library memory issue. You probably shouldn't keep memory in any library, not just yours, libraries should always return allocated memory to the caller to dispose of, you never know if the library is going to ever be entered again.

@lpaulsen93
Copy link
Contributor Author

You probably shouldn't keep memory in any library, not just yours

I am not sure if that is generally done. I didn't check the internals of the libs but I guess libs like OpenSSL and LibXML2 (which have init functions) will reserve memory for themselves. Also I do not want to create the hashtable I use per caller but share it among the plugins.

@elextr
Copy link
Member

elextr commented May 18, 2019

I have never used Openssl, but for libxml, as far as I remember either it returns the memory (the document tree) to the caller, or the caller has to do new/free calls like for newcontext paired with freecontext.

To share some resource between plugins reference count it and have plugins free their reference when they are done with it or are exiting.

@elextr
Copy link
Member

elextr commented May 18, 2019

Probably this PR isn't the place for the resource management design discussion, maybe create a separate issue for it.

@codebrainz
Copy link
Member

codebrainz commented May 18, 2019

Also a little question regarding the plugin API: as the utils lib is not a registered plugin - how can I get a callback on closing of geany to free the memory allocated for the utils lib?

If this library was statically compiled into each plugin (don't think it is), you could probably use g_module_unload.

If you wanted to cover 99% of the cases (GCC or Clang) automatically, you could use the destructor attribute, like maybe:

#ifdef __GNUC__
#define GP_UTILS_DESTRUCTOR __attribute__((destructor))
#else 
#define GP_UTILS_DESTRUCTOR
#edif

GP_UTILS_DESTRUCTOR
void gp_utils_finalize(void)
{
    // ...
}

Alternatively, you could add two functions like gp_utils_init and gp_utils_finalize that could be called from each plugin which uses the library, to initialize and cleanup internal memory of the library.

Also I do not want to create the hashtable I use per caller but share it among the plugins.

Then it would probably have to go inside Geany I suspect, or Geany would have to link to and init/cleanup the utils library, which is sort of counter to its design.

Since stock items still exist in the current major versions of GTK+ that Geany supports, it seems like a lot of work/waste to avoid some deprecation warnings, IMO. Maybe you could make some wrapper header/macro that disables the deprecation warnings in specific bulk-deprecated headers like gtkstock.h or whichever, and include it first before other headers? The deprecation warnings are annoying, but they just mean "Will be removed for GTK+ 4", pretty much.

Edit: I also just noticed the G_GNUC_*_IGNORE_DEPRECATIONS macro, maybe tricky stuff could be wrapped directly like:

#if GTK_CHECK_VERSION(3, 10, 0)
G_GNUC_BEGIN_IGNORE_DEPRECATIONS
#endif
      if (GTK_IS_IMAGE_MENU_ITEM (node->data) &&
          gtk_image_menu_item_get_use_stock (node->data) &&
          gtk_stock_lookup (gtk_menu_item_get_label (node->data), &item)) {
        item_label = g_strdup (item.label);
        use_underline = TRUE;
      } else {
        item_label = g_strdup (gtk_menu_item_get_label (node->data));
        use_underline = gtk_menu_item_get_use_underline (node->data);
      }
#if GTK_CHECK_VERSION(3, 10, 0)
G_GNUC_END_IGNORE_DEPRECATIONS
#endif

Or whatever makes sense for this PR.

@b4n
Copy link
Member

b4n commented Feb 7, 2020

I really don't like this PR: too much code, too much copy, and all this for getting rid of deprecation warnings -- where this code actually has to support the deprecated API.

Edit: I also just noticed the G_GNUC_*_IGNORE_DEPRECATIONS macro, maybe tricky stuff could be wrapped directly

Yes, that sounds like a good solution.

@b4n b4n modified the milestones: 1.36.0, 1.37.0 Feb 7, 2020
@lpaulsen93
Copy link
Contributor Author

I don't see a good solution for this. The gtk people keep on deprecating stuff. At the moment I will not continue work on this - I more tend to wait until gtk2 support is dropped and only one version needs to be supported.

@b4n
Copy link
Member

b4n commented Feb 8, 2020

See #952 for a version based on @codebrainz suggestion, which I like best. It's non-intrusive, does the job and makes sense (as the plugin has to support deprecated APIs so long as they work)

@eht16 eht16 modified the milestones: 1.37.0, 1.38.0 Sep 4, 2021
@eht16 eht16 modified the milestones: 1.38.0, 1.39.0/2.0.0 Oct 9, 2021
@b4n b4n closed this in 38a138f Oct 17, 2023
liomarhora pushed a commit to liomarhora/geany-plugins that referenced this pull request Feb 28, 2024
We have to use the deprecated API because it is the only way to fetch
the correct information from menus that were themselves created with
that deprecated API, so we don't care about deprecation here.

Closes geany#865
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.

None yet

6 participants