Wolfgang Müller <wolf@oriole.systems> writes: > On 2021-05-16 21:53, Junio C Hamano wrote: > >> As die() is end-user facing, you'd probably want >> >> die(_("--path-format requires an argument")); >> >> We do have untranslated die() nearby for the same option, which may >> want to be cleaned up either in a preliminary patch, or in this same >> patch as an unrelated fix "while we are at it". > > I would not mind preparing a preliminary patch that cleans up all > untranslated user-facing calls to die(). My editor finds 15 of those in > rev-parse.c, and I think they all qualify. > > If you'd rather not touch unrelated code paths I'll instead include it > in v2 as an unrelated fix in the same commit. I am puzzled by the last paragraph. Somebody who does not want to see "unrelated" codepaths touched would appreciate if a commit that fixes this segfault does not touch them at the same time. In any case, I now counted existing die() messages in this file, and among 15 of them, only 1 is marked with _(...). I think that it is the best to apply the patch as-is (without _(...)), adding one untranslated message to the file. Then, on top of this change, the 15 untranslated messages can be marked with _(...) a separate commit (and it does not even have to be done by you). > I think I initially went for "--path-format --show-toplevel" because I > was under the assumption that --path-format needs another option it can > modify. It seems that this is not the case, so wouldn't it be simpler > here to do the following instead: > > test_must_fail git rev-parse --path-format > > That way we do not have to worry about subsequent changes to other, > unrelated, options. That's good, too. Simple. Thanks.