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

wikipedia: support media fragments #2388

Merged

Conversation

SnoopJ
Copy link
Contributor

@SnoopJ SnoopJ commented Dec 29, 2022

Description

This PR adds support for Wikipedia links to media embedded in articles (i.e. fragments of the form #/media/File:…), which closes #2316

Edit: While writing up the follow-up issue, I realized that File: is specific to English Wikipedia, so in 63187e9 the prefix check was relaxed to support other references to files on other language communities on Wikipedia. It seems to Just Work™ and the description retrieval is fairly cautious anyway.

Note that this changeset does not emit an image description for links directly to a File:… article, but it's probably possible to rework it to do that too, if desired

Checklist

  • I have read CONTRIBUTING.md
  • I can and do license this contribution under the EFLv2
  • No issues are reported by make qa (runs make quality and make test)
  • I have tested the functionality of the things this change touches

dgw referenced this pull request Dec 29, 2022
Both I and the reviewer missed a bug in #2383 that passed arguments to
`str.join()` wrong. Switched to `str.format()` at another maintainer's
suggestion.

Co-authored-by: SnoopJ <snoopjedi@gmail.com>
@dgw dgw added this to the 8.0.0 milestone Dec 29, 2022
@dgw dgw added Feature Bugfix Generally, PRs that reference (and fix) one or more issue(s) labels Dec 29, 2022
@SnoopJ SnoopJ force-pushed the bugfix/gh2316_support-wikimedia-commons-images branch from 927e951 to 08ccaa5 Compare December 29, 2022 20:24
@dgw
Copy link
Member

dgw commented Dec 29, 2022

About handling File links directly: We probably don't want to open that can of worms this late in 8.0. But it can be a feature request for 8.1 if you see people post those. Maybe we should ask @xnaas if it's worthwhile, since SackBot gets a lot of traffic.

The PR itself, though, I mentioned on IRC that the commit messages don't follow project standards. Beyond that, not all files/images on Wikipedia come from Commons. You said it's intended that e.g. https://en.wikipedia.org/wiki/Google#/media/File:Google1998.png will not output anything, but that seems confusing for users if some (but not all) image links output a description.

@SnoopJ SnoopJ force-pushed the bugfix/gh2316_support-wikimedia-commons-images branch from bba75ea to d78961d Compare December 29, 2022 20:38
Copy link
Member

@dgw dgw left a comment

Choose a reason for hiding this comment

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

I haven't gone hard enough at this to formally "Request changes", but I did have a couple comments.

sopel/modules/wikipedia.py Outdated Show resolved Hide resolved
sopel/modules/wikipedia.py Outdated Show resolved Hide resolved
sopel/modules/wikipedia.py Show resolved Hide resolved
@ghost
Copy link

ghost commented Dec 29, 2022

༼ つ ◕_◕ ༽つ 8.0

Edit: Real answer: can save the "hardcore" stuff for when the plugin is extracted. :P

@dgw
Copy link
Member

dgw commented Dec 29, 2022

༼ つ ◕_◕ ༽つ 8.0

pip install git+https://github.com/sopel-irc/sopel.git

Copy link
Member

@dgw dgw left a comment

Choose a reason for hiding this comment

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

Nit, but you can squash that linter fixup anyway. 😉

sopel/modules/wikipedia.py Show resolved Hide resolved
sopel/modules/wikipedia.py Show resolved Hide resolved
Co-authored-by: dgw <dgw@technobabbl.es>
@SnoopJ SnoopJ force-pushed the bugfix/gh2316_support-wikimedia-commons-images branch from 8b06bca to 0728d35 Compare January 4, 2023 14:30
@dgw dgw merged commit 94065d7 into sopel-irc:master Jan 11, 2023
@SnoopJ SnoopJ deleted the bugfix/gh2316_support-wikimedia-commons-images branch February 19, 2023 03:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bugfix Generally, PRs that reference (and fix) one or more issue(s) Feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

wikipedia: not all fragments are sections
3 participants