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

More customization options for figure_language_filename for localized images #7768

Closed
akien-mga opened this issue Jun 2, 2020 · 6 comments
Closed

Comments

@akien-mga
Copy link

akien-mga commented Jun 2, 2020

Is your feature request related to a problem? Please describe.
I'm working on the localization of the Godot Engine documentation, and we use a separate repository to hold the massive PO files that we use for Weblate and Sphinx.

I'm now working on image localization (upstream issue) and I find that even though figure_language_filename seems to provide some customization option, it's impossible to host the localized images in the separate repository.

Indeed, both the {root} and {path} substitution tokens resolve to absolute paths from the host system (e.g. root: /home/akien/Projects/godot/godot-docs-l10n/docs/tutorials/shading/img/vs_popup), and since I can't do post-processing of the subsituted string, I can't strip the /home/akien/Projects/godot/godot-docs-l10n/docs/ base path to attempt using something like ../img/{language}/{rel_path}/{filename}{ext}.

Describe the solution you'd like
I'd like the addition of one or more new path substitution tokens that can be used to customize the path to localized images more freely.

For example, for this structure:

foo/bar/index.rst
foo/bar/img/image.png

and index.rst referencing .. image:: img/image.png, I could imagine two useful substitution tokens:

relative_path = "img/"
resolved_path = "foo/bar/img/"

Alternatively (and possible as a better solution), {root} and {path} could be changed to actually be relative to the Sphinx project's root folder, i.e. the {resolved_path} in my example above, i.e.:

root = "foo/bar/img/image"
path = "foo/bar/img"

(While the documentation currently states that these would be img/image and img/, which is wrong in my tests on Linux with Sphinx 1.8.5).

I don't specifically need access to the file-relative path img/ in my use case, but I imagine that some projects may benefit from it.

Describe alternatives you've considered
I don't see any alternative option that would enable the workflow I want to use with localized images not being either in the original image's folder, or in one of its subfolders.

Edit: We came up with two three hacks that we could use to workaround the currently limited substitution tokens offered by figure_language_filename:

I did not test with latest Sphinx as it's not properly supported on ReadTheDocs yet, but the relevant code seems to be unchanged from 1.8.5:
https://github.com/sphinx-doc/sphinx/blob/3.x/sphinx/util/i18n.py#L299-L315

Additional context

@akien-mga akien-mga added the type:enhancement enhance or introduce a new feature label Jun 2, 2020
@akien-mga
Copy link
Author

BTW the fact that {root} and {path} are absolute might be a bug, given that the code reads like translated would be supposed to be relative to the doc file:
https://github.com/sphinx-doc/sphinx/blob/3.x/sphinx/util/i18n.py#L318-L327

Yet on my system with Sphinx 1.8.5 the variables would be:

srcdir: /home/akien/Projects/godot/godot-docs-l10n/docs
dirname: getting_started/step_by_step
translated: /home/akien/Projects/godot/godot-docs-l10n/docs/getting_started/step_by_step/img/shooter_instancing.fr.png

path.join seems to be happy to automatically resolve the concatenation of the two absolute paths, which is why it works fine as is.

If this were changed so that the output of get_image_filename_for_language(filename, env) is made relative to the doc name, I would still need an extra token for my use case as I'd need to have the dirname part too from the above code link (path.dirname(env.docname)).

@tk0miya tk0miya added type:bug internals:internationalisation and removed type:enhancement enhance or introduce a new feature labels Jun 2, 2020
@tk0miya tk0miya added this to the 3.1.0 milestone Jun 2, 2020
@tk0miya
Copy link
Member

tk0miya commented Jun 2, 2020

Thank you for reporting. I agree this is a bug. And it seems the configuration was designed as {root} is a relative path from the current document (Our document says dirname/filename is a root token for dirname/filename.png!).

{root} - the filename, including any path component, without the file extension, e.g. dirname/filename

I'll try to fix this not to cause breaking changes later.

@akien-mga
Copy link
Author

Sounds good. Making {root} and {path} relative to the current document and thus matching their documentation definitely makes sense.

For the other, feature proposal part of this issue, I'd suggest to then consider adding a new token that would be basically path.join(path.dirname(env.docname), {path} (with {path} being the actual Python variable used to format it). As in my use case having only the path relative to the current document wouldn't be sufficient, since figure_language_filename doesn't provide any reference to what the current document is.

@tk0miya
Copy link
Member

tk0miya commented Jul 26, 2020

As a first step, I just posted #8006 to fix that an absolute path is passed to figure_language_filename as a {root}.

Now I'm considering about next step, adding a new key for figure_language_filename.
@akien-mga I'd like to confirm just in case, what you want is a dirname of the document, right? If so, I will add a new key like docpath (I'm still looking for more better name for this).

@akien-mga
Copy link
Author

Thanks for the bugfix!

Yeah, I think what I'd need would be a dirname of the document, and possibly always resolved to be relative to the root folder of the project, e.g. with:

community/contributing/img/l10n_01_language_list.png

Referenced in:

community/contributing/editor_and_docs_localization.rst

as:

.. image:: img/l10n_01_language_list.png

I'd need to have access somehow to:

community/contributing/img

So that the figure_language_filename can be configured as:

<arbitrary path>/{docpath}/{filename}.{language}{extension}

In my concrete example, this would resolve to (.. is a separated l10n repo where the main Sphinx project is included as submodule, see https://github.com/godotengine/godot-docs-l10n):

../images/community/contributing/img/l10n_01_language_list.fr.png

for example (French locale).

Currently I'm monkey-patching Sphinx this way and it works fine for the use case, though of course I'm looking forward to using an upstream solution :)
https://github.com/godotengine/godot-docs/blob/04f7c48b90d5a3573486e631ddf665b61d971ac1/conf.py#L204-L231

tk0miya added a commit that referenced this issue Jul 30, 2020
…ilename

Fix #7768: i18n: Wrong root element is passed to figure_language_filename
@tk0miya tk0miya reopened this Jul 30, 2020
@tk0miya
Copy link
Member

tk0miya commented Jul 30, 2020

I just merged #8006 now. And I'll add docpath later.

tk0miya added a commit to tk0miya/sphinx that referenced this issue Aug 1, 2020
…th} token

To build structured i18n imaging directory, figure_language_filename
now supports `{docpath}` token that is a dirname of the current
document.
@tk0miya tk0miya closed this as completed in af15593 Aug 2, 2020
tk0miya added a commit that referenced this issue Aug 2, 2020
…e_filename

Close #7768: i18n: figure_language_filename supports {docpath} token
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants