COVESA / vss-tools

Software for working with VSS (https://github.com/COVESA/vehicle_signal_specification)
Mozilla Public License 2.0
54 stars 55 forks source link

Possible naming conflict -o and -ot #339

Closed erikbosch closed 2 months ago

erikbosch commented 6 months ago

As of today we support -o for overlays for --ot for type output when using structs. When working on enabling -ot only for cases when actually relevant I noticed that -ot then can get intepreted as -o t and you get a totally irrelevant error message.

I see some options for vss-tools 5.0:

erik@debian4:~/vss-tools/tests/vspec/test_structs$ ../../../vspec2ddsidl.py -vt VehicleDataTypes.vspec -u ../test_units.yaml -ot expt.json test.vspec exp.json
usage: vspec2ddsidl.py [-h] [-I dir] [-e EXTENDED_ATTRIBUTES] [-s] [--abort-on-unknown-attribute] [--abort-on-name-style] [--uuid] [-o overlays] [-q quantity_file] [-u unit_file]
                       [-vt vspec_types_file] [--all-idl-features]
                       <vspec_file> <output_file>
vspec2ddsidl.py: error: unrecognized arguments: exp.json
erik@debian4:~/vss-tools/tests/vspec/test_structs$ 
ppb2020 commented 6 months ago

I note that there is a -vt options that may have the same issue. It's nice to have the short options, but you can run out of letters...

Does the option parsing code we use allow for non-conflicting shortening of long options? That is, would --abort be allowed for --abort-on-unknown-attribute if there is no other option that starts with --abort? If so, not having short options for optional options that are not not used often would work well.

Using -t instead of -ot is one way around if short options are necessary, but may be harder or confusing to remember. Would -O be another option.

erikbosch commented 6 months ago

I think it is recommended to only use single characters for short options, so both -vt and -ot are "not recommended". Reason is that arg parser consider for example -x3 and -x 3 to be equal. -vt is currently not a problem as we have no -v

erikbosch commented 6 months ago

MoM: