[Help] Fix argument group ordering.#1015
Conversation
| priority = 2 | ||
| # get alphabetical list of groups and ensure they are in the group priority dictionary | ||
| group_list = sorted(list(set(group_list))) | ||
| for group in [g for g in group_list if g not in self.priorities]: |
There was a problem hiding this comment.
The code logic appears correct. One suggestion though, it usually concerns me for a loop around a collection also updates the collection itself.
If i get it right, the loop can be simplified as following. The group_list won't contains duplicate as you have used "set"
for group_name in group_list:
self.priorities[group] = priority
priority += 1There was a problem hiding this comment.
The problem here is that some of the groups are already in the static dictionary ('Resource Id', 'Global', etc.), so the recommended change would actually change those, which is not what I want. I only want to add entries not already in the static dictionary (and I don't want people to have to add their groups to the static dictionary).
There was a problem hiding this comment.
Thanks for pointing this out. Per our discussion, let us do filter at a line above when we sort the group
|
|
||
| def get_group_priority(self, group_name): | ||
| key = self.priorities.get(group_name, 0) | ||
| return "%06d" % key |
There was a problem hiding this comment.
just a question, why we need to expand to 6 digits, is the comparison based on string, not int?
There was a problem hiding this comment.
That was the logic Burt originally had, so I didn't want to deviate from without him here to review the change.
|
|
||
| def get_group_priority(self, group_name): | ||
| key = self.priorities.get(group_name, 0) | ||
| return "%06d" % key |
5 participants
Fixes #976. Previously any group not within the static "priorities" dictionary in the _help.py file was assigned a priority of 1, meaning many groups shared the same priority. When the parameters were then sorted alphabetically, it could result in multiple group labels being printed. This solution ensures argument groups not in the static dictionary (a) don't need to be added and (b) don't have overlapping priorities.