"Andrew Klotz via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > Currently invalid boolean config values return messages about 'bad numeric', > which I found misleading when the error was due to a boolean string value. > This change makes the error message reflect the boolean value. > > The current approach relies on GIT_TEST_GETTEXT_POISON being a boolean > value, moving its special case out fromdie_bad_number() and into > git_config_bool_or_int(). The approach does not make anything worse than what we currently have, which is good. I am undecided if we want to apply 2/2, or if we want to apply 1/2 alone without 2/2. If we applied 2/2, those who are reading the code in a year who forgot about this review thread would have to wonder if all values assigned to the variable bad_numeric are enclosed in _() and go up to find all assignments. Omitting 2/2 would keep _() around the message string fed to die(), so it may be easier to immediately see that the call to die is not missing basic i18n, but there is a risk to forget marking with N_(). If we were to use 2/2 in addition to 1/2, then squashing them into one commit will make the result easier to follow, because we no longer need an untranslated string in bad_numeric after 1/2 is applied. We are losing "the reason why we use N_() is..." comment in 1/2 anyway so doing what 2/2 does in the same commit would be more sensible than splitting these into two patches. I dunno. Thanks.
