diff mbox series

[RFC] stash save/push: add --index-only option

Message ID 20200212235033.782656-1-eantoranz@gmail.com (mailing list archive)
State New, archived
Headers show
Series [RFC] stash save/push: add --index-only option | expand

Commit Message

Edmundo Carmona Antoranz Feb. 12, 2020, 11:50 p.m. UTC 
Allow git-stash to save only the content of what's in staging area.
No working tree files (unstaged or untracked) will be added
to the stash nor will be modified then the opreration finishes.

Signed-off-by: Edmundo Carmona Antoranz <eantoranz@gmail.com>
---
 builtin/stash.c | 115 ++++++++++++++++++++++++++++++------------------
 1 file changed, 72 insertions(+), 43 deletions(-)

Comments

Edmundo Carmona Antoranz Feb. 12, 2020, 11:52 p.m. UTC | #1 
On Wed, Feb 12, 2020 at 5:50 PM Edmundo Carmona Antoranz
<eantoranz@gmail.com> wrote:
>
> Allow git-stash to save only the content of what's in staging area.
> No working tree files (unstaged or untracked) will be added
> to the stash nor will be modified then the opreration finishes.
>

Sorry for the double tag. This is not fully completed code. Wanted to
know if the community thinks it's something worth developing and if
I'm heading in the right direction in terms of the code that I'm
hitting.

Thanks!
Junio C Hamano Feb. 13, 2020, 5:04 a.m. UTC | #2 
Edmundo Carmona Antoranz <eantoranz@gmail.com> writes:

> On Wed, Feb 12, 2020 at 5:50 PM Edmundo Carmona Antoranz
> <eantoranz@gmail.com> wrote:
>>
>> Allow git-stash to save only the content of what's in staging area.
>> No working tree files (unstaged or untracked) will be added
>> to the stash nor will be modified then the opreration finishes.
>>
>
> Sorry for the double tag. This is not fully completed code. Wanted to
> know if the community thinks it's something worth developing and if
> I'm heading in the right direction in terms of the code that I'm
> hitting.

I am not sure if you explained how useful the "feature" being
proposed is, which is a very important skill to exercise to entice
readers to start reading and helping your code.

Why is it useful to be able to do this?  It is unclear at least to
me.

By the way, per "git help cli", the name for this new option that is
more in line with the rest of the system would be "--cached"; it
would tell Git to work only on the data in the index (as opposed to
the working tree files).
.
Edmundo Carmona Antoranz Feb. 13, 2020, 1:21 p.m. UTC | #3 
On Wed, Feb 12, 2020 at 11:04 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> I am not sure if you explained how useful the "feature" being
> proposed is, which is a very important skill to exercise to entice
> readers to start reading and helping your code.
>
> Why is it useful to be able to do this?  It is unclear at least to
> me.

Fair enough. It's ok to explain here, right? There's no simple way to stash only
what you have on index while retaining your working tree state.

I would do this to achieve it:
save what I have on the index somehow
$ git commit -m "will drop this revision later"

that revision I just created has what I want to stash, actually

$ git stash save -m "want to keep this on working tree when I finish"

my work tree is now clean (perhaps with untracked files)

$ git reset soft HEAD~1 # get what I had on index before

now I can stash... this is what I originally intended

$ git stash save -m "what I really meant to stash"

and now I need to get the working tree the way I had it when I started:

git stash pop stash@{1}


All of that with git stash push --index-only... well, git stash push --cached

>
> By the way, per "git help cli", the name for this new option that is
> more in line with the rest of the system would be "--cached"; it
> would tell Git to work only on the data in the index (as opposed to
> the working tree files).
> .
>
Junio C Hamano Feb. 13, 2020, 4:55 p.m. UTC | #4 
Edmundo Carmona Antoranz <eantoranz@gmail.com> writes:

> Fair enough. It's ok to explain here, right?

Yes, absolutely.   How it works is 

 - an author comes up with a patch and its explanation and
   justificaiton, 

 - reviewers ask clarifications on what was unclear, point out what
   was wrong in the code, etc., and then 

 - the author responds with "here is what I meant", "yeah, you're
   right that the code is wrong--would it be right to write it this
   way?", and/or "not really, the code is OK because ...", etc.

All that exchange is to come up with a more polished version that
has correct code that is explained and justified in a clear manner.

The next iteration the author then sends would of course include the
fixes to bugs pointed out during the review, but also would explain
and justify the change better---the explanation in the original did
not convey what the author wanted to tell, but the explanation the
author gave during the review discussion did so better, so the new
iteration can learn from that exchange.

> There's no simple way to stash only
> what you have on index while retaining your working tree state.

Yes, that much can be inferred from what it does ("there is no easy
way to do X, so I am adding X").

The question is why would one want to do X in the first place.

IIUC, the simplest workflow using the new feature may go like this:

	$ edit file ;# edit a bit
	$ git add file
	$ edit file ;# even even more
	$ git stash --cached

After all of the above is done, "git stash list" may show that there
is a single stash that records the changes you made to the file
right after you added it, without your further changes (because you
are taking what is in the index).  Your working tree has all the
changes you made to file, both before and after "git add", and your
index is clean.

After you got yourself into this state where your index is clean,
your working tree file has all changes, and your stash entry has
only the earlier half of the change, what are you going to do with
that stash entry?  If we learn the answer to that question, perhaps
we may find (or we may even have) a better or easier way to achieve
whatever you were planning to do with that stash entry by some other
means---it might not even involve "git stash"---but without knowing
that, we cannot tell if the new feature is a good idea.

Thanks.
Edmundo Carmona Antoranz Feb. 13, 2020, 5:12 p.m. UTC | #5 
On Thu, Feb 13, 2020 at 10:55 AM Junio C Hamano <gitster@pobox.com> wrote:
> IIUC, the simplest workflow using the new feature may go like this:
>
>         $ edit file ;# edit a bit
>         $ git add file
>         $ edit file ;# even even more
>         $ git stash --cached
>
> After all of the above is done, "git stash list" may show that there
> is a single stash that records the changes you made to the file
> right after you added it, without your further changes (because you
> are taking what is in the index).  Your working tree has all the
> changes you made to file, both before and after "git add", and your
> index is clean.
>
> After you got yourself into this state where your index is clean,
> your working tree file has all changes, and your stash entry has
> only the earlier half of the change, what are you going to do with
> that stash entry?  If we learn the answer to that question, perhaps
> we may find (or we may even have) a better or easier way to achieve
> whatever you were planning to do with that stash entry by some other
> means---it might not even involve "git stash"---but without knowing
> that, we cannot tell if the new feature is a good idea.

Actually, let this conversation burn in /dev/null. What I want to
achieve (stash only some changes and not all of them) can be done by
using git stash push providing pathspec.

>
> Thanks.

Thank you!
diff mbox series

Patch

diff --git a/builtin/stash.c b/builtin/stash.c
index 879fc5f368..d96b849965 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -27,9 +27,10 @@  static const char * const git_stash_usage[] = {
 	N_("git stash clear"),
 	N_("git stash [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n"
 	   "          [-u|--include-untracked] [-a|--all] [-m|--message <message>]\n"
-	   "          [--] [<pathspec>...]]"),
+	   "          [-i|--index-only] [--] [<pathspec>...]]"),
 	N_("git stash save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n"
-	   "          [-u|--include-untracked] [-a|--all] [<message>]"),
+	   "          [-u|--include-untracked] [-a|--all]\n"
+	   "          [-i|--index-only] [<message>]"),
 	NULL
 };
 
@@ -76,13 +77,14 @@  static const char * const git_stash_store_usage[] = {
 static const char * const git_stash_push_usage[] = {
 	N_("git stash [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n"
 	   "          [-u|--include-untracked] [-a|--all] [-m|--message <message>]\n"
-	   "          [--] [<pathspec>...]]"),
+	   "          [-i|--index-only] [--] [<pathspec>...]]"),
 	NULL
 };
 
 static const char * const git_stash_save_usage[] = {
 	N_("git stash save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n"
-	   "          [-u|--include-untracked] [-a|--all] [<message>]"),
+	   "          [-u|--include-untracked] [-i|--index-only] [-a|--all]\n"
+	   "          [<message>]"),
 	NULL
 };
 
@@ -1055,7 +1057,7 @@  static int stash_patch(struct stash_info *info, const struct pathspec *ps,
 	return ret;
 }
 
-static int stash_working_tree(struct stash_info *info, const struct pathspec *ps)
+static int stash_working_tree(struct stash_info *info, const struct pathspec *ps, int index_only)
 {
 	int ret = 0;
 	struct rev_info rev;
@@ -1089,17 +1091,20 @@  static int stash_working_tree(struct stash_info *info, const struct pathspec *ps
 		goto done;
 	}
 
-	cp_upd_index.git_cmd = 1;
-	argv_array_pushl(&cp_upd_index.args, "update-index",
-			 "--ignore-skip-worktree-entries",
-			 "-z", "--add", "--remove", "--stdin", NULL);
-	argv_array_pushf(&cp_upd_index.env_array, "GIT_INDEX_FILE=%s",
-			 stash_index_path.buf);
-
-	if (pipe_command(&cp_upd_index, diff_output.buf, diff_output.len,
-			 NULL, 0, NULL, 0)) {
-		ret = -1;
-		goto done;
+	if (!index_only) {
+		cp_upd_index.git_cmd = 1;
+		argv_array_pushl(&cp_upd_index.args, "update-index",
+				"--ignore-skip-worktree-entries",
+				"-z", "--add", "--remove", "--stdin", NULL);
+		argv_array_pushf(&cp_upd_index.env_array, "GIT_INDEX_FILE=%s",
+				stash_index_path.buf);
+
+		if (pipe_command(&cp_upd_index, diff_output.buf, diff_output.len,
+				NULL, 0, NULL, 0)) {
+			printf("salimos por pipe_command\n");
+			ret = -1;
+			goto done;
+		}
 	}
 
 	if (write_index_as_tree(&info->w_tree, &istate, stash_index_path.buf, 0,
@@ -1119,7 +1124,7 @@  static int stash_working_tree(struct stash_info *info, const struct pathspec *ps
 }
 
 static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_buf,
-			   int include_untracked, int patch_mode,
+			   int include_untracked, int index_only, int patch_mode,
 			   struct stash_info *info, struct strbuf *patch,
 			   int quiet)
 {
@@ -1199,7 +1204,7 @@  static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b
 			goto done;
 		}
 	} else {
-		if (stash_working_tree(info, ps)) {
+		if (stash_working_tree(info, ps, index_only)) {
 			if (!quiet)
 				fprintf_ln(stderr, _("Cannot save the current "
 						     "worktree state"));
@@ -1256,7 +1261,7 @@  static int create_stash(int argc, const char **argv, const char *prefix)
 	if (!check_changes_tracked_files(&ps))
 		return 0;
 
-	ret = do_create_stash(&ps, &stash_msg_buf, 0, 0, &info,
+	ret = do_create_stash(&ps, &stash_msg_buf, 0, 0, 0, &info,
 			      NULL, 0);
 	if (!ret)
 		printf_ln("%s", oid_to_hex(&info.w_commit));
@@ -1266,7 +1271,8 @@  static int create_stash(int argc, const char **argv, const char *prefix)
 }
 
 static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int quiet,
-			 int keep_index, int patch_mode, int include_untracked)
+			 int keep_index, int patch_mode, int include_untracked,
+			 int index_only)
 {
 	int ret = 0;
 	struct stash_info info;
@@ -1283,9 +1289,16 @@  static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q
 		ret = -1;
 		goto done;
 	}
+	
+	if (include_untracked && index_only) {
+		fprintf_ln(stderr, _("Can't use --index_only and --include-untracked"
+				     " at the same time")); // TODO how about --all?
+		ret = -1;
+		goto done;
+	}
 
 	read_cache_preload(NULL);
-	if (!include_untracked && ps->nr) {
+	if (!include_untracked && !index_only && ps->nr) {
 		int i;
 		char *ps_matched = xcalloc(ps->nr, 1);
 
@@ -1322,8 +1335,8 @@  static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q
 
 	if (stash_msg)
 		strbuf_addstr(&stash_msg_buf, stash_msg);
-	if (do_create_stash(ps, &stash_msg_buf, include_untracked, patch_mode,
-			    &info, &patch, quiet)) {
+	if (do_create_stash(ps, &stash_msg_buf, include_untracked, index_only,
+			    patch_mode, &info, &patch, quiet)) {
 		ret = -1;
 		goto done;
 	}
@@ -1335,9 +1348,14 @@  static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q
 		goto done;
 	}
 
-	if (!quiet)
-		printf_ln(_("Saved working directory and index state %s"),
-			  stash_msg_buf.buf);
+	if (!quiet) {
+		if (index_only)
+			printf_ln(_("Saved index state %s"),
+				stash_msg_buf.buf);
+		else
+			printf_ln(_("Saved working directory and index state %s"),
+				stash_msg_buf.buf);
+	}
 
 	if (!patch_mode) {
 		if (include_untracked && !ps->nr) {
@@ -1354,30 +1372,34 @@  static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q
 			}
 		}
 		discard_cache();
-		if (ps->nr) {
-			struct child_process cp_add = CHILD_PROCESS_INIT;
+		if (index_only || ps->nr) {
 			struct child_process cp_diff = CHILD_PROCESS_INIT;
 			struct child_process cp_apply = CHILD_PROCESS_INIT;
 			struct strbuf out = STRBUF_INIT;
 
-			cp_add.git_cmd = 1;
-			argv_array_push(&cp_add.args, "add");
-			if (!include_untracked)
-				argv_array_push(&cp_add.args, "-u");
-			if (include_untracked == INCLUDE_ALL_FILES)
-				argv_array_push(&cp_add.args, "--force");
-			argv_array_push(&cp_add.args, "--");
-			add_pathspecs(&cp_add.args, ps);
-			if (run_command(&cp_add)) {
-				ret = -1;
-				goto done;
+			if (!index_only) {
+				struct child_process cp_add = CHILD_PROCESS_INIT;
+
+				cp_add.git_cmd = 1;
+				argv_array_push(&cp_add.args, "add");
+				if (!include_untracked)
+					argv_array_push(&cp_add.args, "-u");
+				if (include_untracked == INCLUDE_ALL_FILES)
+					argv_array_push(&cp_add.args, "--force");
+				argv_array_push(&cp_add.args, "--");
+				add_pathspecs(&cp_add.args, ps);
+				if (run_command(&cp_add)) {
+					ret = -1;
+					goto done;
+				}
 			}
 
 			cp_diff.git_cmd = 1;
 			argv_array_pushl(&cp_diff.args, "diff-index", "-p",
 					 "--cached", "--binary", "HEAD", "--",
 					 NULL);
-			add_pathspecs(&cp_diff.args, ps);
+			if (!index_only)
+				add_pathspecs(&cp_diff.args, ps);
 			if (pipe_command(&cp_diff, NULL, 0, &out, 0, NULL, 0)) {
 				ret = -1;
 				goto done;
@@ -1391,7 +1413,7 @@  static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q
 				ret = -1;
 				goto done;
 			}
-		} else {
+		} else if (!index_only) {
 			struct child_process cp = CHILD_PROCESS_INIT;
 			cp.git_cmd = 1;
 			argv_array_pushl(&cp.args, "reset", "--hard", "-q",
@@ -1456,6 +1478,7 @@  static int push_stash(int argc, const char **argv, const char *prefix)
 	int keep_index = -1;
 	int patch_mode = 0;
 	int include_untracked = 0;
+	int index_only = 0;
 	int quiet = 0;
 	const char *stash_msg = NULL;
 	struct pathspec ps;
@@ -1467,6 +1490,8 @@  static int push_stash(int argc, const char **argv, const char *prefix)
 		OPT__QUIET(&quiet, N_("quiet mode")),
 		OPT_BOOL('u', "include-untracked", &include_untracked,
 			 N_("include untracked files in stash")),
+		OPT_BOOL('i', "index-only", &index_only,
+			 N_("stash only what is in staging area")),
 		OPT_SET_INT('a', "all", &include_untracked,
 			    N_("include ignore files"), 2),
 		OPT_STRING('m', "message", &stash_msg, N_("message"),
@@ -1482,7 +1507,7 @@  static int push_stash(int argc, const char **argv, const char *prefix)
 	parse_pathspec(&ps, 0, PATHSPEC_PREFER_FULL | PATHSPEC_PREFIX_ORIGIN,
 		       prefix, argv);
 	return do_push_stash(&ps, stash_msg, quiet, keep_index, patch_mode,
-			     include_untracked);
+			     include_untracked, index_only);
 }
 
 static int save_stash(int argc, const char **argv, const char *prefix)
@@ -1490,6 +1515,7 @@  static int save_stash(int argc, const char **argv, const char *prefix)
 	int keep_index = -1;
 	int patch_mode = 0;
 	int include_untracked = 0;
+	int index_only = 0;
 	int quiet = 0;
 	int ret = 0;
 	const char *stash_msg = NULL;
@@ -1503,6 +1529,8 @@  static int save_stash(int argc, const char **argv, const char *prefix)
 		OPT__QUIET(&quiet, N_("quiet mode")),
 		OPT_BOOL('u', "include-untracked", &include_untracked,
 			 N_("include untracked files in stash")),
+		OPT_BOOL('i', "index-only", &index_only,
+			 N_("stash only what is in staging area")),
 		OPT_SET_INT('a', "all", &include_untracked,
 			    N_("include ignore files"), 2),
 		OPT_STRING('m', "message", &stash_msg, "message",
@@ -1519,7 +1547,7 @@  static int save_stash(int argc, const char **argv, const char *prefix)
 
 	memset(&ps, 0, sizeof(ps));
 	ret = do_push_stash(&ps, stash_msg, quiet, keep_index,
-			    patch_mode, include_untracked);
+			    patch_mode, include_untracked, index_only);
 
 	strbuf_release(&stash_msg_buf);
 	return ret;
@@ -1625,6 +1653,7 @@  int cmd_stash(int argc, const char **argv, const char *prefix)
 			    !strcmp(argv[i], "--no-keep-index") ||
 			    !strcmp(argv[i], "--patch") ||
 			    !strcmp(argv[i], "--quiet") ||
+			    !strcmp(argv[i], "--index-only") ||
 			    !strcmp(argv[i], "--include-untracked"))
 				continue;