git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
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


  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).