search

fermitools / htgettoken

Gets OIDC authentication tokens for High Throughput Computing via a Hashicorp vault server
Other
5 stars 4 forks source link

Handle complicated defaults with functions #75

Closed duncanmmacleod closed 1 year ago

duncanmmacleod commented 1 year ago

This PR refactors the default-handling for -o/--outfile and --web-open-command into functions that are called directly when calling add_option(), rather than after-the-fact.

This has the upside that the -h/--help message now shows exactly what the default will be, rather than a statement of logic.

@DrDaveD, one feature that I removed here, is that if the user deliberately passes a string that contains %uid, e.g. via -o ./bt_%uid, that is no longer expanded. However, I have no idea if that is 'supported' formally. Happy to put that back in as requested.

DrDaveD commented 1 year ago

The %uid is documented in the man page, so it is formally supported and needs to stay there.

More significantly, it isn't clear to me that showing the current default value is better than the statement of logic. I think the statement of logic is helpful and that changing help messages depending on the situation can be confusing. Is there another reason for the change that you didn't mention?

duncanmmacleod commented 1 year ago

This change was just my attempt to make the argument parsing more compact, relying on less post-processing of the various options. I'm don't have strong opinions on this really, so if you decide to reject I'm happy to move on.

DrDaveD commented 1 year ago

Ok let's skip this one.