Skip to content

feat: Add cli_shortcuts to CLI settings #624

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

Merged
merged 3 commits into from
Jun 20, 2025

Conversation

karta9821
Copy link
Contributor

@karta9821 karta9821 commented May 28, 2025

It addresses to #623

Selected Reviewer: @dmontagu

@karta9821 karta9821 force-pushed the support-cli-aliases branch 5 times, most recently from ab6752c to 4eb7dd5 Compare May 28, 2025 16:23
@karta9821
Copy link
Contributor Author

@maxnoe Please verify if it is okay

@hramezani
Copy link
Member

@kschwab do you have time to take a look here?

@maxnoe
Copy link

maxnoe commented Jun 3, 2025

Thanks for the quick implementation, I played around with the branch here and it works as I would expect!

Can't comment too much about the code itself.

@karta9821 karta9821 force-pushed the support-cli-aliases branch from 4eb7dd5 to 9abab16 Compare June 3, 2025 08:26
@karta9821
Copy link
Contributor Author

please review

@hramezani
Copy link
Member

I am waiting for @kschwab for a review, as he implemented most of the CLI settings source code.

@kschwab
Copy link
Contributor

kschwab commented Jun 9, 2025

Thanks @karta9821 and @maxnoe for the PR. I am back from vacation. The only two items I see are:

  • Documentation needs to be added.
  • I would simplify and support multiple aliases, similar to how pydantic AliasChoices works. ie, Map[str, str | list[str]], where key should be the target field, and values are the aliases.

The only real concern I have is confusion between cli_alias and pydantic alias. Is it possible to give the config option a different name? @hramezani do you have thoughts here?

Other than those items, everything else looks good.

@hramezani
Copy link
Member

Thanks @kschwab for checking the PR

The only real concern I have is confusion between cli_alias and pydantic alias. Is it possible to give the config option a different name? @hramezani do you have thoughts here?

Agree, Do you have a suggestion for the config name?

@karta9821 Please address the points mentioned by @kschwab. Then we can merge the PR.

@karta9821
Copy link
Contributor Author

karta9821 commented Jun 11, 2025

Thanks @kschwab for checking the PR

The only real concern I have is confusion between cli_alias and pydantic alias. Is it possible to give the config option a different name? @hramezani do you have thoughts here?

Agree, Do you have a suggestion for the config name?

@karta9821 Please address the points mentioned by @kschwab. Then we can merge the PR.

Sure, I can adjust the code :)

How about cli_shortcuts

@hramezani
Copy link
Member

Thanks @kschwab for checking the PR

The only real concern I have is confusion between cli_alias and pydantic alias. Is it possible to give the config option a different name? @hramezani do you have thoughts here?

Agree, Do you have a suggestion for the config name?
@karta9821 Please address the points mentioned by @kschwab. Then we can merge the PR.

Sure, I can adjust the code :)

How about cli_shortcuts

Looks good to me. what do you think @kschwab

@kschwab
Copy link
Contributor

kschwab commented Jun 18, 2025

@hramezani @karta9821 yes, cli_shortcuts sounds good to me as well.

@karta9821 karta9821 force-pushed the support-cli-aliases branch from 9abab16 to 2c4d97f Compare June 19, 2025 08:01
@karta9821 karta9821 changed the title feat: Add cli_aliases to CLI settings feat: Add cli_shortcuts to CLI settings Jun 19, 2025
@karta9821 karta9821 force-pushed the support-cli-aliases branch 2 times, most recently from e2d1608 to 5a53e0b Compare June 19, 2025 08:05
@karta9821
Copy link
Contributor Author

Fixed

`cli_shortcuts` is a mapping of field target to field aliases.
@karta9821 karta9821 force-pushed the support-cli-aliases branch from 5a53e0b to 485a0fb Compare June 19, 2025 08:07
@hramezani
Copy link
Member

@karta9821 Thanks for the update. please add the documentation for it

Copy link
Contributor

hyperlint-ai bot commented Jun 19, 2025

PR Change Summary

Added support for CLI shortcuts in settings configuration, allowing users to define alternative argument names for command-line interface options.

  • Introduced cli_shortcuts option in SettingsConfigDict for defining alternative CLI argument names.
  • Provided examples for both flat and nested settings configurations using CLI shortcuts.
  • Documented behavior for shortcut collisions and precedence in settings.

Modified Files

  • docs/index.md

How can I customize these reviews?

Check out the Hyperlint AI Reviewer docs for more information on how to customize the review.

If you just want to ignore it on this PR, you can add the hyperlint-ignore label to the PR. Future changes won't trigger a Hyperlint review.

Note specifically for link checks, we only check the first 30 links in a file and we cache the results for several hours (for instance, if you just added a page, you might experience this). Our recommendation is to add hyperlint-ignore to the PR to ignore the link check for this PR.

@karta9821 karta9821 force-pushed the support-cli-aliases branch 2 times, most recently from 82ddf29 to 84eda6d Compare June 19, 2025 17:03
@karta9821 karta9821 force-pushed the support-cli-aliases branch from 84eda6d to f9c116c Compare June 19, 2025 17:04
@karta9821
Copy link
Contributor Author

Added

@hramezani
Copy link
Member

Thanks @karta9821 for the quick update.

@kschwab please take a final look when you have time.

@kschwab
Copy link
Contributor

kschwab commented Jun 20, 2025

Hey @karta9821 thanks for the changes! It's a great addition, looks good to me.

@hramezani hramezani merged commit 180e74e into pydantic:main Jun 20, 2025
20 checks passed
@hramezani
Copy link
Member

Thanks all

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

Successfully merging this pull request may close these issues.

5 participants