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

Use str.replaceAll() with Slack link formatter function #24

Merged
merged 5 commits into from
Jul 31, 2024

Conversation

magnetikonline
Copy link
Member

@magnetikonline magnetikonline commented Jul 31, 2024

Was incorrectly using str.replace() to escape &<> characters - only would replace first instance - rather than all.

https://api.slack.com/reference/surfaces/formatting#linking-urls

Granted, none of the URLs currently passed to function include these characters - but no harm in making better.

Also:

  • Bumped esbuild version.
  • Added semi rule to ESLint ruleset.

Object.defineProperty(o, k2, { enumerable: true, get: function() {
return m[k];
} });
} : function(o, m, k, k2) {
if (k2 === void 0)
k2 = k;
if (k2 === void 0) k2 = k;
Copy link
Member Author

Choose a reason for hiding this comment

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

Changes to build output with latest esbuild release.

@magnetikonline magnetikonline force-pushed the slack-link-replace-all branch from 585e978 to 77a40b8 Compare July 31, 2024 05:20
}

url = url.replace('<','&lt;');
Copy link
Member Author

@magnetikonline magnetikonline Jul 31, 2024

Choose a reason for hiding this comment

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

This "double escape" was in error 🤦

But again, current uses of makeSlackLink() - would not have hit this issue, but fixing up since it's still incorrect/wrong.

@magnetikonline
Copy link
Member Author

Thx @jasonltang 👍

@magnetikonline magnetikonline merged commit cefbc93 into main Jul 31, 2024
3 checks passed
@magnetikonline magnetikonline deleted the slack-link-replace-all branch July 31, 2024 05:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants