git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "D. Ben Knoble" <ben.knoble+github@gmail.com>
To: git@vger.kernel.org
Cc: "D. Ben Knoble" <ben.knoble+github@gmail.com>,
	"Junio C Hamano" <gitster@pobox.com>,
	"Patrick Steinhardt" <ps@pks.im>,
	"Karthik Nayak" <karthik.188@gmail.com>,
	"Denton Liu" <liu.denton@gmail.com>,
	"Kyle Meyer" <kyle@kyleam.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH 4/9] stash: restore the index by default when breaking changes are enabled
Date: Sat, 10 May 2025 14:33:39 -0400	[thread overview]
Message-ID: <20250510183358.36806-5-ben.knoble+github@gmail.com> (raw)
In-Reply-To: <20250510183358.36806-1-ben.knoble+github@gmail.com>

This footgun is described in the previous commit to
Documentation/BreakingChanges.adoc; without --index, stash apply and
stash pop confusingly drop stashed index changes.

When compiling with breaking changes, instead restore the index unless
asked not to. Adjust both argument parsing and handling, as well as help
synopses. Don't forget to adjust the error message when application
fails to point to --no-index instead.

Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail.com>
---

Notes:
    Tests will follow in separate patches, as some changes are rather complicated.

 Documentation/git-stash.adoc |  6 ++++++
 builtin/stash.c              | 38 ++++++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/Documentation/git-stash.adoc b/Documentation/git-stash.adoc
index 1a5177f498..3b01f494a3 100644
--- a/Documentation/git-stash.adoc
+++ b/Documentation/git-stash.adoc
@@ -11,8 +11,14 @@ SYNOPSIS
 'git stash' list [<log-options>]
 'git stash' show [-u | --include-untracked | --only-untracked] [<diff-options>] [<stash>]
 'git stash' drop [-q | --quiet] [<stash>]
+ifndef::with-breaking-changes[]
 'git stash' pop [--index] [-q | --quiet] [<stash>]
 'git stash' apply [--index] [-q | --quiet] [<stash>]
+endif::with-breaking-changes[]
+ifdef::with-breaking-changes[]
+'git stash' pop [--no-index] [-q | --quiet] [<stash>]
+'git stash' apply [--no-index] [-q | --quiet] [<stash>]
+endif::with-breaking-changes[]
 'git stash' branch <branchname> [<stash>]
 'git stash' [push [-p | --patch] [-S | --staged] [-k | --[no-]keep-index] [-q | --quiet]
 	     [-u | --include-untracked] [-a | --all] [(-m | --message) <message>]
diff --git a/builtin/stash.c b/builtin/stash.c
index cfbd92852a..4ffa586d07 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -38,10 +38,17 @@
 	N_("git stash show [-u | --include-untracked | --only-untracked] [<diff-options>] [<stash>]")
 #define BUILTIN_STASH_DROP_USAGE \
 	N_("git stash drop [-q | --quiet] [<stash>]")
+#ifdef WITH_BREAKING_CHANGES
+#define BUILTIN_STASH_POP_USAGE \
+	N_("git stash pop [--no-index] [-q | --quiet] [<stash>]")
+#define BUILTIN_STASH_APPLY_USAGE \
+	N_("git stash apply [--no-index] [-q | --quiet] [<stash>]")
+#else
 #define BUILTIN_STASH_POP_USAGE \
 	N_("git stash pop [--index] [-q | --quiet] [<stash>]")
 #define BUILTIN_STASH_APPLY_USAGE \
 	N_("git stash apply [--index] [-q | --quiet] [<stash>]")
+#endif /* WITH_BREAKING_CHANGES */
 #define BUILTIN_STASH_BRANCH_USAGE \
 	N_("git stash branch <branchname> [<stash>]")
 #define BUILTIN_STASH_STORE_USAGE \
@@ -562,8 +569,13 @@ static int do_apply_stash(const char *prefix, struct stash_info *info,
 			ret = apply_cached(&out);
 			strbuf_release(&out);
 			if (ret)
+#ifdef WITH_BREAKING_CHANGES
+				return error(_("conflicts in index. "
+					       "Try with --no-index."));
+#else
 				return error(_("conflicts in index. "
 					       "Try without --index."));
+#endif /* WITH_BREAKING_CHANGES */
 
 			discard_index(the_repository->index);
 			repo_read_index(the_repository);
@@ -658,12 +670,21 @@ static int apply_stash(int argc, const char **argv, const char *prefix,
 {
 	int ret = -1;
 	int quiet = 0;
+#ifdef WITH_BREAKING_CHANGES
+	int no_index = 0;
+#else
 	int index = 0;
+#endif /* WITH_BREAKING_CHANGES */
 	struct stash_info info = STASH_INFO_INIT;
 	struct option options[] = {
 		OPT__QUIET(&quiet, N_("be quiet, only report errors")),
+#ifdef WITH_BREAKING_CHANGES
+		OPT_BOOL(0, "no-index", &no_index,
+			 N_("do not attempt to recreate the index")),
+#else
 		OPT_BOOL(0, "index", &index,
 			 N_("attempt to recreate the index")),
+#endif /* WITH_BREAKING_CHANGES */
 		OPT_END()
 	};
 
@@ -673,7 +694,11 @@ static int apply_stash(int argc, const char **argv, const char *prefix,
 	if (get_stash_info(&info, argc, argv))
 		goto cleanup;
 
+#ifdef WITH_BREAKING_CHANGES
+	ret = do_apply_stash(prefix, &info, !no_index, quiet);
+#else
 	ret = do_apply_stash(prefix, &info, index, quiet);
+#endif /* WITH_BREAKING_CHANGES */
 cleanup:
 	free_stash_info(&info);
 	return ret;
@@ -755,13 +780,22 @@ static int pop_stash(int argc, const char **argv, const char *prefix,
 		     struct repository *repo UNUSED)
 {
 	int ret = -1;
+#ifdef WITH_BREAKING_CHANGES
+	int no_index = 0;
+#else
 	int index = 0;
+#endif /* WITH_BREAKING_CHANGES */
 	int quiet = 0;
 	struct stash_info info = STASH_INFO_INIT;
 	struct option options[] = {
 		OPT__QUIET(&quiet, N_("be quiet, only report errors")),
+#ifdef WITH_BREAKING_CHANGES
+		OPT_BOOL(0, "no-index", &no_index,
+			 N_("do not attempt to recreate the index")),
+#else
 		OPT_BOOL(0, "index", &index,
 			 N_("attempt to recreate the index")),
+#endif /* WITH_BREAKING_CHANGES */
 		OPT_END()
 	};
 
@@ -771,7 +805,11 @@ static int pop_stash(int argc, const char **argv, const char *prefix,
 	if (get_stash_info_assert(&info, argc, argv))
 		goto cleanup;
 
+#ifdef WITH_BREAKING_CHANGES
+	if ((ret = do_apply_stash(prefix, &info, !no_index, quiet)))
+#else
 	if ((ret = do_apply_stash(prefix, &info, index, quiet)))
+#endif /* WITH_BREAKING_CHANGES */
 		printf_ln(_("The stash entry is kept in case "
 			    "you need it again."));
 	else
-- 
2.48.1


  parent reply	other threads:[~2025-05-10 18:34 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-10 18:33 [PATCH 0/9] make stash apply with --index by default D. Ben Knoble
2025-05-10 18:33 ` [PATCH 1/9] t3903: reduce dependencies on previous tests D. Ben Knoble
2025-05-10 18:33 ` [PATCH 2/9] t3905: remove unneeded blank line D. Ben Knoble
2025-05-10 18:33 ` [PATCH 3/9] BreakingChanges: announce stash {apply,pop} will imply --index D. Ben Knoble
2025-05-10 18:33 ` D. Ben Knoble [this message]
2025-05-10 18:33 ` [PATCH 5/9] t0450: mark stash documentation as a known discrepancy D. Ben Knoble
2025-05-10 18:33 ` [PATCH 6/9] t3903: adjust stash test to account for --[no-]index with breaking changes D. Ben Knoble
2025-05-10 18:33 ` [PATCH 7/9] t3904: adjust stash -p test to account for index states " D. Ben Knoble
2025-05-10 18:33 ` [PATCH 8/9] t3905: adjust stash -u tests for " D. Ben Knoble
2025-05-10 18:33 ` [PATCH 9/9] t3906: adjust stash submodule tests to account " D. Ben Knoble
2025-05-12 12:52 ` [PATCH 0/9] make stash apply with --index by default Junio C Hamano
2025-05-20 14:36 ` D. Ben Knoble
2025-05-20 14:39 ` D. Ben Knoble
2025-09-16  0:37 ` [PATCH v2 0/4] Teach git-stash to use --index from config D. Ben Knoble
2025-09-16  0:37   ` [PATCH v2 1/4] t3903: reduce dependencies on previous tests D. Ben Knoble
2025-09-16  0:37   ` [PATCH v2 2/4] t3905: remove unneeded blank line D. Ben Knoble
2025-09-16  0:37   ` [PATCH v2 3/4] stash: refactor private config globals D. Ben Knoble
2025-09-16  0:37   ` [PATCH v2 4/4] stash: honor stash.index in apply, pop modes D. Ben Knoble
2025-09-16  9:18     ` Phillip Wood
2025-09-16 17:07       ` D. Ben Knoble
2025-09-16  9:24   ` [PATCH v2 0/4] Teach git-stash to use --index from config Phillip Wood
2025-09-16 17:06     ` D. Ben Knoble
2025-09-16 16:49   ` Junio C Hamano
2025-09-22  1:39   ` [PATCH v3 " D. Ben Knoble
2025-09-22  1:39     ` [PATCH v3 1/4] t3903: reduce dependencies on previous tests D. Ben Knoble
2025-09-22  1:39     ` [PATCH v3 2/4] t3905: remove unneeded blank line D. Ben Knoble
2025-09-22  1:39     ` [PATCH v3 3/4] stash: refactor private config globals D. Ben Knoble
2025-09-22  1:39     ` [PATCH v3 4/4] stash: honor stash.index in apply, pop modes D. Ben Knoble
2025-09-22 14:11       ` Phillip Wood
2025-09-24 20:40         ` D. Ben Knoble
2025-09-29 10:01           ` Phillip Wood
2025-10-06 12:59             ` [PATCH] doc: explain the impact of stash.index on --autostash options D. Ben Knoble
2025-10-09 22:54               ` Kristoffer Haugsbakk
2025-10-11 14:44                 ` D. Ben Knoble
2025-10-11 17:28                   ` Junio C Hamano
2025-10-12 18:04                     ` Ben Knoble

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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250510183358.36806-5-ben.knoble+github@gmail.com \
    --to=ben.knoble+github@gmail.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=karthik.188@gmail.com \
    --cc=kyle@kyleam.com \
    --cc=liu.denton@gmail.com \
    --cc=ps@pks.im \
    /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.
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 NNTP newsgroup(s).