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.

Sign up for GitHub

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

update test to work with changes from #9 #12

Merged
merged 7 commits into from
Aug 30, 2023
Merged

update test to work with changes from #9 #12

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
eff338c
update test to work with changes from #9
JoshCoady Aug 30, 2023
4239c40
fix yaml format issue
JoshCoady Aug 30, 2023
f13f562
fix missing new line
JoshCoady Aug 30, 2023
8b414ef
Update main.yml
JoshCoady Aug 30, 2023
6ef31a1
Update main.yml
JoshCoady Aug 30, 2023
fb131bc
Update main.yml
JoshCoady Aug 30, 2023
942c8f2
Update main.yml
JoshCoady Aug 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 3 additions & 11 deletions .github/workflows/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,11 @@ jobs:
shell: bash
env:
ACTUAL: ${{ steps.convert.outputs.text }}
EXPECTED: |
*Heading 1*

*Heading 2*

<http://example.com|Link>

• List Item 1
• List Item 2
EXPECTED: '*Heading 1*\n\n*Heading 2*\n\n<http://example.com|Link>\n\n• List Item 1\n• List Item 2\n'
Copy link
Contributor

@bachiri bachiri Aug 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

HI @JoshCoady I just want make sure is this the intended behaviour of the action because \n will be visible in the slack message, I think the PR you merged(v1.0.2) made the \n visible in the output text Like:
Captura de pantalla 2023-08-30 a las 17 48 04

The version 1.0.0
Captura de pantalla 2023-08-30 a las 17 50 27

Copy link

@denist-huma denist-huma Aug 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@bachiri
This the way to use multi-line text in app-generated text for the Slack API, please look https://api.slack.com/reference/surfaces/formatting#line-breaks

I had an issue about it #8

Copy link
Contributor

@bachiri bachiri Aug 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks @denist-huma for your response.

To give you more context the input that I'm getting is from Github cli Release notes Example :

{"body":"## What's Changed\n* Test slakify Text by @bachiri in [https://github.com/repo/pull/22\n\n\n**Full].....

So when applying the Slakify action into the input using the version 1.0.0 it works perfectly(The second screen shoot) but when I apply the second version 1.0.2 I got the result in the first screenshot .

If I'm not mistaken the changes you have done are specific to your case because from what I understood this action is just a wrapper of slackify-markdown which is the best place to push your change(If it applies ).

Can you share the input you are giving the action?

Copy link

@denist-huma denist-huma Aug 31, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@bachiri GitHub/Gitlab are expecting to get markdown, WYSIWYG, a string code \n for example. But Slack API expects to receive a readable \\n in string code. Please look https://api.slack.com/reference/surfaces/formatting#line-breaks.

This leads to that you got to process the output text that can be displayed in GitHub once more time for Slack API to not to break.

My input from #8 has 2 line breaks in it.

**Your server API is ready to use.✅**

The server API links:

Copy link

Choose a reason for hiding this comment

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

If I'm not mistaken the changes you have done are specific to your case because from what I understood this action is just a wrapper of slackify-markdown which is the best place to push your change(If it applies ).

I disagree that the changes I have done are specific to my case. I think that it should fix the bug for everybody who use multi-line strings.

run: |
if [ "$ACTUAL" != "$EXPECTED" ]; then
echo "Actual: $ACTUAL"
echo "Expected: $EXPECTED"
echo "Actual: '$ACTUAL'"
echo "Expected: '$EXPECTED'"
exit 1
fi

Expand Down