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: allow all correct URLs to be formatted #2328

Merged

Conversation

orevial
Copy link
Contributor

@orevial orevial commented Oct 21, 2024

Description

Quill editor has a rule named AutoFormatMultipleLinksRule that auto format HTTP(s) links, however there was a small caveat where path was mandatory if characters others than A-Z were used.

The problem is that the URL can contain fragments and query params without requiring an extra / as per RFC 3986 on URL structure:

scheme://host[:port][/path][?query][#fragment]

Examples of what the rule was handling before the fix:

Proposed PR allows query params and fragments to be used even without a / character beforehand, thus respecting what the RFC is stating.

Also, I removed the non-prefixed URL from the comments because as opposed to what what was said, it didn't work:

/// It works for the following testing URLs:
// www.google.com 

Copy link
Collaborator

@EchoEllet EchoEllet left a comment

Choose a reason for hiding this comment

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

LGTM.

Does this PR fix #2326 or any related issues in the repo?

@EchoEllet EchoEllet merged commit 0ac3188 into singerdmx:master Oct 22, 2024
2 checks passed
@EchoEllet
Copy link
Collaborator

Thanks for your contributions.

@orevial
Copy link
Contributor Author

orevial commented Oct 25, 2024

LGTM.

Does this PR fix #2326 or any related issues in the repo?

Nope, but to me issue #2326 is not an actual problem we want to solve, see my comment.

CatHood0 pushed a commit to CatHood0/flutter-quill that referenced this pull request Oct 28, 2024
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.

2 participants