Skip to content
This repository was archived by the owner on Oct 10, 2024. It is now read-only.

Conversation

@Bashamega
Copy link
Member

Fixes Issue

closes #783

Changes proposed

Check List (Check all the applicable boxes)

  • My code follows the code style of this project.
  • My change requires changes to the documentation.
  • I have updated the documentation accordingly.
  • All new and existing tests passed.
  • This PR does not contain plagiarized content.
  • The title of my pull request is a short description of the requested changes.

Screenshots

Note to reviewers

@Bashamega
Copy link
Member Author

@eddiejaoude

): Promise<EmbedBuilder[]> => {
const embeds: EmbedBuilder[] = [];
try {
const urlPattern = /https?:\/\/[^\s]+/g;
Copy link
Member

Choose a reason for hiding this comment

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

Could we use the existing link regex check from the other file? Maybe move to the config?

const urlPattern =
/[Hh][Tt][Tt][Pp][Ss]?:\/\/([\w-]+(\.[\w-]+)+)(\/[\w-./?%&=]*)?/g;

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure

Copy link
Member

Choose a reason for hiding this comment

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

What about moving it to a reusable config, so we don't have duplication? urlRegex.js

https://github.com/EddieHubCommunity/EddieBot/tree/9fc6b257c48b594fb3a48b421cccfabbef6a7a08/src/config

Copy link
Member

@eddiejaoude eddiejaoude left a comment

Choose a reason for hiding this comment

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

Thank you! Looks good 👍

Just 1 comment about reusing existing regex pattern

@Bashamega Bashamega requested a review from eddiejaoude July 28, 2024 10:57
Copy link
Member

@eddiejaoude eddiejaoude left a comment

Choose a reason for hiding this comment

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

Looks good, thank you 👍

@eddiejaoude eddiejaoude merged commit 21f59e3 into EddieHubCommunity:main Jul 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE] Don't check the text in url

2 participants