Skip to content

Move from sys.argv to argparse - #6

Merged
HaroldCindy merged 2 commits into
secondlife:mainfrom
FelixWolf:argparse
Jul 15, 2025
Merged

Move from sys.argv to argparse#6
HaroldCindy merged 2 commits into
secondlife:mainfrom
FelixWolf:argparse

Conversation

@FelixWolf

@FelixWolf FelixWolf commented Jul 11, 2025

Copy link
Copy Markdown
Contributor

Hoping I understood the "No API changes" policy in the contributing correctly here. (Interpretted it as "No changes to the LSL api", rather than "No changes to the script")

Currently the script is using sys.argv for parsing arguments, which works, with some caveats.
If a argument is missing, it raises a IndexError, rather than an error message on what type of argument is missing.
In addition, there isn't a -h / --help to help with listing arguments and available parameters. While this is obtainable by reading the source code, I believe that it would be more helpful to have self documenting arguments.

Before:

[felix@crocuta lsl-definitions]$ python3 gen_definitions.py
Traceback (most recent call last):
  File "/home/felix/source/other/lsl-definitions/gen_definitions.py", line 1968, in <module>
    main()
    ~~~~^^
  File "/home/felix/source/other/lsl-definitions/gen_definitions.py", line 1931, in main
    definitions = parser.parse_file(sys.argv[1])
                                    ~~~~~~~~^^^
IndexError: list index out of range

After

[felix@crocuta lsl-definitions]$ python3 gen_definitions.py
usage: gen_definitions.py [-h]
                          definitions
                          {syntax,gen_constant_lsl_script,gen_lexer_file,gen_parser_file,gen_tree_header_file,gen_tree_source_file,gen_cpp_constants,gen_func_call_scripts,gen_lscript_library_defs,gen_mono_library_defs,gen_lscript_interface,gen_mono_bind_interfaces,gen_lscript_library_bind_pure,gen_builtins_txt,gen_lua_registrations} ...
gen_definitions.py: error: the following arguments are required: definitions, mode


[felix@crocuta lsl-definitions]$ python3 gen_definitions.py lsl_definitions.yaml gen_lexer_file --help
usage: gen_definitions.py definitions gen_lexer_file [-h] template_file output_file

positional arguments:
  template_file
  output_file

options:
  -h, --help     show this help message and exit

This change was done in a manner to ensure that the arguments follow the same exact syntax as before, as such existing scripts will not break.
There is room for improvement if existing scripts could be modified, for example with gen_lua_registrations, instead of specifying 0 or 1, we could do --pure as a optional argument, or using argparse's FileType to allow output to stdin / stdout. However a change like that I feel needs to be further looked into and approved, to not break internal scripts that rely on this.
I feel, for now, the current changes are sufficient enough to help people further test yaml changes and improvements they make.

@HaroldCindy HaroldCindy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable to me, thanks!

@HaroldCindy

Copy link
Copy Markdown
Collaborator

@FelixWolf Looks like it might require some formatting changes to pass CI https://github.com/secondlife/lsl-definitions/actions/runs/16227903266/job/45978129373?pr=6

@HaroldCindy
HaroldCindy merged commit 2d80dda into secondlife:main Jul 15, 2025
3 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 15, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants