Re: [PATCH v5 5/5] worktree: copy sparse-checkout patterns and config on add

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Le 07/02/2022 à 15:10, Derrick Stolee a écrit :
On 2/6/2022 5:36 AM, Jean-Noël AVILA wrote:
On Monday, 31 January 2022 16:00:59 CET Derrick Stolee via GitGitGadget wrote:

Hi Jean-Noël. Thanks for your attention to the translatable messages
here:

error(_("failed to copy '%s' to '%s'; sparse-checkout may not work correctly"),
       from_file, to_file);

die(_("failed to copy worktree config from '%s' to '%s'"),
     from_file, to_file);

error(_("failed to unset 'core.bare' in '%s'"), to_file);

error(_("failed to unset 'core.worktree' in '%s'"), to_file);

In the first patch of this series, you use _("unable to set '%s' in'%s'). Does it make sense to reuse this string here?

I would argue that "unable to set" is not appropriate for any of these
messages. Perhaps the "failed to copy" messages might be able to use
"unable to set", but the information that the config setting is coming
from settings the user controlled is valuable.

The "failed to unset" means "we are trying to _remove_ this setting
from the config file", so "unable to set" does not seem to work here.

I'm open to revisiting this if you disagree.

Thanks,
-Stolee


Hi Derrick,

Sorry for not being more precise. The first two errors were not the subject of this remark.

For the last two, this is quite surprising that the same function failing (git_config_set_in_file_gently) can lead to different error messages.

In any case, I would argue at least for  shifting to :

    error(_("failed to unset '%s' in '%s'"), 'core.bare", to_file);
and
    error(_("failed to unset '%s' in '%s'"), "core.worktree", to_file);

in order to factorize the message and get the option name out of the way.

Thanks,

Jean-Noël



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux