Page MenuHome
Differential D6995

Addons: Allow a manual url prefix
ClosedPublic
Actions

Authored by Aaron Carlisle (Blendify) on Mar 2 2020, 8:41 PM.

Details

Reviewers
Campbell Barton (campbellbarton)
Sybren A. Stüvel (sybren)
Maniphest Tasks
T74017: Documentation links broken in 2.82 after recent documentation url changes for IO addons
Commits
rBa0ea0153c262: Addons: Allow a user manual url prefix in doc_url
Summary

This was raised in T74017, the issue being that we point to /dev version of the manual for the addons when we want to point to a specific version instead.

Instead of manually updating the URL every release we can do this.

The bl_info for addons will need to be updated in the format of 'wiki_url': "$BLENDER_MANUAL_URL/addons/import_export/scene_gltf2.html",

Diff Detail

Repository
rB Blender

Event Timeline

Aaron Carlisle (Blendify) created this revision.Mar 2 2020, 8:41 PM
Aaron Carlisle (Blendify) edited the summary of this revision. (Show Details)
Sybren A. Stüvel (sybren) added inline comments.Mar 3 2020, 5:32 PM
release/scripts/modules/addon_utils.py
531–538

Constructing the manual URL only has to be done once. Blender won't change versions while it's running. This means that you could do it in a different function and decorate it with @functools.lru_cache(1).

538

I would use string formatting here, like this:

url = f"https://docs.blender.org/manual/en/{manual_version}"
540

There is no need to use string.Template just to replace a string. You can just do this instead:

addon_info["wiki_url"] = addon_info["wiki_url"].replace('$BLENDER_MANUAL_URL', url)
Sybren A. Stüvel (sybren) requested changes to this revision.Mar 3 2020, 5:32 PM
This revision now requires changes to proceed.Mar 3 2020, 5:32 PM
Brecht Van Lommel (brecht) added a subscriber: Brecht Van Lommel (brecht).Mar 3 2020, 6:37 PM
Brecht Van Lommel (brecht) added inline comments.
release/scripts/modules/addon_utils.py
531–538

Most developers won't know what @functools.lru_cache(1) is, none of this is performance critical so I prefer the code to be easy to understand.

Aaron Carlisle (Blendify) added a task: T74017: Documentation links broken in 2.82 after recent documentation url changes for IO addons.Mar 3 2020, 6:59 PM
Aaron Carlisle (Blendify) updated this revision to Diff 22348.Mar 3 2020, 11:20 PM
  • Separate manual version into a function
  • Do not use string template
  • Use string formatting for URL

I decided to not use the decorate to stick to the programming style as Brecht suggested.

Aaron Carlisle (Blendify) updated this revision to Diff 22406.Mar 5 2020, 3:53 AM

Rebase with master

Campbell Barton (campbellbarton) added a comment.Mar 5 2020, 4:15 AM
  • Use _ prefix for manual_version, since this isn't part of the public API.
  • Use {..} instead of $.. to allow {.._SUFFIX} later on.

We could set manual_version once, however this doesn't run on startup so I'm not too worried about this.

P1283

Aaron Carlisle (Blendify) updated this revision to Diff 22407.Mar 5 2020, 4:33 AM

Update based on feedback

Campbell Barton (campbellbarton) accepted this revision.Mar 5 2020, 5:14 AM
This revision was not accepted when it landed; it landed in state Needs Review.Mar 5 2020, 5:24 AM
Closed by commit rBa0ea0153c262: Addons: Allow a user manual url prefix in doc_url (authored by Aaron Carlisle (Blendify)). · Explain Why
This revision was automatically updated to reflect the committed changes.
Aaron Carlisle (Blendify) added a commit: rBa0ea0153c262: Addons: Allow a user manual url prefix in doc_url.
Sybren A. Stüvel (sybren) added a comment.Mar 5 2020, 10:05 AM
In D6995#167335, @Campbell Barton (campbellbarton) wrote:
  • Use {..} instead of $.. to allow {.._SUFFIX} later on.

This was originally proposed in T74017, but there (T74017#876887) I also said:

I like the idea, but this is very close to Python's f"" notation. Maybe "$BLENDER_MANUAL_URL/addons/import_export/scene_gltf2.html"? Or ${BLENDER_MANUAL_URL} if you want it to be more 'enclosed'.

Now the interpretation (by humans) of that string hinges on spotting the lack of f in front of the string. Since f-strings aren't that common yet, I think it's a good idea to use something more visibiliy different here. ${…_SUFFIX} also works fine.

Please don't commit too hastily, especially when a reviewer hasn't accepted a patch yet and when the new changes go against what that reviewer suggested earlier...

Sybren A. Stüvel (sybren) mentioned this in rBae223ff52bed: Addons: deprecate 'wiki_url'.Mar 5 2020, 10:11 AM

Revision Contents
Changeset List

PathSize
release/
scripts/
modules/
18 lines

Diff 22408

View Options

release/scripts/modules/addon_utils.py

Loading...