-
-
Notifications
You must be signed in to change notification settings - Fork 225
fix: validate default answer only after user input #2278
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
base: master
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2278 +/- ##
==========================================
- Coverage 98.00% 97.90% -0.10%
==========================================
Files 55 55
Lines 6106 6115 +9
==========================================
+ Hits 5984 5987 +3
- Misses 122 128 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Although this use of The question in your test case could be split into two questions – Perhaps Copier is missing a feature that allows to provide an initial value to a required question, and this value does not need to pass validation. A question with a default value is considered optional in the sense that passing |
I'm not sure I fully agree with this. When using The other issue I see with enforcing that defaults have to be valid is currently if an invalid default is used accidentally in a template it is easy to miss such mistakes if you questions are skipped with |
This is very similar to team:
default:
value: @copier-org/everyone
partial: @copier-org/ With this then |
I think that copier copy ... # ✅ (because of valid interactive input)
copier copy --defaults ... # ❌ (because of invalid default value)
copier copy --defaults -d team=@copier-org/everyone ... # ✅ (because of valid non-interactive input)
copier copy -d team=@copier-org/everyone ... # ✅ (because of valid non-interactive input) would be inconsistent. It is not up to a template author to decide which alternative is used by a template user, so they should all work.
I tend to agree. Specifying a validator in a question spec is conceptually similar to type refinement in runtime validation libraries (e.g., Zod's refinement), adding constraints to a base type. Off the top of my head, I can't think of a reason why it should not apply to default values. That said, I think it's important to be clear about the (current) interplay of # copier.yml
q1:
type: str
when: false
q2:
type: str
default: foo
when: false
_message_after_copy: |
q1: {{ q1 is undefined }}
q2: {{ q2 == 'foo' }} $ copier copy ...
...
q1: True
q2: True When a default value is specified for a editors:
type: str
help: IDE integrations
choices:
- vscode
- pycharm
- sublime
devcontainer:
type: bool
help: Enable dev container support
default: false
when: "{{ 'vscode' in editors }}" Here, VS Code and PyCharm have dev container support, so the user may choose to enable it. But the question is skipped when Sublime Text has been selected because it doesn't (seem to) support dev containers. In all cases, the
Yes, there might be use cases where a question should be skipped and its Jinja variable should be undefined (i.e., not present in the render context) because it has no meaningful default value. For example: database_engine:
type: str
help: Database engine
choices:
- postgres
- mysql
- none
default: postgres
database_url:
type: str
help: Database URL
default: >-
{%- if database_engine == 'postgres' -%}
postgresql://user:pass@localhost:5432/dbname
{%- elif database_engine == 'mysql' -%}
mysql://user:pass@localhost:3306/dbname
{%- endif -%}
when: "{{ database_engine != 'none' }}"
# Simplified for illustration purposes
validator: "{% if '://' not in database_url %}Invalid{% endif %}" Here, the default value of the About your idea to have a partial default value: We'd need to check whether |
Follow up to #2145. Copier versions before 9.8.0 used to allow invalid defaults as long as the user would modify them to make them valid before continuing. This PR restores that old behavior while still ensuring that
validator
checks are still run on default when--defaults
is used.The rational for using a
default
instead of aplaceholder
in this case is to provide partial input to the answer to reduce the amount of typing required during interactive use.