From 9da49befd0ac6d014433c1ecb1a55570dbca9290 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Justo?= Date: Sat, 30 Mar 2024 15:07:27 +0100 Subject: [PATCH 1/3] add: use advise_if_enabled for ADVICE_ADD_IGNORED_FILE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since b3b18d1621 (advice: revamp advise API, 2020-03-02), we can use advise_if_enabled() to display an advice. This API encapsulates three actions: 1.- checking the visibility of the advice 2.- displaying the advice when appropriate 3.- displaying instructions on how to disable the advice, when appropriate The code we have in builtin/add.c to display the ADVICE_ADD_IGNORED_FILE advice, is doing these three things. However, the instructions displayed on how to disable the hint are not shown in the normalized way that advise_if_enabled() introduced. This may cause distraction. There is no reason not to use the new API here. On the contrary, by using it we gain simplicity in the code and avoid possible distractions. For these reasons, use the newer advise_if_enabled() machinery to show the ADVICE_ADD_IGNORED_FILE advice, and don't bother checking the visibility or displaying the instruction on how to disable the advice. Signed-off-by: Rubén Justo Signed-off-by: Junio C Hamano --- builtin/add.c | 6 ++---- t/t3700-add.sh | 3 +-- t/t7400-submodule-basic.sh | 3 +-- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/builtin/add.c b/builtin/add.c index ada7719561..092d3120b5 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -328,10 +328,8 @@ static int add_files(struct dir_struct *dir, int flags) fprintf(stderr, _(ignore_error)); for (i = 0; i < dir->ignored_nr; i++) fprintf(stderr, "%s\n", dir->ignored[i]->name); - if (advice_enabled(ADVICE_ADD_IGNORED_FILE)) - advise(_("Use -f if you really want to add them.\n" - "Turn this message off by running\n" - "\"git config advice.addIgnoredFile false\"")); + advise_if_enabled(ADVICE_ADD_IGNORED_FILE, + _("Use -f if you really want to add them.")); exit_status = 1; } diff --git a/t/t3700-add.sh b/t/t3700-add.sh index f23d39f0d5..76c2c9e7b0 100755 --- a/t/t3700-add.sh +++ b/t/t3700-add.sh @@ -370,8 +370,7 @@ cat >expect.err <<\EOF The following paths are ignored by one of your .gitignore files: ignored-file hint: Use -f if you really want to add them. -hint: Turn this message off by running -hint: "git config advice.addIgnoredFile false" +hint: Disable this message with "git config advice.addIgnoredFile false" EOF cat >expect.out <<\EOF add 'track-this' diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh index 00c1f1aab1..5c4a89df5c 100755 --- a/t/t7400-submodule-basic.sh +++ b/t/t7400-submodule-basic.sh @@ -212,8 +212,7 @@ test_expect_success 'submodule add to .gitignored path fails' ' The following paths are ignored by one of your .gitignore files: submod hint: Use -f if you really want to add them. - hint: Turn this message off by running - hint: "git config advice.addIgnoredFile false" + hint: Disable this message with "git config advice.addIgnoredFile false" EOF # Does not use test_commit due to the ignore echo "*" > .gitignore && From 1028db00f7bc47e2cecda8a4a9967fcc05fffb48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Justo?= Date: Sat, 30 Mar 2024 15:08:59 +0100 Subject: [PATCH 2/3] add: use advise_if_enabled for ADVICE_ADD_EMPTY_PATHSPEC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since 93b0d86aaf (git-add: error out when given no arguments., 2006-12-20) we display a message when no arguments are given to "git add". Part of that message was converted to advice in bf66db37f1 (add: use advise function to display hints, 2020-01-07). Following the same line of reasoning as in the previous commit, it is sensible to use advise_if_enabled() here. Therefore, use advise_if_enabled() in builtin/add.c to show the ADVICE_ADD_EMPTY_PATHSPEC advice, and don't bother checking there the visibility of the advice or displaying the instruction on how to disable it. Also add a test for these messages, in order to detect a possible change in them. Signed-off-by: Rubén Justo Signed-off-by: Junio C Hamano --- builtin/add.c | 6 ++---- t/t3700-add.sh | 10 ++++++++++ 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/builtin/add.c b/builtin/add.c index 092d3120b5..0f981f5aa1 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -438,10 +438,8 @@ int cmd_add(int argc, const char **argv, const char *prefix) if (require_pathspec && pathspec.nr == 0) { fprintf(stderr, _("Nothing specified, nothing added.\n")); - if (advice_enabled(ADVICE_ADD_EMPTY_PATHSPEC)) - advise( _("Maybe you wanted to say 'git add .'?\n" - "Turn this message off by running\n" - "\"git config advice.addEmptyPathspec false\"")); + advise_if_enabled(ADVICE_ADD_EMPTY_PATHSPEC, + _("Maybe you wanted to say 'git add .'?")); return 0; } diff --git a/t/t3700-add.sh b/t/t3700-add.sh index 76c2c9e7b0..681081e0d5 100755 --- a/t/t3700-add.sh +++ b/t/t3700-add.sh @@ -28,6 +28,16 @@ test_expect_success 'Test of git add' ' touch foo && git add foo ' +test_expect_success 'Test with no pathspecs' ' + cat >expect <<-EOF && + Nothing specified, nothing added. + hint: Maybe you wanted to say ${SQ}git add .${SQ}? + hint: Disable this message with "git config advice.addEmptyPathspec false" + EOF + git add 2>actual && + test_cmp expect actual +' + test_expect_success 'Post-check that foo is in the index' ' git ls-files foo | grep foo ' From 6412d0152774155393ed97003a5c6121cf0a4684 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Justo?= Date: Sat, 30 Mar 2024 15:09:29 +0100 Subject: [PATCH 3/3] add: use advise_if_enabled for ADVICE_ADD_EMBEDDED_REPO MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit By following a similar reasoning as in previous commits, there are no reason why we should not use the advise_if_enabled() API to display the ADVICE_ADD_EMBEDDED_REPO advice. This advice was introduced in 532139940c (add: warn when adding an embedded repository, 2017-06-14). Some tests were included in the commit, but none is testing this advice. Which, note, we only want to display once per run. So, use the advise_if_enabled() machinery to show the ADVICE_ADD_EMBEDDED_REPO advice and include a test to notice any possible breakage. Signed-off-by: Junio C Hamano Signed-off-by: Rubén Justo Signed-off-by: Junio C Hamano --- builtin/add.c | 6 +++--- t/t3700-add.sh | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/builtin/add.c b/builtin/add.c index 0f981f5aa1..ec3bff1017 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -310,9 +310,9 @@ static void check_embedded_repo(const char *path) strbuf_strip_suffix(&name, "/"); warning(_("adding embedded git repository: %s"), name.buf); - if (!adviced_on_embedded_repo && - advice_enabled(ADVICE_ADD_EMBEDDED_REPO)) { - advise(embedded_advice, name.buf, name.buf); + if (!adviced_on_embedded_repo) { + advise_if_enabled(ADVICE_ADD_EMBEDDED_REPO, + embedded_advice, name.buf, name.buf); adviced_on_embedded_repo = 1; } diff --git a/t/t3700-add.sh b/t/t3700-add.sh index 681081e0d5..839c904745 100755 --- a/t/t3700-add.sh +++ b/t/t3700-add.sh @@ -349,6 +349,40 @@ test_expect_success '"git add ." in empty repo' ' ) ' +test_expect_success '"git add" a embedded repository' ' + rm -fr outer && git init outer && + ( + cd outer && + for i in 1 2 + do + name=inner$i && + git init $name && + git -C $name commit --allow-empty -m $name || + return 1 + done && + git add . 2>actual && + cat >expect <<-EOF && + warning: adding embedded git repository: inner1 + hint: You${SQ}ve added another git repository inside your current repository. + hint: Clones of the outer repository will not contain the contents of + hint: the embedded repository and will not know how to obtain it. + hint: If you meant to add a submodule, use: + hint: + hint: git submodule add inner1 + hint: + hint: If you added this path by mistake, you can remove it from the + hint: index with: + hint: + hint: git rm --cached inner1 + hint: + hint: See "git help submodule" for more information. + hint: Disable this message with "git config advice.addEmbeddedRepo false" + warning: adding embedded git repository: inner2 + EOF + test_cmp expect actual + ) +' + test_expect_success 'error on a repository with no commits' ' rm -fr empty && git init empty &&