I18n: Extract and disambiguate a few messages #122969

Merged
Bastien Montagne merged 9 commits from pioverfour/blender:dp_disambiguate into blender-v4.2-release 2024-06-13 12:16:09 +02:00
Member

Extract

  • Cycles denoiser enum.
  • Extensions user preferences UI.
  • Node operator poll message from new node function.

Improve

  • Split "(Enabled|Disabled) on startup, overriding the preference."
    into two messages.

Disambiguate

  • "Add" when describing the action of adding something should use the
    Operator context.
  • "Dimensions", in noise textures.
  • "Transform" as a noun, the matrix transform type of Geometry Nodes,
    as opposed to the verb to move things in space.
  • "Parent" as a noun or verb (the parent of an object, to parent an
    object to another).

Some issues reported by Satoshi Yamasaki, deathblood, and Gabriel Gazzán.

Extract - Cycles denoiser enum. - Extensions user preferences UI. - Node operator poll message from new node function. Improve - Split "(Enabled|Disabled) on startup, overriding the preference." into two messages. Disambiguate - "Add" when describing the action of adding something should use the Operator context. - "Dimensions", in noise textures. - "Transform" as a noun, the matrix transform type of Geometry Nodes, as opposed to the verb to move things in space. - "Parent" as a noun or verb (the parent of an object, to parent an object to another). Some issues reported by Satoshi Yamasaki, deathblood, and Gabriel Gazzán.
Damien Picard added the
Interest
Translations
Module
User Interface
labels 2024-06-09 20:45:14 +02:00
Damien Picard added 1 commit 2024-06-09 20:45:19 +02:00
Extract
- Cycles denoiser enum.
- Extensions user preferences UI.
- Node operator poll message from new node function.

Improve
- Split "(Enabled|Disabled) on startup, overriding the preference."
into two messages.

Disambiguate
- "Add" when describing the action of adding something should use the
  Operator context.
- "Dimensions", in noise textures.
- "Transform" as a noun, the matrix transform type of Geometry Nodes,
  as opposed to the verb to move things in space.

Some issues reported by Satoshi Yamasaki and deathblood.
Damien Picard requested review from Bastien Montagne 2024-06-09 20:45:34 +02:00
Bastien Montagne requested changes 2024-06-10 09:58:34 +02:00
Dismissed
@ -264,3 +265,3 @@
if _cycles.with_openimagedenoise:
return [('OPENIMAGEDENOISE', "OpenImageDenoise",
"Use Intel OpenImageDenoise AI denoiser", 4)]
tip_("Use Intel OpenImageDenoise AI denoiser"), 4)]

This needs to be solved in another way. In fact, even existing code is not correct (see warning in https://docs.blender.org/api/master/bpy.props.html#bpy.props.EnumProperty ).

These strings are not extracted currently because my workstation/Blender build (which I use to generate the messages) do not have these advanced rendering device options.

But you do not want to get the 'source' strings (in the Enum data) translated, this should only happen when requested (usually for UI drawing).

Think the proper fix here would be to have a 'translation marker', like in C++ code, and use it to tag all of these strings (note that the label ones should also be marked for translation extraction actually). Will try to get this in today.

This needs to be solved in another way. In fact, even existing code is not correct (see warning in https://docs.blender.org/api/master/bpy.props.html#bpy.props.EnumProperty ). These strings are not extracted currently because my workstation/Blender build (which I use to generate the messages) do not have these advanced rendering device options. But you do not want to get the 'source' strings (in the Enum data) translated, this should only happen when requested (usually for UI drawing). Think the proper fix here would be to have a 'translation marker', like in C++ code, and use it to tag all of these strings (note that the label ones should also be marked for translation extraction actually). Will try to get this in today.

Actually, you already did that in !122971, so just need to land this one first and then update that PR!

Actually, you already did that in !122971, so just need to land this one first and then update that PR!
pioverfour marked this conversation as resolved
@ -186,3 +186,3 @@
if module_parent_dirname(mod.__file__) == "addons_core":
sub.label(text="Core: " + bl_info["name"], translate=False)
sub.label(text=iface_("Core:") + " " + iface_(bl_info["name"]), translate=False)

I don't think the name should be translated?

I don't think the name should be translated?
Author
Member

That’s what we’ve been doing previously. In many cases the add-on name is like a product name not to translate, but in others it’s a straightforward description that should be translated IMO.

That’s what we’ve been doing previously. In many cases the add-on name is like a product name not to translate, but in others it’s a straightforward description that should be translated IMO.
mont29 marked this conversation as resolved
@ -208,2 +208,3 @@
if bl_info["description"]:
col_a.label(text="{:s}.".format(bl_info["description"]))
col_a.label(
text="{:s}.".format(iface_(bl_info["description"])),

This is closer to a tooltip than a label, so would rather use tip_() here?

This is closer to a tooltip than a label, so would rather use `tip_()` here?
Author
Member

Well we’ve been making tip_() apply only to tooltips lately. But also I think it looks a bit weird when everything around is translated:
image

Well we’ve been making `tip_()` apply only to tooltips lately. But also I think it looks a bit weird when everything around is translated: ![image](/attachments/413037db-122a-4960-93bf-11483cf2a685)

It's even weirder for people who disable 'label' translation to keep basic UI in English, but then enable tooltip translation to get detailed translated info. Then they get a full English long & complex description...

And I'd expect cases where 'label' translation is enbaled, but 'tooltips' one is disabled, to be anecdotal at best?

It's even weirder for people who disable 'label' translation to keep basic UI in English, but then enable tooltip translation to get detailed translated info. Then they get a full English long & complex description... And I'd expect cases where 'label' translation is enbaled, but 'tooltips' one is disabled, to be anecdotal at best?
pioverfour marked this conversation as resolved
@ -797,2 +800,3 @@
# The full tagline may be multiple lines (not yet supported by Blender's UI).
col_a.label(text="{:s}.".format(item_remote["tagline"]), translate=False)
col_a.label(
text="{:s}.".format(iface_(item_remote["tagline"])),

This is closer to a tooltip than a lable, so would rather use tip_() here?

This is closer to a tooltip than a lable, so would rather use `tip_()` here?
pioverfour marked this conversation as resolved
@ -2651,3 +2651,3 @@
# Weak, add/remove as properties.
op_add: BoolProperty(name="Add")
op_add: BoolProperty(name="Add", translation_context=i18n_contexts.operator_default)
op_remove: BoolProperty(name="Remove")

This one should also have the Operator context

This one should also have the `Operator` context
Author
Member

Do you mean "Remove"? I can do that but then 8 other messages may need it as well.
"Add" is problematic because it can be either the math operator / blending mode or the action, but Remove doesn’t have this issue AFAIK.

Do you mean "Remove"? I can do that but then 8 other messages may need it as well. "Add" is problematic because it can be either the math operator / blending mode or the action, but Remove doesn’t have this issue AFAIK.

I'd rather be consistent, and tag 'action' messages, especially very short ones, as such?

I'd rather be consistent, and tag 'action' messages, especially very short ones, as such?
pioverfour marked this conversation as resolved
@ -5039,6 +5039,7 @@ static void def_sh_tex_noise(StructRNA *srna)
RNA_def_property_enum_sdna(prop, nullptr, "dimensions");
RNA_def_property_enum_items(prop, rna_enum_node_tex_dimensions_items);
RNA_def_property_ui_text(prop, "Dimensions", "Number of dimensions to output noise for");
RNA_def_property_translation_context(prop, BLT_I18NCONTEXT_ID_NODETREE);

Wonder if BLT_I18NCONTEXT_ID_TEXTURE would not be a better (more logical) choice here?

Wonder if `BLT_I18NCONTEXT_ID_TEXTURE` would not be a better (more logical) choice here?
pioverfour marked this conversation as resolved
@ -16,3 +16,3 @@
b.add_input<decl::Rotation>("Rotation");
b.add_input<decl::Vector>("Scale").default_value(float3(1)).subtype(PROP_XYZ);
b.add_output<decl::Matrix>("Transform");
b.add_output<decl::Matrix>("Transform").translation_context(BLT_I18NCONTEXT_ID_NODETREE);

Not sure I understand why this needs a context?

I would expect only need for two contexts for this word ('noun' case with default context, 'verb' case with operator context)?

Not sure I understand why this needs a context? I would expect only need for two contexts for this word ('noun' case with default context, 'verb' case with operator context)?
Author
Member

At first it was because the French translation for the noun was wrong and used a verb, but then I checked some translations for a few languages and it appears that the translation is not always the same (in Chinese and Arabic, for example).
I’ll ask in the chat.

At first it was because the French translation for the noun was wrong and used a verb, but then I checked some translations for a few languages and it appears that the translation is not always the same (in Chinese and Arabic, for example). I’ll ask in the chat.
pioverfour marked this conversation as resolved
Damien Picard added 1 commit 2024-06-10 22:46:29 +02:00
Damien Picard added 1 commit 2024-06-11 00:27:01 +02:00
Author
Member

While this is open I added disambiguation for "Parent", in a new commit.

While this is open I added disambiguation for "Parent", in a new commit.
Damien Picard added 4 commits 2024-06-11 20:06:32 +02:00
There didn't seem to be much response from translators regarding the
need for this context. Could be added later if needed.
Author
Member

I added the Operator context for "Remove", except here where the label and description seem fishy. I’ll ask the animation module for advice.

I added the Operator context for "Remove", except [here](https://projects.blender.org/blender/blender/src/branch/main/source/blender/editors/space_graph/graph_slider_ops.cc#L630) where the label and description seem fishy. I’ll ask the animation module for advice.
Bastien Montagne approved these changes 2024-06-12 10:58:02 +02:00
Bastien Montagne left a comment
Owner

Thanks, LGTM!

Thanks, LGTM!

Ahhh, looks like the branch needs some merge before it can land though :)

Ahhh, looks like the branch needs some merge before it can land though :)
Damien Picard added 2 commits 2024-06-12 18:57:10 +02:00
Use proper translation command in extensions UI
All checks were successful
buildbot/vexp-code-patch-lint Build done.
buildbot/vexp-code-patch-linux-x86_64 Build done.
buildbot/vexp-code-patch-darwin-x86_64 Build done.
buildbot/vexp-code-patch-darwin-arm64 Build done.
buildbot/vexp-code-patch-windows-amd64 Build done.
buildbot/vexp-code-patch-coordinator Build done.
c1f93a2452
Author
Member

Done!
By the way a previous pull request (!122971) wasn’t squashed before merge, is this something I need to do?

Done! By the way a previous pull request (!122971) wasn’t squashed before merge, is this something I need to do?

By the way a previous pull request (!122971) wasn’t squashed before merge, is this something I need to do?

It depends, sometimes it's better to commit things in several times to separate logical aspects of a PR (e.g. adding a new feature, and using it in existing code). This helps when investigating potential issues later. ! could probably just have been split in only 2 commits, but was simpler/faster to directly commit the four, than spend some time rebasing things. ;)

> By the way a previous pull request (!122971) wasn’t squashed before merge, is this something I need to do? It depends, sometimes it's better to commit things in several times to separate logical aspects of a PR (e.g. adding a new feature, and using it in existing code). This helps when investigating potential issues later. ! could probably just have been split in only 2 commits, but was simpler/faster to directly commit the four, than spend some time rebasing things. ;)

@blender-bot build

@blender-bot build
Author
Member

could probably just have been split in only 2 commits, but was simpler/faster to directly commit the four, than spend some time rebasing things. ;)

I see, thanks!

> could probably just have been split in only 2 commits, but was simpler/faster to directly commit the four, than spend some time rebasing things. ;) I see, thanks!
Bastien Montagne merged commit f87d4e4e40 into blender-v4.2-release 2024-06-13 12:16:09 +02:00
Bastien Montagne deleted branch dp_disambiguate 2024-06-13 12:16:11 +02:00
Sign in to join this conversation.
No reviewers
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset System
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Code Documentation
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Viewport & EEVEE
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Asset Browser Project
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Module
Viewport & EEVEE
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Severity
High
Severity
Low
Severity
Normal
Severity
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Reference: blender/blender#122969
No description provided.