Change Summary:
rebased
Commits: |
|
|||||||
---|---|---|---|---|---|---|---|---|
Diff: |
Revision 2 (+682) |
Review Request #1356 — Created March 18, 2022 and submitted
This is going to be used in the GMenu's for plugins, protocols, accounts, etc where there are dynamic items and we need to be able to set a target for them dynamically.
Ran the new unit tests.
Summary | ID |
---|---|
3add3ad26aeebf88cd8a62401834285075b4c429 |
Description | From | Last Updated |
---|---|---|
Should be the GIR link syntax. |
QuLogic | |
dynamic-target (or maybe the constant macro can be used?) |
QuLogic | |
Should be [func@Account.get_id] or whatever it is. |
QuLogic | |
recursing |
QuLogic | |
Should use the macro instead of the dynamic-target string? |
QuLogic | |
Not tabs. |
QuLogic | |
This is going to be confusing for func, because it will be receiving a GMenuModel that is not the one … |
QuLogic | |
Possibly should assert that n == n_items and l == NULL at the end. |
QuLogic | |
Maybe rename to expected (in all tests). |
QuLogic | |
So this doesn't work when the account is first enabled because it doesn't have a connection. |
QuLogic | |
Missing period. |
lifesfaded | |
If I've understood the connected signals correctly, this will never be true if you only have one account, since this … |
QuLogic | |
Quote dynamic-target, to match the example below. |
QuLogic | |
Should that not be a g_return_if_fail? |
QuLogic | |
Could be g_free(item)? |
QuLogic | |
Maybe also a test with a dynamic-target, but with a name that's not passed to purple_menu_populate_dynamic_targets? |
QuLogic |
rebased
Commits: |
|
|||||||
---|---|---|---|---|---|---|---|---|
Diff: |
Revision 2 (+682) |
libpurple/tests/test_menu.c (Diff revision 2) |
---|
Possibly should assert that
n == n_items
andl == NULL
at the end.
address issues in the review and add a public walk function that simplifies testing and helps us deal with the nested menu structures.
Commits: |
|
|||||||
---|---|---|---|---|---|---|---|---|
Diff: |
Revision 3 (+956 -58) |
libpurple/purplemenu.c (Diff revisions 2 - 3) |
---|
This is going to be confusing for
func
, because it will be receiving aGMenuModel
that is not the one that was passed in topurple_menu_walk
.
Commits: |
|
|||||||
---|---|---|---|---|---|---|---|---|
Diff: |
Revision 4 (+960 -58) |
pidgin/pidginaccountsenabledmenu.c (Diff revision 4) |
---|
So this doesn't work when the account is first enabled because it doesn't have a
connection
.
avoid a crash
Commits: |
|
|||||||
---|---|---|---|---|---|---|---|---|
Diff: |
Revision 5 (+968 -58) |
fix punctuation
Commits: |
|
|||||||
---|---|---|---|---|---|---|---|---|
Diff: |
Revision 6 (+968 -58) |
pidgin/pidginaccountsenabledmenu.c (Diff revision 6) |
---|
If I've understood the connected signals correctly, this will never be true if you only have one account, since this is only triggered on account enabled/disabled.
If you have multiple accounts, then this may be true if the second account gets enabled after the first gets a connection, but that depends on some timing.
refresh the menu when an account is done connecting as well as when it is disconnected.
Commits: |
|
|||||||
---|---|---|---|---|---|---|---|---|
Diff: |
Revision 7 (+1014 -58) |
rebased
Commits: |
|
|||||||
---|---|---|---|---|---|---|---|---|
Diff: |
Revision 8 (+1014 -58) |
libpurple/tests/test_menu.c (Diff revision 8) |
---|
Maybe also a test with a
dynamic-target
, but with a name that's not passed topurple_menu_populate_dynamic_targets
?
rebased and addressed comments
Commits: |
|
|||||||
---|---|---|---|---|---|---|---|---|
Diff: |
Revision 9 (+1066 -58) |