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

feat(core): handle comma seperated lists in getMultilineInput #1183

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rickstaa
Copy link

@rickstaa rickstaa commented Sep 25, 2022

This PR makes sure that the core.getMultilineInput function can also parse comma-separated lists (i.e. [val1,val2,val3] and val1,val2,val3).

Fixes #184 (comment).

@rickstaa rickstaa requested a review from a team as a code owner September 25, 2022 07:40
@rickstaa rickstaa force-pushed the improve_getMultiLineInput branch 3 times, most recently from d74927b to 6224d95 Compare September 25, 2022 08:24
This commit makes sure that the core.getMultilineInput function can also
parse comma-separated lists (i.e. `[val1,val2,val3]` and `val1,val2,val3`).
@DmitriyBobrovskiy
Copy link

any update on this?

@shoogle
Copy link

shoogle commented Sep 3, 2023

It would be better to put this in a new function core.getCommaSeparatedInput() or core.getJsonInput().

People might already be using core.getMultilineInput() with list items that contain commas, so it's not safe to start treating commas as a list separator retrospectively.

- uses: myaction
  with:
    multiline-param: |
      item1, with comma
      item2 no comma
      item3, with, several, commas

This list has 3 items according to the current implementation, but your code would parse it as 7 items, potentially breaking the workflow.

@rickstaa
Copy link
Author

rickstaa commented Oct 19, 2023

It would be better to put this in a new function core.getCommaSeparatedInput() or core.getJsonInput().

People might already be using core.getMultilineInput() with list items that contain commas, so it's not safe to start treating commas as a list separator retrospectively.

- uses: myaction
  with:
    multiline-param: |
      item1, with comma
      item2 no comma
      item3, with, several, commas

This list has 3 items according to the current implementation, but your code would parse it as 7 items, potentially breaking the workflow.

Good comment 👍🏻! I've opened issue #1566 to tackle your concerns. Regarding getCommaSeparatedInput, we could also introduce a more versatile getJsonInput function. Unfortunately, I do not have the time right now to implement this 😅.

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 this pull request may close these issues.

Support list input type
4 participants