On Mon, Feb 3, 2025 at 5:34 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Mon, Feb 3, 2025 at 6:16 AM Peter Smith <smithpb2...@gmail.com> wrote: > > > > > > 2. > > + appendStringInfo(&err_hint, "You might need to increase \"%s\".", > > + "max_slot_wal_keep_size"); > > break; > > 2a. > > In this case, shouldn't you really be using macro _("You might need to > > increase \"%s\".") so that the common format string would be got using > > gettext()? > > > > ~ > > > > > > ~~~ > > > > 3. > > + appendStringInfo(&err_hint, "You might need to increase \"%s\".", > > + "idle_replication_slot_timeout"); > > + break; > > > > 3a. > > Ditto above. IMO this common format string should be got using macro. > > e.g.: _("You might need to increase \"%s\".") > > > > ~ > > Instead, we can directly use '_(' in errhint as we are doing in one > other similar place "errhint("%s", _(view_updatable_error))));". I > think we didn't use it for errdetail because, in one of the cases, it > needs to use ngettext >
-1 for this suggestion because this will end up causing a gettext() on the entire hint where the GUC has already been substituted. e.g. it is effectively doing _("You might need to increase \"max_slot_wal_keep_size\".") _("You might need to increase \"idle_replication_slot_timeout\".") But that is contrary to the goal of reducing the burden on translators by using *common* messages wherever possible. IMO we should only request translation of the *common* part of the hint message. e.g. _("You might need to increase \"%s\".") ~~~ We always do GUC name substitution into a *common* fmt message because then translators only need to maintain a single translated string instead of many. You can find examples of this everywhere. For example, notice the GUC is always substituted into the translated fmt msgs below; they never have the GUC name included explicitly. The result is just a single fmt message is needed. $ grep -r . -e 'errhint("You might need to increase' | grep '.c:' ./src/backend/replication/logical/launcher.c: errhint("You might need to increase \"%s\".", "max_logical_replication_workers"))); ./src/backend/replication/logical/launcher.c: errhint("You might need to increase \"%s\".", "max_worker_processes"))); ./src/backend/storage/lmgr/predicate.c: errhint("You might need to increase \"%s\".", "max_pred_locks_per_transaction"))); ./src/backend/storage/lmgr/predicate.c: errhint("You might need to increase \"%s\".", "max_pred_locks_per_transaction"))); ./src/backend/storage/lmgr/predicate.c: errhint("You might need to increase \"%s\".", "max_pred_locks_per_transaction"))); ./src/backend/storage/lmgr/lock.c: errhint("You might need to increase \"%s\".", "max_locks_per_transaction"))); ./src/backend/storage/lmgr/lock.c: errhint("You might need to increase \"%s\".", "max_locks_per_transaction"))); ./src/backend/storage/lmgr/lock.c: errhint("You might need to increase \"%s\".", "max_locks_per_transaction"))); ./src/backend/storage/lmgr/lock.c: errhint("You might need to increase \"%s\".", "max_locks_per_transaction"))); ./src/backend/storage/lmgr/lock.c: errhint("You might need to increase \"%s\".", "max_locks_per_transaction"))); ====== Kind Regards, Peter Smith. Fujitsu Australia