[PATCH v4 0/7] you-still-use-that??: improve breaking changes troubleshooting

[kristofferhaugsbakk@fastmail.com]

From: Kristoffer Haugsbakk <code@khaugsbakk.name>

Based on the recent i-still-use-that reports about whatchanged, improve
the error reporting with this command in mind:

1. Give more possible actions instead of just (only) asking them to send
   an email
2. Hint how to replace their git-whatchanged(1) use with git-log(1) or
   an alias `whatchanged` (you can alias deprecated commands now)
3. Minor documentation changes
4. Add `deprecated` to `git --list-cmds`

§ What the errors now look like

    $ /git whatchanged
    'git whatchanged' is nominated for removal.

    hint: You can replace 'git whatchanged <opts>' with:
    hint:   git log <opts> --raw --no-merges
    hint: Or make an alias:
    hint:   git config set --global alias.whatchanged 'log --raw --no-merges'

    If you still use this command, here's what you can do:

    - read https://git-scm.com/docs/BreakingChanges.html
    - check if anyone has discussed this on the mailing
      list and if they came up with something that can
      help you: https://lore.kernel.org/git/?q=git%20whatchanged
    - send an email to <git@vger.kernel.org> to let us
      know that you still use this command and were unable
      to determine a suitable replacement

    fatal: refusing to run without --i-still-use-this
    $ git pack-redundant
    'git pack-redundant' is nominated for removal.
    If you still use this command, here's what you can do:

    - read https://git-scm.com/docs/BreakingChanges.html
    - check if anyone has discussed this on the mailing
      list and if they came up with something that can
      help you: https://lore.kernel.org/git/?q=git%20pack-redundant
    - send an email to <git@vger.kernel.org> to let us
      know that you still use this command and were unable
      to determine a suitable replacement

    fatal: refusing to run without --i-still-use-this

§ Changes in v4

• Patch 1: updated with suggestion from Patrick
• Patch 2: better `is_deprecated_command` implementation suggested by
  Junio and Peff
• Minor adjustments based on the previous

Kristoffer Haugsbakk (7):
  git: add `deprecated` category to --list-cmds
  git: allow alias-shadowing deprecated builtins
  t0014: test shadowing of aliases for a sample of builtins
  you-still-use-that??: help the user help themselves
  whatchanged: tell users the git-log(1) equivalent
  whatchanged: remove not-even-shorter clause
  BreakingChanges: remove claim about whatchanged reports

 Documentation/BreakingChanges.adoc |  2 +-
 Documentation/config/alias.adoc    |  3 ++-
 Documentation/git-whatchanged.adoc |  8 ++++--
 Documentation/git.adoc             |  3 ++-
 builtin/log.c                      |  8 +++++-
 builtin/pack-redundant.c           |  2 +-
 git-compat-util.h                  |  2 +-
 git.c                              | 43 ++++++++++++++++++++++++------
 t/t0014-alias.sh                   | 34 +++++++++++++++++++++++
 usage.c                            | 33 ++++++++++++++++++-----
 10 files changed, 115 insertions(+), 23 deletions(-)

Interdiff against v3:
diff --git a/git.c b/git.c
index a452ce3f9e9..b3aafebfe4c 100644
--- a/git.c
+++ b/git.c
@@ -52,13 +52,9 @@ const char git_more_info_string[] =
 
 static int use_pager = -1;
 
-/*
- * 'include_option' and 'exclude_option' are mutually exclusive.
- *
- * The default ('!include_option') is to include everything
- * except those filtered out by 'exclude_option'.
- */
-static void list_builtins(struct string_list *list, unsigned int include_option, unsigned int exclude_option);
+static void list_builtins(struct string_list *list,
+			  unsigned int include_option,
+			  unsigned int exclude_option);
 
 static void exclude_helpers_from_list(struct string_list *list)
 {
@@ -677,18 +673,16 @@ int is_builtin(const char *s)
 	return !!get_builtin(s);
 }
 
-static void list_builtins(struct string_list *out, unsigned int include_option, unsigned int exclude_option)
+static void list_builtins(struct string_list *out,
+			  unsigned int include_option,
+			  unsigned int exclude_option)
 {
-	if (include_option && exclude_option)
-		BUG("'include_option' and 'exclude_option' are mutually exclusive");
-	if (include_option) {
-		for (size_t i = 0; i < ARRAY_SIZE(commands); i++)
-			if (commands[i].option & include_option)
-				string_list_append(out, commands[i].cmd);
-	} else {
-		for (size_t i = 0; i < ARRAY_SIZE(commands); i++)
-			if (!(commands[i].option & exclude_option))
-				string_list_append(out, commands[i].cmd);
+	for (size_t i = 0; i < ARRAY_SIZE(commands); i++) {
+		if (include_option && !(commands[i].option & include_option))
+			continue;
+		if (exclude_option && (commands[i].option & exclude_option))
+			continue;
+		string_list_append(out, commands[i].cmd);
 	}
 }
 
@@ -809,8 +803,8 @@ static void execv_dashed_external(const char **argv)
 
 static int is_deprecated_command(const char *cmd)
 {
-	return !strcmp(cmd, "whatchanged") ||
-	       !strcmp(cmd, "pack-redundant");
+	struct cmd_struct *builtin = get_builtin(cmd);
+	return builtin && (builtin->option & DEPRECATED);
 }
 
 static int run_argv(struct strvec *args)
Range-diff against v3:
1:  bdc683a92b3 ! 1:  66e6a9554b1 git: add `deprecated` category to --list-cmds
    @@ Commit message
     
         Let’s expand the experimental `--list-cmds`[1] to allow users and
         programs to query for this information.  We will also use this in an
    -    upcoming commit to assert that all deprecated commands will have been
    -    covered in some manner.
    +    upcoming commit to implement `is_deprecated_command`.
     
         [1]: Using something which is experimental to query for deprecations is
             perhaps not the most ideal approach, but it is simple to implement
             and better than having to scan the documentation
     
    +    Acked-by: Patrick Steinhardt <ps@pks.im>
    +    Helped-by: Patrick Steinhardt <ps@pks.im>
         Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
     
     
      ## Notes (series) ##
    +    v4:
    +
    +    Incorporate Patrick’s suggestions about the for-loop refactor and
    +    formatting the overlong lines.  Now drop the function doc since it
    +    doesn’t apply anymore.
    +
    +    Also adjust the commit message now that we use it in the C source in an
    +    upcoming commit, not just/only as an assert-helper (in an upcoming
    +    commit).
    +
         v3 (new):
     
         This is something I wanted to submit independently until the point about
    @@ git.c: const char git_more_info_string[] =
      static int use_pager = -1;
      
     -static void list_builtins(struct string_list *list, unsigned int exclude_option);
    -+/*
    -+ * 'include_option' and 'exclude_option' are mutually exclusive.
    -+ *
    -+ * The default ('!include_option') is to include everything
    -+ * except those filtered out by 'exclude_option'.
    -+ */
    -+static void list_builtins(struct string_list *list, unsigned int include_option, unsigned int exclude_option);
    ++static void list_builtins(struct string_list *list,
    ++			  unsigned int include_option,
    ++			  unsigned int exclude_option);
      
      static void exclude_helpers_from_list(struct string_list *list)
      {
    @@ git.c: int is_builtin(const char *s)
      }
      
     -static void list_builtins(struct string_list *out, unsigned int exclude_option)
    -+static void list_builtins(struct string_list *out, unsigned int include_option, unsigned int exclude_option)
    ++static void list_builtins(struct string_list *out,
    ++			  unsigned int include_option,
    ++			  unsigned int exclude_option)
      {
    --	for (size_t i = 0; i < ARRAY_SIZE(commands); i++) {
    + 	for (size_t i = 0; i < ARRAY_SIZE(commands); i++) {
     -		if (exclude_option &&
     -		    (commands[i].option & exclude_option))
    --			continue;
    --		string_list_append(out, commands[i].cmd);
    -+	if (include_option && exclude_option)
    -+		BUG("'include_option' and 'exclude_option' are mutually exclusive");
    -+	if (include_option) {
    -+		for (size_t i = 0; i < ARRAY_SIZE(commands); i++)
    -+			if (commands[i].option & include_option)
    -+				string_list_append(out, commands[i].cmd);
    -+	} else {
    -+		for (size_t i = 0; i < ARRAY_SIZE(commands); i++) {
    -+			if (commands[i].option & exclude_option)
    -+				continue;
    -+			string_list_append(out, commands[i].cmd);
    -+		}
    ++		if (include_option && !(commands[i].option & include_option))
    ++			continue;
    ++		if (exclude_option && (commands[i].option & exclude_option))
    + 			continue;
    + 		string_list_append(out, commands[i].cmd);
      	}
    - }
    - 
2:  183dd68d09d < -:  ----------- git: make the two loops look more symmetric
3:  eec01cbac16 ! 2:  672253e0e71 git: allow alias-shadowing deprecated builtins
    @@ Commit message
     
     
      ## Notes (series) ##
    +    v4:
    +
    +    Better `is_deprecated_command` implementation.
    +
         v3 (new):
     
         Prerequisite for telling the user that they can alias `whatchanged` to
    @@ git.c: static void execv_dashed_external(const char **argv)
      
     +static int is_deprecated_command(const char *cmd)
     +{
    -+	return !strcmp(cmd, "whatchanged") ||
    -+	       !strcmp(cmd, "pack-redundant");
    ++	struct cmd_struct *builtin = get_builtin(cmd);
    ++	return builtin && (builtin->option & DEPRECATED);
     +}
     +
      static int run_argv(struct strvec *args)
4:  80fb02caeeb = 3:  00108f28f82 t0014: test shadowing of aliases for a sample of builtins
5:  d25ee26f989 = 4:  6bdcaf7f80f you-still-use-that??: help the user help themselves
6:  50621a0748f = 5:  58de9767b22 whatchanged: tell users the git-log(1) equivalent
7:  812c9870f1b = 6:  407b430d02c whatchanged: remove not-even-shorter clause
8:  0d23a4badf0 = 7:  fee752d2fb0 BreakingChanges: remove claim about whatchanged reports

base-commit: c44beea485f0f2feaf460e2ac87fdd5608d63cf0
-- 
2.51.0.16.gcd94ab5bf81