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

fix: Don't parse tags inside words & URLs #266

Merged
merged 1 commit into from
Nov 22, 2023

Conversation

TheBlob42
Copy link
Contributor

fixes #265

It is an easy solution but it should cover at least all cases I can think of 🤔
Is there a nicer way for "early returns" in F#?

@TheBlob42
Copy link
Contributor Author

Some non-alphanumeric characters are allowed in URLs that could potentially appear before the # character (-, _, %, ., ;, = etc.). I personally don't know about any website that uses such URLs, but maybe someone knows a little more about it.

Here are some examples of made up valid URLs that my fix would not catch:

  • www.demo-#demo.com
  • www.demo_#demo.com
  • www.demo.com?#demo

Copy link
Owner

@artempyanykh artempyanykh left a comment

Choose a reason for hiding this comment

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

Looks reasonable and will likely cover 99% of real-world cases. Thanks for the fix.

P.S. Early return is a bit of a pain in F#. You can emulate it using a computational expression and a guard, but this would be an overkill in this case.

@artempyanykh artempyanykh merged commit b846249 into artempyanykh:main Nov 22, 2023
3 checks passed
@petobens
Copy link

Hi! So something like https://tableau.com/#/site/Sales/views/elastico/Elstico is expected not to be caught? Beyond theses cases (I have a few of those) things work much better now.

@TheBlob42
Copy link
Contributor Author

Yes you are correct this is currently not caught. And with #268 now which allows / inside tags all of #/site/Sales/views/elastico/Elstico would be considered a tag 😅 to completely filter out URLs would require more work in regards to the parser

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.

Filter out hashtags within an url when listing tags
3 participants