This series is a simple change, in builtin/add.c, from:
if (advice_enabled(XXX))
advise(MMM)
to the newer:
advise_if_enabled(XXX, MMM)
Rubén Justo (3):
add: use advise_if_enabled for ADVICE_ADD_IGNORED_FILE
add: use advise_if_enabled for ADVICE_ADD_EMPTY_PATHSPEC
add: use advise_if_enabled for ADVICE_ADD_EMBEDDED_REPO
builtin/add.c | 18 ++++++---------
t/t3700-add.sh | 47 ++++++++++++++++++++++++++++++++++++--
t/t7400-submodule-basic.sh | 3 +--
3 files changed, 53 insertions(+), 15 deletions(-)
Range-diff against v1:
1: c599ff8b98 ! 1: 9462b7045d add: use advise_if_enabled for ADVICE_ADD_IGNORED_FILE
@@ Metadata
## Commit message ##
add: use advise_if_enabled for ADVICE_ADD_IGNORED_FILE
- Use the newer advise_if_enabled() machinery to show the advice.
+ 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 <rjusto@xxxxxxxxx>
2: 76550e01e1 ! 2: f892013059 add: use advise_if_enabled for ADVICE_ADD_EMPTY_PATHSPEC
@@ Metadata
## Commit message ##
add: use advise_if_enabled for ADVICE_ADD_EMPTY_PATHSPEC
- Use the newer advise_if_enabled() machinery to show the advice.
+ 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".
- We don't have a test for this. Add one.
+ 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 <rjusto@xxxxxxxxx>
3: 3017dd2188 ! 3: 254ece0ee4 add: use advise_if_enabled for ADVICE_ADD_EMBEDDED_REPO
@@ Metadata
## Commit message ##
add: use advise_if_enabled for ADVICE_ADD_EMBEDDED_REPO
- Use the newer advise_if_enabled() machinery to show the advice.
+ 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.
- We don't have a test for this. Add one.
+ 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 <gitster@xxxxxxxxx>
Signed-off-by: Rubén Justo <rjusto@xxxxxxxxx>
## builtin/add.c ##
@@ t/t3700-add.sh: test_expect_success '"git add ." in empty repo' '
)
'
-+test_expect_success '"git add" a nested repository' '
-+ rm -fr empty &&
-+ git init empty &&
++test_expect_success '"git add" a embedded repository' '
++ rm -fr outer && git init outer &&
+ (
-+ cd empty &&
-+ git init empty &&
-+ (
-+ cd empty &&
-+ git commit --allow-empty -m "foo"
-+ ) &&
-+ git add empty 2>actual &&
++ 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: empty
++ 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 <url> empty
++ hint: git submodule add <url> inner1
+ hint:
+ hint: If you added this path by mistake, you can remove it from the
+ hint: index with:
+ hint:
-+ hint: git rm --cached empty
++ 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
+ )
base-commit: 2d8cf94b28de9da683ddd40961a3a572f2741cf3
--
2.44.0.417.g254ece0ee4
