-
Notifications
You must be signed in to change notification settings - Fork 2k
Add md5 hash query param to images to bust the cache when we update one inplace #3746
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
Conversation
…pdate one inplace
To turn off screen sharing, click the Connect system tray icon and unselect **Allow screen sharing**. Your Raspberry Pi remains signed into Connect, but you won't be able to create a screen sharing session from the Connect dashboard. | ||
|
||
image:images/screen-sharing-disabled-desktop.png[width="80%"] | ||
image::images/screen-sharing-disabled-desktop.png[width="80%"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These work fine as inline images, but I messed up the source asciidoc here when I wrote the page. Turned out to be some nice 'test cases' because they were the only images not detected by my regex!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or you could fix your regex so that it works with both one or two colons? ::?
Works great locally; merging so I can test it with #3747 |
line += edit_text + "\n\n" | ||
else: | ||
# find all image references, append md5 hash at end to bust the cache if we change the image | ||
m = re.match(r'^(image::)(.+)(\[(.+)]\n?)$', line) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nathan-contino Do we need to use non-greedy matching here? E.g. .+?
instead of .+
? Since it's just one line, it's pretty low risk, but I always get nervous about this stuff. But also not sure if that would change the functionality at all...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it's valid syntax to put things on a line after an image other than the image itself, and most images should have the square brackets. So we can probably get away without worrying. I couldn't find a less-greedy approach that worked with all images, since names vary a lot!
line += edit_text + "\n\n" | ||
else: | ||
# find all image references, append md5 hash at end to bust the cache if we change the image | ||
m = re.match(r'^(image::)(.+)(\[(.+)]\n?)$', line) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the square-brackets-part at the end need to be made an optional match?
EDIT: Also, looks like you probably don't need the capture-group (round brackets) inside the literal square brackets?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually plan to add alt text to every one of the images in our docs, so... technically no. But it might be safer to do so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point that we can likely improve this regex. However, I've tested this one (to verify that it works for all images in our repo) and need it for another PR, so I'm just going to roll ahead with this version right now. I will circle back and try to improve it at a later date!
Works like the title says.