support externally hosted images (v2) #705
Conversation
breathe/renderer/sphinxrenderer.py
Outdated
| path_to_image = self.project_info.sphinx_abs_path_to_file(node.name) | ||
| path_to_image = node.name | ||
| if not url_re.match(path_to_image): | ||
| path_to_image = self.project_info.relative_path_to_xml_file(path_to_image) |
There was a problem hiding this comment.
@jdknight It used to call abs path function, but now calls rel path to xml file function. At a glance, this appears to be a mistake, can you confirm?
There was a problem hiding this comment.
I cannot seem to recall why the change (and it's odd that nothing about it was mentioned in the commit; my apologies).
I do not know if this is a mistake though. Looking at the implementation, sphinx_abs_path_to_file is the same as relative_path_to_xml_file, but with an injected path separator at the start (so a srcdir-rooted abspath?). It appears off to me to build a nodes.images with a uri with a leading path seperator -- unknown what that means for Windows instances and how various plugins interpret the root location of uri. I know a Sphinx extension I help maintain, we've observed various plugins generating uri entries with relative paths to the working directory, relative paths on the source/environment directory and absolute paths on the system. By injecting the / on Linux systems, one may (incorrectly) interpret it is as an absolute path to the entire system (which would not work in this case). I am not sure off hand what is the "right" value for a uri entry; I have not seen any specifics in docutils/Sphinx on what is the "right" way to format these uri attributes.
That all being said, I have no problems if we adjust this commit back to using sphinx_abs_path_to_file to just focus on resolving the external hosted URI entries. If there is a desire to use relative_path_to_xml_file, I could go back and re-test against tinyxml and oglft's documentations against various builders and report back.
There was a problem hiding this comment.
@vermeeren, I have updated the pull request to fall back to the original sphinx_abs_path_to_file implementation. This is have this pull request focus on URL-provided images.
Any attempts to switch from sphinx_abs_path_to_file to relative_path_to_xml_file could be done in a separate pull request (which can outline in more details on the specific error cases; if any).
vermeeren
added
code
|
@jdknight - is this for tinyxml 1 or 2? Is it the copy of tinyxml we have in the breathe repo? I can't see any image links in there. It still seems like a reasonable change but I'm not sure what example you're working with and I'd like to be able to test it myself. |
|
@michaeljones, this is using tinyxml2. I believe for this project, the images are found without a An example of this output can be found here. |
When building an image node, check to see if the URI defines a URL schema. If so, assume the source is an external image and just passthrough the value to the generate image node's uri value. Otherwise, default to building a Sphinx project's absolute path to the image. Signed-off-by: James Knight <james.d.knight@live.com>
Observed an issue when attempting to process tinyxml documentation. After generating XML documentation from tinyxml, processing it through breathe would result in the following warning:
The project prefix
xml/tinyxml/had been injected into the URI of the image. This commit tries to help mitigate using an absolute path building for external images by checking if the URI defines a URL schema (using implementation from Sphinx's utility class). If a URL schema is detected, assume the source is an external image and just passthrough the value to the generate image node's uri value. Otherwise, default to building a Sphinx project's absolute path to the image.url_recall should be available in oldest supported Sphinx revision listed (v2.3.1).