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

Using --data with a multiselect based choice #1474

Open
chrisdaish opened this issue Jan 9, 2024 · 8 comments
Open

Using --data with a multiselect based choice #1474

chrisdaish opened this issue Jan 9, 2024 · 8 comments
Labels
Milestone

Comments

@chrisdaish
Copy link

Describe the problem

Follow on from: https://github.com/orgs/copier-org/discussions/1473

Template

fruit:
    type: str
    help: "Fruit?"
    choices:
        - apple
        - pear
        - banana
    multiselect: true

To Reproduce

$ copier copy --data 'fruit=apple' ../copier-issue .
...
ValueError: Invalid choice

Logs

No response

Expected behavior

Primary:
--data is able to find the 'apple' from the 'fruit' multiselect based choice, and select it.

Secondary:
To support multiple options perhaps multiple --data entries are required akin to:

--data 'fruit=apple' --data 'fruit=banana'

or comma separated

--data 'fruit=apple, banana'

or an array?

--data 'fruit=[apple, banana]'

Screenshots/screencasts/logs

No response

Operating system

macOS

Operating system distribution and version

Sonoma 14.2.1

Copier version

copier 9.1.0

Python version

Python 3.9.6

Installation method

pip+pypi

Additional context

No response

@chrisdaish chrisdaish added bug triage Trying to make sure if this is valid or not labels Jan 9, 2024
@noirbizarre
Copy link
Contributor

My 2 cts (as the author of the multiselect feature):

Given https://github.com/copier-org/copier/blob/master/copier/cli.py#L162-L178 is already a list, and current behavior is "last value overrides previous", I would avoid the multiple flags approach as it would mean different behavior and different parsing strategies.

Between --data 'fruit=apple,banana' and --data 'fruit=[apple, banana]', I would chose the first one based on this:

  • I think comma separated list are more common as parameter and potentially simpler to parse
  • this is very personal, but I don't like CLI expecting JSON-like data: most of the time it's a lot of noise for nothing (accepting JSON but expecting only a subset which most of the time is easier to write in basic cli style), and often double escaping required
  • if it's not JSON, it means it needs a custom parser and so more complexity and maintenance than splitting on commas

@sisp
Copy link
Member

sisp commented Jan 10, 2024

I think comma-separated values won't work in general, e.g. type: yaml or type: json might contain commas which should not be used to split multiple answer values for a multiselect question. 🤔

@yajo
Copy link
Member

yajo commented Jan 10, 2024

Have you actually tried the array way? Something like:

copier copy --data 'fruit=[apple]' ../copier-issue .

@sisp
Copy link
Member

sisp commented Jan 11, 2024

@yajo I've tried several ways, also with an array, none works. The problem is that the answer passed via CLI is a string, which gets parsed by

answer = question.parse_answer(result.init[var_name])

which for multiselect questions does not work as expected because a list is expected:

copier/copier/user_data.py

Lines 439 to 440 in f92c1fe

if self.multiselect:
return [self._parse_answer(a) for a in answer]

I'm not sure what the best solution to this problem is though. Should we support string answers for all questions, which get parsed according to the question type?

@yajo
Copy link
Member

yajo commented Jan 15, 2024

I think the we just need a specific parser for multiselect questions. It could be a yaml that asserts the value becomes a list and casts its elements to the type of the question.

@lhupfeldt
Copy link

I just stumbled into this error. I also tried all the combinations of passing date mentioned above before deciding to open an issue, an then found this existing one 🥲
Is there really a need for allowing , in (multiselect) choice values?
I would like to vote for the simple comma separated format. If needed '\,' could be used on the commandline to allow ,.
Not being able to specify all data on the commandline is a real showstopper for automation.

@johannesloibl
Copy link

I have the same problem. I would prefer any solution, may it be comma-separated or JSON.
Until then it is: modify .copier-answers.yml, commit and update :(

@yajo yajo removed the triage Trying to make sure if this is valid or not label Aug 29, 2024
@yajo yajo added this to the Soon milestone Aug 29, 2024
@chrisdaish
Copy link
Author

chrisdaish commented Sep 4, 2024

@johannesloibl rather than editing the .coper-answers.yml file directly - we have had more success by using the --data-file argument (--data-file fruit.yml for example) that we then author before hand (programatically) to include the following:

---
fruit:
- apple
- banana

kenden added a commit to kenden/copier that referenced this issue Sep 20, 2024
The documentation to update, changing only one answer  when the question is a multi-select base choice, is missing.

This is from copier-org#1474 (comment), but it also indicate to create the `.yaml` file in another directory (to avoid error `Destination repository is dirty; cannot continue. Please commit or stash your local changes and retry.`).
yajo added a commit that referenced this issue Sep 22, 2024
* Document how to update when changing a multiselect base answer

The documentation to update, changing only one answer  when the question is a multi-select base choice, is missing.

This is from #1474 (comment), but it also indicate to create the `.yaml` file in another directory (to avoid error `Destination repository is dirty; cannot continue. Please commit or stash your local changes and retry.`).

* style: autoformat with pre-commit

* docs: adapt to global copywriting style

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Jairo Llopis <973709+yajo@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants