I18n: write messages of the vertex group lock operator explicitly #104484

Damien Picard · 2023-02-08T22:10:47+01:00

Damien Picard commented

2023-02-08 22:10:47 +01:00

The operator's description used a format string like:

BLI_sprintfN(TIP_("%s %s vertex groups of the active object"),
             action_str, target_str)

which is almost guaranteed to be impossible to properly localize to
some languages -- I know there are a couple of issues for French
already.

So instead of hoping formatting works, write all possible strings
explicitly, even if it looks stupidly verbose.

Review started in D16240.

The operator's description used a format string like: ``` BLI_sprintfN(TIP_("%s %s vertex groups of the active object"), action_str, target_str) ``` which is almost guaranteed to be impossible to properly localize to some languages -- I know there are a couple of issues for French already. So instead of hoping formatting works, write all possible strings explicitly, even if it looks stupidly verbose. ----- Review started in [D16240](https://archive.blender.org/developer/D16240).

Damien Picard requested review from Campbell Barton 2023-02-08 22:12:13 +01:00

Damien Picard requested review from Bastien Montagne 2023-02-08 22:12:13 +01:00

Hans Goudey reviewed 2023-02-08 23:10:06 +01:00

source/blender/editors/object/object_vgroup.cc

						
				@ -3453,1 +3453,3 @@

				      action_str = TIP_("Lock");

				      switch (mask) {

				        case VGROUP_MASK_ALL:

				          description = TIP_("Lock all vertex groups of the active object");

Hans Goudey commented

2023-02-08 23:09:36 +01:00

I'd suggest putting the return directly in the switch cases and removing the break lines like

case VGROUP_MASK_ALL:
          return BLI_strdup(TIP_("Lock all vertex groups of the active object"));
case ...:

That would make the duplication here more manageable (and just take up less screen space!)

I'd suggest putting the `return` directly in the switch cases and removing the `break` lines like ``` case VGROUP_MASK_ALL: return BLI_strdup(TIP_("Lock all vertex groups of the active object")); case ...: ``` That would make the duplication here more manageable (and just take up less screen space!)

Damien Picard commented

2023-02-09 20:21:31 +01:00

You’re right, it’s much more legible!

pioverfour marked this conversation as resolved

Damien Picard force-pushed dp_improve_lock_vertex_group from 0fd822864b to a06a875fc6

2023-02-09 20:20:15 +01:00

Compare

Campbell Barton reviewed 2023-02-10 00:30:05 +01:00

source/blender/editors/object/object_vgroup.cc

						
				@ -3448,4 +3448,2 @@

				  const char *action_str, *target_str;

				  switch (action) {

Campbell Barton commented

2023-02-10 00:30:05 +01:00

It would be good to include a comment, adding context to why strings are handled this way, otherwise someone who stumbles on the code might this as an opportunity to de-duplicate text.

/* NOTE: constructing the following string literals can be done in a less verbose way,
 * however the resulting strings can't be usefully translated, (via `TIP_`). */`

It would be good to include a comment, adding context to why strings are handled this way, otherwise someone who stumbles on the code might this as an opportunity to de-duplicate text. ``` /* NOTE: constructing the following string literals can be done in a less verbose way, * however the resulting strings can't be usefully translated, (via `TIP_`). */`

👍 1

pioverfour marked this conversation as resolved

Campbell Barton approved these changes 2023-02-10 00:32:00 +01:00

Campbell Barton left a comment

Approving with one minor request.

Damien Picard force-pushed dp_improve_lock_vertex_group from a06a875fc6 to fe384fdc41

2023-02-10 01:00:08 +01:00

Compare

Brecht Van Lommel added this to the User Interface project 2023-02-13 09:22:44 +01:00

Bastien Montagne added the

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

I18n: write messages of the vertex group lock operator explicitly #104484

Pull request closed