-
-
Notifications
You must be signed in to change notification settings - Fork 55
URLs in nav items in subrepos are incorrectly rewritten #123
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
base: main
Are you sure you want to change the base?
Conversation
Version Number CheckUpdate the version in pyproject.toml if you want this PR to generate a new release (main is 0.6.3 >= 0.6.3). In addition, please update the CHANGELOG.md. |
| for index, entry in enumerate(nav): | ||
| if isinstance(entry, str): | ||
| nav[index] = str(section_name / Path(entry)).replace("\\", "/") | ||
| if entry.startswith("http"): |
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.
And ftp or other schemas? What about using urlparse?
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.
also, you can make this one line.
nav[index] = str(section_name / Path(entry)).replace("\\", "/") if not urlparse.some_method(entry) else entry| if type(value) is list: | ||
| resolve_nav_paths(value, section_name) | ||
| else: | ||
| elif type(value) is str and value.startswith("http"): |
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.
Dito.
jdoiro3
left a comment
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.
Thanks for contributing. You can also ignore the PR comment. I'm going to merge this once you use urlparse as @Spacetown suggested.
| for index, entry in enumerate(nav): | ||
| if isinstance(entry, str): | ||
| nav[index] = str(section_name / Path(entry)).replace("\\", "/") | ||
| if entry.startswith("http"): |
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.
also, you can make this one line.
nav[index] = str(section_name / Path(entry)).replace("\\", "/") if not urlparse.some_method(entry) else entry|
Hey, I ran into this issue as well. What would be needed to get this fixed. I am happy to provide a PR, with the changes above. |
If a subrepo has a URL in a nav item:
support.py:resolve_nav_paths()rewrites the link making it unusable.