Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Add extract_urls-helper#854

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
JosXa wants to merge40 commits intomasterfromextract-urls
Closed

Add extract_urls-helper#854

JosXa wants to merge40 commits intomasterfromextract-urls

Conversation

JosXa
Copy link
Contributor

Added a new helper that extracts all URLs contained in a message and returned from Telegram

Lulzx reacted with thumbs up emoji
@JosXaJosXa requested a review fromjsmnbomOctober 2, 2017 01:49
@91DarioDev
Copy link

What about to include also captions in the helper using regex?

@JosXa
Copy link
ContributorAuthor

@91DarioDev Good point.

@JosXa
Copy link
ContributorAuthor

JosXa commentedOct 2, 2017
edited
Loading

@91DarioDev

  • Extracting URLs from Captions

@python-telegram-botpython-telegram-bot deleted a comment fromcodecovbotOct 2, 2017
@Eldinnie
Copy link
Member

Fails on all except py3.6 due tourllib.parse

@JosXaJosXa self-assigned thisOct 7, 2017
@Eldinnie
Copy link
Member

@JosXa Can you please make sure the PR passes pre-commit and tests?

@python-telegram-botpython-telegram-bot deleted a comment fromcodecovbotFeb 1, 2018
@JosXa
Copy link
ContributorAuthor

test_helpers succeeds in all test cases. However, unrelated tests fails. So lgtm, but@Reviewer please have another look at the tests.

@Eldinnie
Copy link
Member

@JosXa Can you remind me why we would need/want this? And anyway I think it's more logical to sort in order of appearance rather then alphabetical? Also to parse them from captions please use the parse_caption_entites

@Bibo-Joshi
Copy link
Member

Bibo-Joshi commentedOct 17, 2019
edited
Loading

Turns out the issue was hidden in the call ofmessage.parse_entities. This builds a dict which isnot sorted in python <=3.5. So now after parsing, the entities are just sorted.
One thing I want to mention: The method now can only handlesingle trailing slashes, i.e.[github.com/, google.com, google.com/ github.com//] will produce[github.com/, google.com/ github.com//]. However I guess trailing double slashes are very rare (if even existent?), so that should be ok IMO …

PS: AppVeyor fail is unrelated ;)

@jh0kerTelegram GithubBot Revised
Copy link
Member

@Bibo-Joshi I'm not quite sure why you would want to filter this at all. The difference between having a trailing slash or not in a URL is not necessarily insignificant.

Unrelated, the comment in line 185 says "dublicates" instead of "duplicates".

@Bibo-JoshiTelegram GithubBot Revised
Copy link
Member

@jh0ker I guess that 9-9-19 me interpreted@nmlorg s comment such that in doubt it's better to have a trailing slash than not. But neither 9-9-19 me nor now-me have much expertise on that area (or any area at all 😅) …

@Bibo-JoshiBibo-Joshi added 📋 pending-reviewwork status: pending-review and removed 📋 pending-replywork status: pending-reply labelsOct 20, 2019
@PoolitzerPoolitzer added this to the12.3 milestoneNov 11, 2019
@tsnoamtsnoam modified the milestones:12.3,12.4Nov 15, 2019
@JosXa
Copy link
ContributorAuthor

JosXa commentedDec 16, 2019
edited
Loading

@SpEcHiDe brought up an interesting point. He says that he's using something like this in his bot:

image

Maybe we could also provide some abstractions over join links, message references, and t.me URLs 🤔

@JosXaJosXa mentioned this pull requestJan 26, 2020
@Bibo-Joshi
Copy link
Member

Test fail is unrelated

@tsnoam
Copy link
Member

After long considerations and real intent to see this PR merged, we, the maintainers, decided that this PR is too specific to be included in a general purpose library.
Therefor we've decided to close the PR without merging.

@tsnoamtsnoam closed thisFeb 2, 2020
@tsnoamtsnoam deleted the extract-urls branchFebruary 2, 2020 21:34
@github-actionsgithub-actionsbot locked and limited conversation to collaboratorsAug 19, 2020
@Bibo-Joshi
Copy link
Member

For future reference: this will be made available through the newptbcontrib library

@Bibo-JoshiBibo-Joshi added 🔌 enhancementpr description: enhancement and removed enhancement labelsNov 3, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@EldinnieEldinnieEldinnie requested changes

@nmlorgnmlorgnmlorg left review comments

@tsnoamtsnoamtsnoam requested changes

@jsmnbomjsmnbomAwaiting requested review from jsmnbom

Assignees

@JosXaJosXa

@Bibo-JoshiBibo-Joshi

Labels
🔌 enhancementpr description: enhancement📋 pending-reviewwork status: pending-review
Projects
None yet
Milestone
12.4
Development

Successfully merging this pull request may close these issues.

8 participants
@JosXa@91DarioDev@Eldinnie@tsnoam@Bibo-Joshi@jh0ker@nmlorg@Poolitzer

[8]ページ先頭

©2009-2025 Movatter.jp