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

Disallow str-parseable input by API #280

Closed
yajo opened this issue Sep 26, 2020 · 2 comments · Fixed by #295
Closed

Disallow str-parseable input by API #280

yajo opened this issue Sep 26, 2020 · 2 comments · Fixed by #295

Comments

@yajo
Copy link
Member

yajo commented Sep 26, 2020

Copier allows API input such as "true" for a bool question, and converts that to True. And the same for all data types.

The reasoning behind this is that CLI always sends strings.

But this overcomplicates the implementation and makes CLI arguments more surprising, as they can get casted to different answers depending on the question type.

Gotta remove that nonsense. Instead, make the CLI app transform all data before sending it to the API. And use always the same transform method: YAML.

@yajo yajo added this to the v6.0.0 milestone Sep 26, 2020
yajo pushed a commit that referenced this issue Oct 19, 2020
If calling Copier through CLI, it's normal to convert all strings to expected data, but if calling through API, you should know what kind of data you're passing in, so it shouldn't try to be smarter than you.

Fix #280.
@yajo
Copy link
Member Author

yajo commented Oct 23, 2020

After developing the "fix" in #295 I just realized that it's better how Copier handles this now. It makes the same data conversions if data comes from CLI, from API, from copier-answers or from user answers. That's the most expected use IMHO, so closing.

@yajo yajo closed this as completed Oct 23, 2020
@yajo yajo removed this from the v6.0.0 milestone Oct 23, 2020
@yajo
Copy link
Member Author

yajo commented Jan 23, 2021

Still reconsidering this... 🤔

@yajo yajo reopened this Jan 23, 2021
yajo added a commit that referenced this issue Jan 31, 2021
- Removal of Questionary class, which only had 1 meaningful method that is now merged into Worker to avoid circular dependencies.
- Fix #280 in a simple way: only user answers are type-casted inside API, and CLI transforms all `--data` using YAML always. Predictable.
- Use prereleases correctly.
- Reorder AnswersMap to have a more significative repr.
- Simpler cache for old `Question.get_choices()` method (renamed now).
@yajo yajo closed this as completed in 0441b86 Feb 9, 2021
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 a pull request may close this issue.

1 participant