Show flag aliases before auto-generated names in helptext #247
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #247 +/- ##
=======================================
Coverage 99.78% 99.78%
=======================================
Files 28 28
Lines 2326 2326
=======================================
Hits 2321 2321
Misses 5 5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -611,4 +611,4 @@ def _rule_apply_argconf( | |||
| if arg.field.argconf.metavar is not None: | |||
| lowered.metavar = arg.field.argconf.metavar | |||
| if arg.field.argconf.aliases is not None: | |||
| lowered.name_or_flags = lowered.name_or_flags + arg.field.argconf.aliases | |||
| lowered.name_or_flags = arg.field.argconf.aliases + lowered.name_or_flags | |||
There was a problem hiding this comment.
Ah. I guess that since all of the aliases are part of the same list, it would require extra work to pull just the short ones out and place them before everything else. In the issue that I filed, I wasn't looking to flip the order of the destination variable-derived name (lowered.name_or_flags) versus all of the aliases... only the short options (e.g., -d.).
However, I'm not against this change. Don't know if it might cause too much surprise to other Tyro users.
There was a problem hiding this comment.
Yeah, I think all of the options seemed similar:
- Pull short args only to the front
- Sort by length
- Just order as
(*user args, automatic arg)
But the final one just seemed the least magical
There was a problem hiding this comment.
That makes sense: keep it simple. Hopefully none of the other users will complain about the change in order.
Fixes #245