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

Fix image name collision in docs #4783

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ffreyer
Copy link
Collaborator

@ffreyer ffreyer commented Feb 12, 2025

Description

Fixes #4388

Type of change

  • Bug fix (non-breaking change which fixes an issue)

@ffreyer ffreyer added the skip-changelog Skips changelog enforcer label Feb 12, 2025
@ffreyer ffreyer requested a review from jkrumbiegel February 12, 2025 18:25
@ffreyer ffreyer marked this pull request as ready for review February 12, 2025 18:25
@jkrumbiegel
Copy link
Member

I think the fix should rather be to hash the code after the transformation step a line below?

I actually don't remember the reason for this design, but I wonder if one couldn't just use the page plus a running index to disambiguate. Using the transformed code there would still be collision problems when two examples have the same code, like a simple fig where some state is modified in between to show an effect on the visual output.

@ffreyer
Copy link
Collaborator Author

ffreyer commented Feb 13, 2025

The transformation step also adds save($(id)...) so the id needs to exist beforehand

@jkrumbiegel
Copy link
Member

Ah ok missed that on my phone, can you try the running index version instead, where the hash is page, number_of_figure_block? Not sure how to get the number, probably needs some kind of state sadly. But we already have state, maybe just reuse that. Could also just do number_of_figure_globally as we have a dict of all of them I guess?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-changelog Skips changelog enforcer
Projects
Status: Ready to review
Development

Successfully merging this pull request may close these issues.

Apparent error in docs on "transparency"
2 participants