Skip to content

new: Type hints for python module#2

Merged
qjerome merged 9 commits intomainfrom
typing
Oct 6, 2025
Merged

new: Type hints for python module#2
qjerome merged 9 commits intomainfrom
typing

Conversation

@Rafiot
Copy link
Contributor

@Rafiot Rafiot commented Oct 2, 2025

This is very much not done.

Missing bits:

 from pyfaup import FaupCompat as Faup

faup = Faup()
faup.decode("https://user:pass@sub.example.com:8080/path?query=value#fragment")
faup.url # <= returns the bytes encoded url: b"https://user:pass@sub.example.com:8080/path?query=value#fragment"

faup.get_domain_without_tld() # <= returns "sub.example", but the string to remove must only be at the end of the hostname, so we cannot just replace ".com" anywhere in the string

Copy link
Collaborator

@qjerome qjerome left a comment

Choose a reason for hiding this comment

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

Please take the suggestions into account

@Rafiot Rafiot marked this pull request as ready for review October 3, 2025 11:55
Copy link
Collaborator

@qjerome qjerome left a comment

Choose a reason for hiding this comment

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

Please fix :)

@Rafiot Rafiot force-pushed the typing branch 2 times, most recently from c6b59df to a6cb363 Compare October 3, 2025 15:58
Copy link
Collaborator

@qjerome qjerome left a comment

Choose a reason for hiding this comment

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

Could you please fix the latest change ?

Also, don't you want to use/expose the orig field you just added to the Url structure ?

@Rafiot
Copy link
Contributor Author

Rafiot commented Oct 6, 2025

orig is exposed, isn't it? I added it in the tests.

@Rafiot Rafiot force-pushed the typing branch 9 times, most recently from 0a6839d to 69a8671 Compare October 6, 2025 14:08
@qjerome qjerome merged commit f58ca1b into main Oct 6, 2025
17 checks passed
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