Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

command-line params don't override config (again) #69

Open
jepler opened this issue Jul 11, 2022 · 2 comments
Open

command-line params don't override config (again) #69

jepler opened this issue Jul 11, 2022 · 2 comments
Labels

Comments

@jepler
Copy link

jepler commented Jul 11, 2022

Similar to #65, there's another scenario where command-line params do not correctly override config. I originally reported this as though it was a problem in qmk (qmk/qmk_firmware#17624) but I think the problem may be inside milc.

Test program (hello_milc.py)

from milc import cli
import milc.subcommand.config
import pprint

@cli.argument('-j', '--parallel', type=int, default=1, help="Set the number of parallel make jobs; 0 means unlimited.")
@cli.entrypoint('configuration and args')
def main(cli):
    print(f"jobs info: {cli.args.parallel=} {cli.config.general.parallel=}")
    pprint.pprint(cli.config_source)

if __name__ == '__main__':
    cli()

Out of all the various ways to specify the parallel value, -jN still doesn't work. -j N, --parallel N and --parallel=N all work.

jepler@bert:~$ python3 hello_milc.py config general.parallel=3
general.parallel: 1 -> 3
ℹ Wrote configuration to /home/jepler/.config/hello_milc/hello_milc.ini
jepler@bert:~$ python3 hello_milc.py -j 5
jobs info: cli.args.parallel=5 cli.config.general.parallel=5
{'general': {'parallel': 'argument'}}
jepler@bert:~$ python3 hello_milc.py -j5
jobs info: cli.args.parallel=5 cli.config.general.parallel=3
{'general': {'parallel': 'config_file'}}
jepler added a commit to jepler/milc that referenced this issue Jul 11, 2022
Before, _index_arg (and _in_argv) would not function correctly in this
case.

This change makes the recently added tests pass and closes clueboard#69.
@github-actions
Copy link

This issue has been idle for 6 months and will be closed in 2 months if the stale lable is not removed.

@github-actions github-actions bot added the Stale label Jun 14, 2023
@jepler
Copy link
Author

jepler commented Jan 27, 2024

repeating @skullydazed 's comment from #70:

this particular approach leads to some confusing results if you try to combine short style flags in other ways, EG using -sv instead of -s -v. I don't remember all the details now, but last I looked into it the basic problem is that I can't get the necessary information from argparse to unambiguously parse these types of flags consistently with how argparse does it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant