From: "Rubén Justo" <rjusto@gmail.com>
To: Git List <git@vger.kernel.org>
Cc: Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v2 0/3] add: use advise_if_enabled
Date: Sat, 30 Mar 2024 15:00:11 +0100 [thread overview]
Message-ID: <46fba030-d7aa-49d2-88fa-e506850f7b6a@gmail.com> (raw)
In-Reply-To: <06c9b422-b22e-4310-ad5b-1686616ab860@gmail.com>
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@gmail.com>
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@gmail.com>
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@pobox.com>
Signed-off-by: Rubén Justo <rjusto@gmail.com>
## 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
next prev parent reply other threads:[~2024-03-30 14:00 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-29 4:14 [PATCH 0/3] add: use advise_if_enabled Rubén Justo
2024-03-29 4:19 ` [PATCH 1/3] add: use advise_if_enabled for ADVICE_ADD_IGNORED_FILE Rubén Justo
2024-03-29 17:40 ` Junio C Hamano
2024-03-29 4:19 ` [PATCH 2/3] add: use advise_if_enabled for ADVICE_ADD_EMPTY_PATHSPEC Rubén Justo
2024-03-29 4:19 ` [PATCH 3/3] add: use advise_if_enabled for ADVICE_ADD_EMBEDDED_REPO Rubén Justo
2024-03-29 17:55 ` Junio C Hamano
2024-03-29 19:04 ` Rubén Justo
2024-03-29 19:31 ` Junio C Hamano
2024-03-29 19:59 ` Rubén Justo
2024-03-29 20:59 ` Junio C Hamano
2024-03-30 13:35 ` Rubén Justo
2024-03-29 17:28 ` [PATCH 0/3] add: use advise_if_enabled Junio C Hamano
2024-03-29 19:16 ` Rubén Justo
2024-03-30 14:00 ` Rubén Justo [this message]
2024-03-30 14:07 ` [PATCH v2 1/3] add: use advise_if_enabled for ADVICE_ADD_IGNORED_FILE Rubén Justo
2024-03-30 14:08 ` [PATCH v2 2/3] add: use advise_if_enabled for ADVICE_ADD_EMPTY_PATHSPEC Rubén Justo
2024-03-30 14:09 ` [PATCH v2 3/3] add: use advise_if_enabled for ADVICE_ADD_EMBEDDED_REPO Rubén Justo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=46fba030-d7aa-49d2-88fa-e506850f7b6a@gmail.com \
--to=rjusto@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).