search

FHIR / sushi

SUSHI (aka "SUSHI Unshortens Short Hand Inputs") is a reference implementation command-line interpreter/compiler for FHIR Shorthand (FSH).
Apache License 2.0
145 stars 44 forks source link

Override sushi-config options from command line #1442

Closed LodewijkSioen closed 5 months ago

LodewijkSioen commented 7 months ago

1439

Added command line option to override some elements in sushi-config.yaml:

 -c, --config <config>    override elements in sushi-config.yaml (supported: 'version', 'status', 'releaselabel') (eg: --config status:draft)
cmoesel commented 7 months ago

Thank you for the contribution, @LodewijkSioen! We're currently discussing this with the team. We're trying to decide if the way you've done it is best (which is very clear but adds three new options to the CLI) or if it might be better to introduce a single option for which the thing you're overriding is part of the value (e.g., -ig version:1.2.3 -ig status:draft -ig releaselable:qa-draft). The single option is a tiny bit harder to use, but allows for easily supporting other overrides without continuing to increase the number of different option flags. Do you have any thoughts on this?

LodewijkSioen commented 7 months ago

@cmoesel I'm ok with that. I can imagine the option bloat. However, I don't see a way to do that using commander.js except custom option parsing. This would introduce a bunch of parsing logic. Alternatively, the IG override options could be a json string.

One drawback is that the built-in --help documentation will be less useful.

cmoesel commented 6 months ago

Hi @LodewijkSioen. I apologize for the delay in responding. We've discussed this some more as a team, and we do think that this feature would work better as a single option (perhaps -c / --config since you're overriding config data). We feel that this is actually more user-friendly because having too many command-line options can be overwhelming.

I don't think having the user express overrides as a JSON string would be very user-friendly. But I also don't think that you need to implement this using commander's custom option parsing. I'd suggest that you specify the option as a variadic option. This would allow users to pass the option multiple times (e.g., -c version:1.2.3 -c status:draft), which makes the resulting option value an array (e.g., ['version:1.2.3', 'status:draft']). Then in updateConfig, you just iterate that array, split each item on : to get your key and value, and set it in the config (with perhaps a gating function to ensure only allowed keys are used).

How does that sound to you?

LodewijkSioen commented 6 months ago

Sounds good to me. This seems more elegant and in line with how most command line options work. I can take a stab at it the week after next.

LodewijkSioen commented 6 months ago

@cmoesel is this more like it?

LodewijkSioen commented 5 months ago

Prettier should be fixed now.

LodewijkSioen commented 5 months ago

Release notes updated