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

add raw-output option for json output #900

Merged
merged 13 commits into from Dec 30, 2022

Conversation

adonisgarciac
Copy link
Contributor

@adonisgarciac adonisgarciac commented Dec 28, 2022

When you run this action with json: true, the output doesn't work properly because double quotes are escaped in the content of array:

code:

      - name: Get changed files
        id: changed-files
        uses: tj-actions/changed-files@v35
        with:
          json: true

      - uses: distributhor/workflow-webhook@v2
        env:
          webhook_type: 'json-extended'
          webhook_url: ${{ secrets.WEBHOOK_URL_DEV }}
          webhook_secret: ${{ secrets.WEBHOOK_SECRET_DEV }}
          data: '{ "modified_files": ${{ steps.changed-files.outputs.all_changed_files }} }'

output

    data: { "modified_files": [\".github/workflows/github-dev-webhook.yml\",\"file.yml\"] }

Error:

parse error: Invalid numeric literal at line 1, column 23

If I check the code where json is parsed:

$ echo hello | jq -R 'split("|") | @json'| sed -r 's/^"|"$//g' | tr -s /
[\"hello\"]

With this change, the output will be:

$ echo hello | jq -r -R 'split("|") | @json'| sed -r 's/^"|"$//g' | tr -s /
["hello"]

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for implementing a fix, could you ensure that the test covers your changes if applicable.

@jackton1
Copy link
Member

jackton1 commented Dec 28, 2022

@adonisgarciac Thanks for submitting this PR. I’ll suggest we make this an optional behaviour via an input I.e json-raw: true. Since this breaks the usage of the action with fromJSON.

See: https://docs.github.com/en/actions/learn-github-actions/expressions#fromjson

Failed matrix job: https://github.com/tj-actions/changed-files/actions/runs/3795491847

Copy link
Member

@jackton1 jackton1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add optional input to return raw JSON output from jq. You’ll need to abstract the behaviour into a function that can be used with pipe

@adonisgarciac
Copy link
Contributor Author

Add optional input to return raw JSON output from jq. You’ll need to abstract the behaviour into a function that can be used with pipe

Thank you @jackton1. You are totally right! Let me know if there are some mistakes in the last changes that I have done with jq_raw_input as a variable.

@jackton1
Copy link
Member

jackton1 commented Dec 30, 2022

Nice work @adonisgarciac made some quick changes the arguments for jq would just need to be controlled internally except there's a real use case for supporting more arguments.

@all-contributors please add @adonisgarciac for code, docs

@allcontributors
Copy link
Contributor

@jackton1

I've put up a pull request to add @adonisgarciac! 🎉

@jackton1 jackton1 added the merge when passing Merge the PR automatically once all status checks have passed label Dec 30, 2022
@jackton1 jackton1 merged commit 171fd35 into tj-actions:main Dec 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge when passing Merge the PR automatically once all status checks have passed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants