Convert all of the buddylist context menus to GMenu

Review Request #1481 — Created May 27, 2022 and submitted

grim
pidgin/pidgin
default
pidgin
Convert all of the buddylist context menus to GMenu

Every item every way I think...

Summary
Convert all of the buddylist context menus to GMenu
Description From Last Updated

Delete?

QuLogicQuLogic

Can this be changed for make data the buddy list instead of looking at a global?

QuLogicQuLogic

Ditto.

QuLogicQuLogic

Can this use the blist from data instead of the gktblist global?

QuLogicQuLogic

Isn't that blist, and node?

QuLogicQuLogic

This probably shouldn't be in between the two custom icon actions.

QuLogicQuLogic

Why so far away from the audio one?

QuLogicQuLogic

Maybe add vars for account and name, since they're used 3 times here.

QuLogicQuLogic

Maybe cast to GtkApplication and GActionMap? I think they're never used as these types.

QuLogicQuLogic

Should you not check for PURPLE_PROTOCOL_IMPLEMENTS(protocol, MEDIA, get_caps)?

QuLogicQuLogic

Can use enabled variable here, if it fits better?

QuLogicQuLogic

Again, maybe pre-cast?

QuLogicQuLogic

Check protocol != NULL, or else drop the check for xfer above? Also, remove outdated comment.

QuLogicQuLogic

Didn't this always return the same menu? Do you need to search for custom-icon every time?

QuLogicQuLogic
grim
grim
QuLogic
  1. 
      
  2. pidgin/gtkblist.c (Diff revision 3)
     
     

    Delete?

  3. pidgin/gtkblist.c (Diff revision 3)
     
     

    Can this be changed for make data the buddy list instead of looking at a global?

  4. pidgin/gtkblist.c (Diff revision 3)
     
     
  5. pidgin/gtkblist.c (Diff revision 3)
     
     
     
     
     
     
     
     
     

    Can this use the blist from data instead of the gktblist global?

  6. pidgin/gtkblist.c (Diff revision 3)
     
     

    Isn't that blist, and node?

    1. not sure where you're seeing node..?

    2. Ah, I must have expanded too far, or maybe it was changed between rev 1 and 3 when I originally commented.

  7. pidgin/gtkblist.c (Diff revision 3)
     
     

    This probably shouldn't be in between the two custom icon actions.

    1. I was trying to alphabetize them but they don't all have the same namespace...

  8. pidgin/gtkblist.c (Diff revision 3)
     
     

    Why so far away from the audio one?

  9. pidgin/gtkblist.c (Diff revision 3)
     
     
     

    Maybe add vars for account and name, since they're used 3 times here.

  10. pidgin/gtkblist.c (Diff revision 3)
     
     
     
     

    Maybe cast to GtkApplication and GActionMap? I think they're never used as these types.

  11. pidgin/gtkblist.c (Diff revision 3)
     
     

    Should you not check for PURPLE_PROTOCOL_IMPLEMENTS(protocol, MEDIA, get_caps)?

  12. pidgin/gtkblist.c (Diff revision 3)
     
     

    Can use enabled variable here, if it fits better?

  13. pidgin/gtkblist.c (Diff revision 3)
     
     

    Again, maybe pre-cast?

  14. 
      
grim
QuLogic
  1. 
      
  2. pidgin/gtkblist.c (Diff revision 4)
     
     
     

    Check protocol != NULL, or else drop the check for xfer above?

    Also, remove outdated comment.

  3. pidgin/gtkblist.c (Diff revision 4)
     
     

    Didn't this always return the same menu? Do you need to search for custom-icon every time?

  4. 
      
grim
QuLogic
  1. Ship It!
  2. 
      
grim
Review request changed

Status: Closed (submitted)

Loading...