Hi all, Finally got a chance to send the v2 of this one. Changes in v2: - Removed the 'if' from the message as Atharva mentioned that it was added inadvertently - Moved 'fatal:' prefix from middle of the message to the beginning. - Expanded the commit message to also mention the output after the change For reference, the v1 could be found here: https://public-inbox.org/git/20210805192803.679948-1-kaartic.sivaraam@xxxxxxxxx/ ... and the range-diff against v1 could be found below. Reviews appreciated. -- Sivaraam Kaartic Sivaraam (1): submodule--helper: fix incorrect newlines in an error message builtin/submodule--helper.c | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-) Range-diff against v1: 1: c00617bc03 ! 1: c6daed7a92 submodule--helper: fix incorrect newlines in an error message @@ Commit message After the refactoring the error started appearing like so: - --- START OF OUTPUT --- + --- START OF OUTPUT --- $ git submodule add ../sub/ subm A git directory for 'subm' is found locally with remote(s): origin /me/git-repos-for-test/sub fatal: If you want to reuse this local git directory instead of cloning again from @@ Commit message Make the error message consistent with the error message that used to be printed before the refactoring. + This also moves the 'fatal:' prefix that appears in the middle of the + error message to the first line as it would more appropriate to have + it in the first line. The output after the change would look like: + + --- START OF OUTPUT --- + $ git submodule add ../sub/ subm + fatal: A git directory for 'subm' is found locally with remote(s): + origin /me/git-repos-for-test/sub + If you want to reuse this local git directory instead of cloning again from + /me/git-repos-for-test/sub + use the '--force' option. If the local git directory is not the correct repo + or you are unsure what this means choose another name with the '--name' option. + --- END OF OUTPUT --- + [1]: https://lore.kernel.org/git/20210710074801.19917-5-raykar.ath@xxxxxxxxx/#t ## builtin/submodule--helper.c ## +@@ builtin/submodule--helper.c: struct add_data { + }; + #define ADD_DATA_INIT { .depth = -1 } + +-static void show_fetch_remotes(FILE *output, const char *sm_name, const char *git_dir_path) ++static void show_fetch_remotes(struct strbuf *msg, const char *sm_name, const char *git_dir_path) + { + struct child_process cp_remote = CHILD_PROCESS_INIT; + struct strbuf sb_remote_out = STRBUF_INIT; +@@ builtin/submodule--helper.c: static void show_fetch_remotes(FILE *output, const char *sm_name, const char *gi + while ((next_line = strchr(line, '\n')) != NULL) { + size_t len = next_line - line; + if (strip_suffix_mem(line, &len, " (fetch)")) +- fprintf(output, " %.*s\n", (int)len, line); ++ strbuf_addf(msg, " %.*s\n", (int)len, line); + line = next_line + 1; + } + } @@ builtin/submodule--helper.c: static int add_submodule(const struct add_data *add_data) + if (is_directory(submod_gitdir_path)) { if (!add_data->force) { - fprintf(stderr, _("A git directory for '%s' is found " +- fprintf(stderr, _("A git directory for '%s' is found " - "locally with remote(s):"), -+ "locally with remote(s):\n"), - add_data->sm_name); - show_fetch_remotes(stderr, add_data->sm_name, +- add_data->sm_name); +- show_fetch_remotes(stderr, add_data->sm_name, ++ struct strbuf msg = STRBUF_INIT; ++ char *die_msg; ++ ++ strbuf_addf(&msg, _("A git directory for '%s' is found " ++ "locally with remote(s):\n"), ++ add_data->sm_name); ++ ++ show_fetch_remotes(&msg, add_data->sm_name, submod_gitdir_path); -@@ builtin/submodule--helper.c: static int add_submodule(const struct add_data *add_data) - "use the '--force' option. If the local git " - "directory is not the correct repo\n" - "or if you are unsure what this means, choose " + free(submod_gitdir_path); +- die(_("If you want to reuse this local git " +- "directory instead of cloning again from\n" +- " %s\n" +- "use the '--force' option. If the local git " +- "directory is not the correct repo\n" +- "or if you are unsure what this means, choose " - "another name with the '--name' option.\n"), -+ "another name with the '--name' option."), - add_data->realrepo); +- add_data->realrepo); ++ ++ strbuf_addf(&msg, _("If you want to reuse this local git " ++ "directory instead of cloning again from\n" ++ " %s\n" ++ "use the '--force' option. If the local git " ++ "directory is not the correct repo\n" ++ "or you are unsure what this means choose " ++ "another name with the '--name' option."), ++ add_data->realrepo); ++ ++ die_msg = strbuf_detach(&msg, NULL); ++ die("%s", die_msg); } else { printf(_("Reactivating local git directory for " + "submodule '%s'\n"), add_data->sm_name); -- 2.32.0.385.gc00617bc03.dirty
