fix: validate default answer only after user input

[omus]

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 a placeholder in this case is to provide partial input to the answer to reduce the amount of typing required during interactive use.

[codecov[bot]]

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 97.90%. Comparing base (d106ea5) to head (7cbf6ed).

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     

Flag	Coverage Δ
unittests	97.90% <100.00%> (-0.10%)	:arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[sisp]

Although this use of default as an initial value for interactive prompting might have worked before v9.8.0, I think it doesn't align with the intended semantics of a default value. IMO, a default value should always be valid, i.e. it should be parsable accoding to type and satisfy all additional constraints (choices and/or validator expression if specified). Otherwise, running copier copy --defaults ... will always fail because it will use the invalid default value and not prompt for user input.

The question in your test case could be split into two questions – org and team – where org has a default value and team doesn't. But this might not be representative of your real use case.

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 --defaults will not prompt for user input. A question with only an initial value (not a default value) might always prompt for user input but pre-fill the answer with the initial value. We could think about adding a new question property like initial for this purpose, which would be mutually exclusive with default. It would only make sense for text questions though, not for choice and confirm questions. WDYT, @omus & @copier-org/maintainers?

[omus]

Although this use of default as an initial value for interactive prompting might have worked before v9.8.0, I think it doesn't align with the intended semantics of a default value. IMO, a default value should always be valid, i.e. it should be parsable accoding to type and satisfy all additional constraints (choices and/or validator expression if specified). Otherwise, running copier copy --defaults ... will always fail because it will use the invalid default value and not prompt for user input.

I'm not sure I fully agree with this. When using copier copy --defaults -d team=@copier-org/everyone would allow the template to be used with an invalid default as we would just check the user input and ignore the default entirely. Typically, Copier tends to perform validation on the answer supplied and doesn't verify that the answer is valid at each step.

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 when conditions. The end result of this is that users are often the ones to discover invalid defaults during interactive usage when Copier abends rather than letting the user change the invalid default. This can be worked around with -d` again but this approach seems user unfriendly. So if we want to ensure that defaults always valid we may want to go as far as always validating the defaults even when the question is skipped.

[omus]

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 --defaults will not prompt for user input. A question with only an initial value (not a default value) might always prompt for user input but pre-fill the answer with the initial value. We could think about adding a new question property like initial for this purpose, which would be mutually exclusive with default.

This is very similar to placeholder (which is kind of mutually exclusive with default) but the value would persist when the user types. If we decide to go in this direction maybe the syntax should be something like:

team:
  default:
    value: @copier-org/everyone
    partial: @copier-org/

With this then default.value could be used when using --defaults where as partial would only be used in interactive use. The user would initially see value (like placeholder now) and when a user starts typing it would be added to the partial value. These two could be combined in the initial output which would show the partial as user entered text where as the remaining value could be shown with a different formatting to indicate it is just a hint. For this to work value would always need to start with partial.

[sisp]

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.

So if we want to ensure that defaults always valid we may want to go as far as always validating the defaults even when the question is skipped.

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 default and when:

# 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 when: false question (aka "skipped question", or "computed value" if false is hardcoded), then the corresponding Jinja variable is defined and its value is the default value. Thus, a when: false question with a default value is only skipped during prompting, but its answer does exist in the Jinja context. This behavior probably makes the most sense for questions that are still meaningful in the context of the questionnaire but don't require a user answer (e.g., because the answer can be derived from previous answers). For example:

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 devcontainer variable is exposed in the Jinja context because it has a default value. This makes sense because the .devcontainer/devcontainer.json file shall be generated whenever dev container support is enabled, only customizations may contain IDE-specific settings.

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 when conditions

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 database_url question is only defined for the database engines "postgres" and "mysql" but not for "none", and we can't render, e.g., an empty string as the default value for "none" because the validator would fail. We could surround the actual validator condition with {% if database_engine != 'none' %}...{% endif %}, but that feels hacky and redundant with the when expression. What we'd actually want is to fully skip this question – I'd say by declaring the default value as undefined. An undefined default answer of a skipped question would mean that there is no answer, so the validator wouldn't be applied. With this, IMO it seems perfectly fine to require default values to always pass validation. Support for unsetting default values is a feature I've had in mind for a while, so I took the liberty and drafted a PR: #2286 WDYT?

About your idea to have a partial default value: We'd need to check whether questionary/prompt-toolkit support this level of UX. But I'm not yet convinced by the need for this feature because this example can be refactored such that the partial default value isn't needed. Do you have a real use case – and if so, are you able to share it?