Add icon-name, icon-search-path, and icon-resource-path to PurpleProtocol

Review Request #790 — Created July 8, 2021 and submitted

grim
pidgin/pidgin
default
9713338010e9
pidgin

These are now used by pidgin_create_icon_from_protocol which will attempt to
load the icon for the protocol from the default GtkIconTheme if the protocol
has a non NULL value for icon-name.

pidgin_ui_init was also updated to add icon-search-path and icon-resource-path
to the default GtkIconTheme when a protocol is registered. It also scans all of
the protocols that have been already been registered as that happens before the
ui is initialized.

Removed old local icon and then ran with an inverted icon to make sure pidgin was displaying the icon from the resource.

Description From Last Updated

Maybe it should return a GIcon (implemented as a GLoadableIcon or something), that can return multiple sizes?

QuLogicQuLogic

This is not a gresource?

QuLogicQuLogic

embedded

QuLogicQuLogic

Dashes instead of underscore?

QuLogicQuLogic

Ditto.

QuLogicQuLogic

Ditto.

QuLogicQuLogic

Ditto.

QuLogicQuLogic

its

QuLogicQuLogic
rekkanoryo
  1. Ship It!
  2. Admittedly I did not review this super in-depth, but I didn't see anything that jumped out at me in a cursory review.

  3. 
      
QuLogic
  1. 
      
  2. Maybe it should return a GIcon (implemented as a GLoadableIcon or something), that can return multiple sizes?

    1. We could just make it a GIcon and then the prpls can decide what they want to do. I assume most would use embeded resources to create the icon via g_bytes_icon_new and then gtk can resize it and stuff, although I'm not sure how difficult it'll be to use that in another ui, but it looks like that's where GLoadableIcon comes in so maybe that's the way to go?

    2. Part of my thinking was to pass a size enum or something along so that protocols who's icon's don't scale well for smaller sizes could use another icon or something.

    3. Right, that's what the GLoadableIcon interface provides.

    4. After digging into that a bit more it looks like protocol plugins would need to implement that to control file for the sizes. However, that also means a whole bunch of resource shennigans...

      However, we could create a PurpleIcon that would take a GResource and a path prefix to off load all of that. I'd opt for the PurpleIcon name so it can be reused for plugins and stuff too.

      Thoughts?

    5. I just submitted RR 885 and realized that if we leave the icon name and instead of loading the file from disk, we can just look it up in the icon theme that the protocol plugins can provide via their resources. I'll update this RR to do that now.

    6. After some more thought, we'll need to have the protocol plugin export the resource path that we'll pass into gtk_icon_theme_add_resource_path as well as the name. Then, Pidgin will need to connect to the ProtocolManager's registered signal to add the resource path from the protocol via gtk_icon_theme_add_resource_path. However, there is no way to unregister an icon path, which should be okay as an unloaded plugin is still in memory and not actually closed.

  3. 
      
grim
QuLogic
  1. 
      
  2. libpurple/protocol.h (Diff revision 2)
     
     

    This is not a gresource?

    1. Yeah copy paste error. The icon_search_path support is for protocols that aren't using glib-compile-resource.

  3. libpurple/protocol.h (Diff revision 2)
     
     

    embedded

  4. libpurple/protocol.c (Diff revision 2)
     
     

    Dashes instead of underscore?

  5. libpurple/protocol.c (Diff revision 2)
     
     
     
  6. libpurple/protocol.c (Diff revision 2)
     
     
  7. libpurple/protocol.c (Diff revision 2)
     
     
  8. pidgin/libpidgin.c (Diff revision 2)
     
     
  9. 
      
grim
QuLogic
  1. Ship It!
  2. 
      
grim
grim
Review request changed

Status: Closed (submitted)

Loading...