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

[BUG] xkcd doesn't embed alt text from json #274

Closed
MrMrRubic opened this issue Aug 2, 2024 · 3 comments · Fixed by #276
Closed

[BUG] xkcd doesn't embed alt text from json #274

MrMrRubic opened this issue Aug 2, 2024 · 3 comments · Fixed by #276
Labels
bug Something isn't working cog-xkcd Issues related to the XKCD cog.

Comments

@MrMrRubic
Copy link

Describe the bug
When the bot retrieves XKCD comics, it's supposed to also embed the alt text from the json if there is any, though this doesn't happen.

To Reproduce
Steps to reproduce the behavior:

  1. make the bot retrieve an XKCD comic with alt text (for example 2966)
  2. Notice the lack of embedded alt text.

Expected behavior
Alt text should be embedded if it exists.

Screenshots
xkcd_screenshot

Additional context
JSON of the relevant comic

@MrMrRubic MrMrRubic added the bug Something isn't working label Aug 2, 2024
@tigattack tigattack added the cog-xkcd Issues related to the XKCD cog. label Aug 2, 2024
@tigattack
Copy link
Member

How would you suggest this be shown? Joined to the titled with a - separator? A third field titled "Description", shown when alt-text is present? In the embed footer? Something else?

@MrMrRubic MrMrRubic changed the title [BUG] xkcd doesn't alt text from json [BUG] xkcd doesn't embed alt text from json Aug 2, 2024
@MrMrRubic
Copy link
Author

MrMrRubic commented Aug 2, 2024

Well, considering the existing code for it in xkcd.py:

# If there is alt text add it to the embed, otherwise don't
        if not data["alt"]:
            xkcd_embed.add_field(name="Comic Alt Text", value=data["alt"])
        else:
            pass

Unless i'm reading it wrong (which is likely considering how little proficient i am with python), it seems backwards? It checks if there isn't data in alt, and if there isn't it makes the field. If there is data, it skips it. So it should be the other way? Skip if nothing there, othwise make the field. So maybe something like:

# If there is alt text add it to the embed, otherwise don't
        if data["alt"]:
            xkcd_embed.add_field(name="Comic Alt Text", value=data["alt"])
        else:
            pass

or

# If there is alt text add it to the embed, otherwise don't
        if not data["alt"]:
            pass
        else:
            xkcd_embed.add_field(name="Comic Alt Text", value=data["alt"])

@tigattack
Copy link
Member

Ha, I didn't even notice there was existing code for it 🤦 You're spot on. That's an even easier fix than I imagined. Thanks for the report!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working cog-xkcd Issues related to the XKCD cog.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants