Đoàn Trần Công Danh <congdanhqx@xxxxxxxxx> writes:
>> In other words, which of the following 3 is the most acceptable?
>>
>> 1. As Junio suggested, quoted above.
>
> I think this approach is the most expensive one, _() needs to query
> the gettext infrastructure, which is usually costly.
> However, I think that cost doesn't matter much since we're about to
> open an editor soon.
See note below.
>> 2.
>> status_printf(s, GIT_COLOR_NORMAL, allow_empty_message ?
>> _("...") :
>> _("...."), comment_line_char);
>
> install_branch_config() uses this style.
>
>>
>> 3.
>> const char *hint_foo = allow_empty_message ?
>> _("...") :
>> _("....");
>
> builtin/remote.c:show_local_info_item() writes:
>
> const char *msg;
> if (condition)
> msg = _("some message");
> else
> msg = _("other message");
>
> So, I guess it's fine either way. And people will need to see the
> patch to see which one is better.
Yeah, #1 and #3 are better than the patch posted or #2 in that by
extracting the large message body out-of-line from the code that do
use the messages, they make it simpler to follow the logic that uses
these messages. That is
if (cleanup_mode == CLEANUP_ALL)
status_printf(..., hint_cleanup_all);
else if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS)
...;
else /* all the rest */
status_printf(..., hint_cleanup_space, comment_line_char);
would be far easier to follow than
if (cleanup_mode == CLEANUP_ALL)
status_printf(...,
condition
? large-large-message-1
: large-large-message-1-plus-note-about-empty-message);
else if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS)
...;
else /* all the rest */
status_printf(...,
condition
? large-large-message-2
: large-large-message-2-plus-note-about-empty-message,
comment_line_char);
as the overall structure is easier to follow without the minute
detail of using slightly different messages depending on the
allow-empty setting.
By the way, if you want to avoid calling _() twice with the approach
#1, you can do
hint_cleanup_all = N_("<cleanup and note about empty message>");
...
if (condition) {
hint_cleanup_all = N_("<cleanup without note>");
...
}
and use _(hint_cleanup_all) at the site that uses the message.
