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

Resolve relative images when caching plugin data #796

Closed
wants to merge 10 commits into from

Conversation

codemonkey800
Copy link
Collaborator

@codemonkey800 codemonkey800 commented Dec 17, 2022

Description

Closes #541

This PR adds relative image resolution for plugin markdown descriptions. This works by following the image resolution rules outlined here: #140 (comment)

This also handles the edge case regarding repo default branch by using the default_branch property from the GitHub repo API.

The only edge case that isn't handled is versioning, but since we currently do not support viewing versioned plugins on the hub, we can defer handling versioned images for later

Demo

Relative images starting with ./

https://www.napari-hub.org/plugins/napari-dzi-zarr
https://dev-rel-img-frontend.dev.imaging.cziscience.com/plugins/napari-dzi-zarr

Relative images starting with ../

https://www.napari-hub.org/plugins/napari-bleach-correct
https://dev-rel-img-frontend.dev.imaging.cziscience.com/plugins/napari-bleach-correct

@codemonkey800 codemonkey800 added the improvement Release Label: Used for categorizing improvements in automated CI release notes label Dec 17, 2022
@codemonkey800 codemonkey800 marked this pull request as ready for review December 19, 2022 18:13
@richaagarwal richaagarwal removed their request for review January 9, 2023 23:03
klai95
klai95 previously approved these changes Jan 9, 2023
Copy link
Contributor

@klai95 klai95 left a comment

Choose a reason for hiding this comment

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

Looks good to me!

url = get_raw_github_url(
self.repo,
self.branch,
src.replace(pattern, ''),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would replace also work for ../ as it would have to point to the parent folder?

Copy link
Collaborator

Choose a reason for hiding this comment

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

It turns out we can leverage the normpath from the os.path module for handling the above issue. I was thinking through this, and here is an untested snippet that may work for our case.

If we parsed the URL into its components, we could use the base_path to update our path.

from urllib.parse import urlparse, urlunparse
url = f'https://raw.githubusercontent.com/{repo}/{branch}/'
scheme, netloc, base_path, *_ = urlparse(url)

This way, we could retain the other components (if any) from the relative path.

if src.startswith(pattern):
    *_, path, params, query, fragment = urlparse(src)
    complete_path = os.path.normpath(os.path.join(base_path, path))
    absolute_path = urlunparse(scheme=scheme, 
                               netloc=netloc, 
                               path=complete_path, 
                               params=params, 
                               query=query, 
                               fragment=fragment)
    img.set('src', absolute_path)

self.branch = branch

def run(self, root):
for img in root.iter('img'):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we also consider adding this for the anchor tags?

@codemonkey800 codemonkey800 force-pushed the jeremy/markdown-relative-images branch 2 times, most recently from 0b23fd3 to e31c5d6 Compare February 9, 2023 03:03
@codemonkey800
Copy link
Collaborator Author

closing this now since implementation was originally in S3, we would need to move this to the data workflows step if we ever decide to pick this up again

@codemonkey800 codemonkey800 deleted the jeremy/markdown-relative-images branch October 7, 2023 03:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Release Label: Used for categorizing improvements in automated CI release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add support for relative links to images
3 participants