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

Improve URL detection in auto_hyperlinks example #250

Merged
merged 9 commits into from
Feb 1, 2024
Merged

Improve URL detection in auto_hyperlinks example #250

merged 9 commits into from
Feb 1, 2024

Conversation

24rr
Copy link
Contributor

@24rr 24rr commented Jan 26, 2024

Fixes #141

@MarshalX
Copy link
Owner

Hi thank you! Could pls return blank empty line? Linter fails

@24rr
Copy link
Contributor Author

24rr commented Jan 26, 2024

Hi thank you! Could pls return blank empty line? Linter fails

Where do I return it?

@MarshalX
Copy link
Owner

Hi thank you! Could pls return blank empty line? Linter fails

Where do I return it?

Please check changes tab or GitHub actions logs. Line number 6. Between imports and function definition

@24rr
Copy link
Contributor Author

24rr commented Jan 26, 2024

Hi thank you! Could pls return blank empty line? Linter fails

Where do I return it?

Please check changes tab or GitHub actions logs. Line number 6. Between imports and function definition

Oh my bad. Done

@MarshalX
Copy link
Owner

@editor-syntax did i something wrong? just testing your regex with example from the issue

https://regex101.com/r/d3Hg78/1

image

@24rr
Copy link
Contributor Author

24rr commented Jan 27, 2024

@editor-syntax did i something wrong? just testing your regex with example from the issue

https://regex101.com/r/d3Hg78/1

image

Nope, but I did something wrong lol. It's updated now.
image

@MarshalX
Copy link
Owner

now it doesnt work for links without (/) :(

image

@24rr
Copy link
Contributor Author

24rr commented Jan 27, 2024

now it doesnt work for links without (/) :(

image

Updated. That's my last resort lol!
image
image
image

@MarshalX
Copy link
Owner

MarshalX commented Jan 27, 2024

@editor-syntax it looks working. But you deleted the usage of "aggressive" bool. Now we have only 1 regex. But you didn't delete the argument of the function. If we are gonna delete "aggressive" mode we should remove all related things to it:

  1. argument in the function
    text: str, *, aggressive: bool, encoding: str = 'UTF-8'
  2. tune docstring of the function
    """If aggressive is False, only links beginning http or https will be detected"""
  3. remove flag from here:
    url_positions = extract_url_byte_positions(text, aggressive=True)
  4. remove this because now we will not have matches for links without protocol
    uri = link[0] if link[0].startswith('http') else f'https://{link[0]}'

@24rr
Copy link
Contributor Author

24rr commented Jan 27, 2024

@editor-syntax it looks working. But you deleted the usage of "aggressive" bool. Now we have only 1 regex. But you didn't delete the argument of the function. If we are gonna delete "aggressive" mode we should remove all related things to it:

  1. argument in the function
    text: str, *, aggressive: bool, encoding: str = 'UTF-8'
  2. tune docstring of the function
    """If aggressive is False, only links beginning http or https will be detected"""
  3. remove flag from here:
    url_positions = extract_url_byte_positions(text, aggressive=True)
  4. remove this because now we will not have matches for links without protocol
    uri = link[0] if link[0].startswith('http') else f'https://{link[0]}'

Yes, we can do that. I'll edit the example code and you edit the rest

@MarshalX MarshalX changed the title Improve URL detection in regex pattern to include parentheses Improve URL detection in auto_hyperlinks example Feb 1, 2024
@MarshalX MarshalX merged commit cfaa697 into MarshalX:main Feb 1, 2024
22 of 23 checks passed
@MarshalX
Copy link
Owner

MarshalX commented Feb 1, 2024

thank you!

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.

Misdetection of URLs / links in the "auto_huperlinks" example
2 participants