On Tue, May 15, 2018 at 6:44 PM, Stefan Beller <sbeller@xxxxxxxxxx> wrote: > On Tue, May 15, 2018 at 6:13 AM, Duy Nguyen <pclouds@xxxxxxxxx> wrote: >> On Tue, May 15, 2018 at 3:04 AM, Stefan Beller <sbeller@xxxxxxxxxx> wrote: >>> Any other caller of 'repo_read_index' dies upon a negative return of >>> it, so grep should, too. >>> >>> Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx> >>> --- >>> >>> Found while reviewing the series >>> https://public-inbox.org/git/20180514105823.8378-1-ao2@xxxxxx/ >>> >>> builtin/grep.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/builtin/grep.c b/builtin/grep.c >>> index 6e7bc76785a..69f0743619f 100644 >>> --- a/builtin/grep.c >>> +++ b/builtin/grep.c >>> @@ -488,7 +488,8 @@ static int grep_cache(struct grep_opt *opt, struct repository *repo, >>> strbuf_addstr(&name, repo->submodule_prefix); >>> } >>> >>> - repo_read_index(repo); >>> + if (repo_read_index(repo) < 0) >>> + die("index file corrupt"); >> >> _() the string (and maybe reuse an existing phrase if found to reduce >> workload on translators) > > sbeller@sbeller:/u/git$ git grep -A 1 repo_read_index > builtin/grep.c:491: if (repo_read_index(repo) < 0) > builtin/grep.c-492- die("index file corrupt"); > -- > builtin/ls-files.c:213: if (repo_read_index(&submodule) < 0) > builtin/ls-files.c-214- die("index file corrupt"); > -- > builtin/ls-files.c:582: if (repo_read_index(the_repository) < 0) > builtin/ls-files.c-583- die("index file corrupt"); > -- > dir.c:3028: if (repo_read_index(&subrepo) < 0) > dir.c-3029- die("index file corrupt in repo %s", subrepo.gitdir); > -- > repository.c:245:int repo_read_index(struct repository *repo) > repository.c-246-{ > -- > repository.h:70: * 'repo_read_index()' can be used to populate 'index'. > repository.h-71- */ > -- > repository.h:119:extern int repo_read_index(struct repository *repo); > repository.h-120- > -- > submodule-config.c:583: if (repo_read_index(repo) < 0) > submodule-config.c-584- return; > -- > submodule.c:1336: if (repo_read_index(r) < 0) > submodule.c-1337- die("index file corrupt"); > > I think this is as good as it gets for using an existing phrase. > None of them are translated, which I would defer to a follow up patch > that translates all(?) of them or just the porcelains. If you have time, yes translate them all. I don't see how any of these strings are meant for script. If not, just _() the new string you added is fine. With a majority of call sites dying like this though, I wonder if we should just add repo_read_index_or_die() with die() inside. Then the next person won't likely accidentally forget _() > > Thanks, > Stefan -- Duy