这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@jere8184
Copy link
Contributor

No description provided.

@jere8184 jere8184 force-pushed the suppress_add_dll_search_path branch 8 times, most recently from 32c138d to 1525fac Compare December 29, 2024 20:49
@jere8184 jere8184 changed the title Suppress add dll search path Suppress "--add_dll_search_path" in sub-parser Dec 29, 2024
@TheJJ
Copy link
Member

TheJJ commented Dec 29, 2024

why is this change needed? the parameter is added to the toplevel cli on windows only already?

@jere8184
Copy link
Contributor Author

why is this change needed? the parameter is added to the toplevel cli on windows only already?

if you fail to provide a subcommand this parameter will be passed to the 'main' sub-parser causing it to fail with an error.
run locally (on windows)
'python -m openage --add-dll-search-path <valid_path>'
and see for yourself.

you should get the following error
'openage main: error: unrecognized arguments: --add-dll-search-path <valid_path>'
which is pretty confusing.

This change stops an error from occurring and invokes 'openage main' as if no arguments have been passed.

@TheJJ
Copy link
Member

TheJJ commented Dec 30, 2024

ah, then the proper solution is to add this as a common parent to the subparsers that make use of the option. duplication is not the proper way, unfortunately.

@jere8184 jere8184 force-pushed the suppress_add_dll_search_path branch from 1525fac to 0a03e2d Compare December 31, 2024 13:18
@jere8184
Copy link
Contributor Author

@TheJJ i've went a slightly different, route see changes.

Copy link
Member

@heinezen heinezen left a comment

Choose a reason for hiding this comment

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

Looks good to me

@heinezen heinezen merged commit b35c9bf into SFTtech:master Dec 31, 2024
13 checks passed
@jere8184 jere8184 deleted the suppress_add_dll_search_path branch January 7, 2025 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants