Change Summary:
rebased
Commits: |
|
||||||
---|---|---|---|---|---|---|---|
Diff: |
Revision 2 (+682) |
Review Request #1356 — Created March 17, 2022 and submitted
Information | |
---|---|
grim | |
pidgin/pidgin | |
default | |
1364, 1365 | |
Reviewers | |
pidgin | |
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 |
---|
Description | From | Last Updated |
---|---|---|
Should be the GIR link syntax. |
|
|
dynamic-target (or maybe the constant macro can be used?) |
|
|
Should be [func@Account.get_id] or whatever it is. |
|
|
recursing |
|
|
Should use the macro instead of the dynamic-target string? |
|
|
Not tabs. |
|
|
This is going to be confusing for func, because it will be receiving a GMenuModel that is not the one ... |
|
|
Possibly should assert that n == n_items and l == NULL at the end. |
|
|
Maybe rename to expected (in all tests). |
|
|
So this doesn't work when the account is first enabled because it doesn't have a connection. |
|
|
Missing period. |
![]() |
|
If I've understood the connected signals correctly, this will never be true if you only have one account, since this ... |
|
|
Quote dynamic-target, to match the example below. |
|
|
Should that not be a g_return_if_fail? |
|
|
Could be g_free(item)? |
|
|
Maybe also a test with a dynamic-target, but with a name that's not passed to purple_menu_populate_dynamic_targets? |
|
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) |