diff mbox series

[v2] stash: implement '--staged' option for 'push' and 'save'

Message ID 87fst2gwia.fsf_-_@osv.gnss.ru (mailing list archive)
State Superseded
Headers show
Series [v2] stash: implement '--staged' option for 'push' and 'save' | expand

Commit Message

Sergey Organov Oct. 15, 2021, 3:04 p.m. UTC
Stash only the changes that are staged.

This mode allows to easily stash-out for later reuse some changes
unrelated to the current work in progress.

Unlike 'stash push --patch', --staged supports use of any tool to
select the changes to stash-out, including, but not limited to 'git
add --interactive'.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---

This operation that is essentially just a 'git commit', but to the stash
rather than to the current branch, is somehow missed, complicating the
task that is otherwise simple and natural. For example, see discussions
here:

https://stackoverflow.com/questions/14759748/stashing-only-staged-changes-in-git-is-it-possible

Changes in v2:

  * Fixed English grammar in commit message

  * Fixed copy-paste error in help description

Changes in v1:

  * Implement separate stash_staged() instead of re-using and changing
    stash_patch()

  * Add test

  * Minor documentation cleanup

 Documentation/git-stash.txt | 34 ++++++++++++++--
 builtin/stash.c             | 80 ++++++++++++++++++++++++++++++++-----
 t/t3903-stash.sh            | 11 +++++
 3 files changed, 113 insertions(+), 12 deletions(-)

Comments

Junio C Hamano Oct. 15, 2021, 5:58 p.m. UTC | #1
Sergey Organov <sorganov@gmail.com> writes:

> @@ -205,6 +205,16 @@ to learn how to operate the `--patch` mode.
>  The `--patch` option implies `--keep-index`.  You can use
>  `--no-keep-index` to override this.
>  
> +-S::
> +--staged::
> +	This option is only valid for `push` and `save` commands.
> ++
> +Stash only the changes that are currently staged. This is similar to
> +basic `git commit` except the state is committed to the stash instead
> +of current branch.
> ++
> +The `--patch` option has priority over this one.
> +
>  --pathspec-from-file=<file>::
>  	This option is only valid for `push` command.
>  +
> @@ -341,6 +351,24 @@ $ edit/build/test remaining parts
>  $ git commit foo -m 'Remaining parts'
>  ----------------------------------------------------------------
>  
> +Saving unrelated changes for future use::
> +
> +When you are in the middle of massive changes and you find some
> +unrelated issue that you don't want to forget to fix, you can do the
> +change(s), stage them, and use `git stash push --staged` to stash them
> +out for future use. This is similar to committing the staged changes,
> +only the commit ends-up being in the stash and not on the current branch.
> ++
> +----------------------------------------------------------------
> +# ... hack hack hack ...
> +$ git add --patch foo           # add unrelated changes to the index
> +$ git stash push --staged       # save these changes to the stash
> +# ... hack hack hack, finish curent changes ...
> +$ git commit -m 'Massive'       # commit fully tested changes
> +$ git switch fixup-branch       # switch to another branch
> +$ git stash pop                 # to finish work on the saved changes
> +----------------------------------------------------------------
> +

The last step would more like "to start working on top of the saved
changes", I would think, as the user did not want to bother thinking
about them earlier while working on the other theme.  Otherwise, the
user would have done

    git checkout -b fixup-branch
    git add -p
    git commit -m '[WIP] mostly done' -e
    git checkout -

to remember that the fixup is not quite but mostly done and what was
done so far.

But I'd agree that the new mode would fit in such a workflow.


> +static int stash_staged(struct stash_info *info, const struct pathspec *ps,
> +		       struct strbuf *out_patch, int quiet)
> +{
> +	int ret = 0;
> +	struct child_process cp_diff_tree = CHILD_PROCESS_INIT;
> +	struct index_state istate = { NULL };
> +
> +	if (write_index_as_tree(&info->w_tree, &istate, the_repository->index_file,
> +				0, NULL)) {
> +		ret = -1;
> +		goto done;
> +	}

OK.  So what is currently in the index becomes the w-tree.

> +	cp_diff_tree.git_cmd = 1;
> +	strvec_pushl(&cp_diff_tree.args, "diff-tree", "-p", "-U1", "HEAD",
> +		     oid_to_hex(&info->w_tree), "--", NULL);
> +	if (pipe_command(&cp_diff_tree, NULL, 0, out_patch, 0, NULL, 0)) {
> +		ret = -1;
> +		goto done;
> +	}
> +
> +	if (!out_patch->len) {
> +		if (!quiet)
> +			fprintf_ln(stderr, _("No changes selected"));
> +		ret = 1;
> +	}

This seems to have been taken from the "stash_patch()" flow, but
unlike the "stash -p" that goes interactive to let the user pick
hunks, in which context "oh, no, you did not SELECT anything" makes
perfect sense as an error message, this message would be confusing
to users who weren't offered a chance to select.

> +done:
> +	discard_index(&istate);
> +	return ret;
> +}
> +

Also, as stash_staged() and stash_patch() are _so_ close, I suspect
that we might want to make a common helper out of the original
stash_patch() and make stash_patch() a thin wrapper around it in the
step #1 of the series, and then add stash_staged() as a second
caller to the common helper.  That might result in a cleaner end
result.  And optionally the "do we really need to spawn diff-tree"
optimization can be done on top after the dust settles [*].

    [Side note] As we already have the tree object for the stashed
    state, I wonder if it is overkill to run "diff-tree" here,
    though.  Wouldn't it be a matter of comparing that tree object
    name with the tree object name of the HEAD?  Something like

            get_oid_treeish("HEAD:", &head_tree);
            if (oideq(&head_tree, &info->w_tree)) {
                    "Nothing staged";
                    ret -1;
            }

    may be a good starting point, perhaps?

    But as I hinted above, such an optimization is outside the scope
    of this topic.  As long as we do not duplicate the code to spawn
    diff-tree from stash_patch(), but use a shared helper between
    the two codepath, such an optimization is easily doable later.

>  static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_buf,
> -			   int include_untracked, int patch_mode,
> +			   int include_untracked, int patch_mode, int only_staged,

Let's not keep adding a parameter to represent just a bit (or less).
Can't we at least treat this as an auxiliary bit in the "patch_mode"
flag?  The traditional patch_mode may be variant #1 while the one
that does the same thing but skips the interactive hunk selection
(i.e. only-staged) becomes the variant #2, or something?

> @@ -1321,6 +1353,16 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b
>  		} else if (ret > 0) {
>  			goto done;
>  		}
> +	} else if (only_staged) {
> +		ret = stash_staged(info, ps, patch, quiet);
> +		if (ret < 0) {
> +			if (!quiet)
> +				fprintf_ln(stderr, _("Cannot save the current "
> +						     "staged state"));
> +			goto done;
> +		} else if (ret > 0) {
> +			goto done;
> +		}

... which would reduce the need to add yet another else-if to this
cascade.

> @@ -1379,7 +1421,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);

and no need to touch this hunk.

> @@ -1389,7 +1431,7 @@ 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 only_staged)
>  {

nor this one.

I think I can agree with the motivation now (thanks for the
discussion); the code may want a bit more cleaning up.

Thanks.
Sergey Organov Oct. 15, 2021, 7:05 p.m. UTC | #2
Junio C Hamano <gitster@pobox.com> writes:

> Sergey Organov <sorganov@gmail.com> writes:
>
>> @@ -205,6 +205,16 @@ to learn how to operate the `--patch` mode.
>>  The `--patch` option implies `--keep-index`.  You can use
>>  `--no-keep-index` to override this.
>>  
>> +-S::
>> +--staged::
>> +	This option is only valid for `push` and `save` commands.
>> ++
>> +Stash only the changes that are currently staged. This is similar to
>> +basic `git commit` except the state is committed to the stash instead
>> +of current branch.
>> ++
>> +The `--patch` option has priority over this one.
>> +
>>  --pathspec-from-file=<file>::
>>  	This option is only valid for `push` command.
>>  +
>> @@ -341,6 +351,24 @@ $ edit/build/test remaining parts
>>  $ git commit foo -m 'Remaining parts'
>>  ----------------------------------------------------------------
>>  
>> +Saving unrelated changes for future use::
>> +
>> +When you are in the middle of massive changes and you find some
>> +unrelated issue that you don't want to forget to fix, you can do the
>> +change(s), stage them, and use `git stash push --staged` to stash them
>> +out for future use. This is similar to committing the staged changes,
>> +only the commit ends-up being in the stash and not on the current branch.
>> ++
>> +----------------------------------------------------------------
>> +# ... hack hack hack ...
>> +$ git add --patch foo           # add unrelated changes to the index
>> +$ git stash push --staged       # save these changes to the stash
>> +# ... hack hack hack, finish curent changes ...
>> +$ git commit -m 'Massive'       # commit fully tested changes
>> +$ git switch fixup-branch       # switch to another branch
>> +$ git stash pop                 # to finish work on the saved changes
>> +----------------------------------------------------------------
>> +
>
> The last step would more like "to start working on top of the saved
> changes", I would think, as the user did not want to bother thinking
> about them earlier while working on the other theme.  Otherwise, the
> user would have done
>
>     git checkout -b fixup-branch
>     git add -p
>     git commit -m '[WIP] mostly done' -e
>     git checkout -
>
> to remember that the fixup is not quite but mostly done and what was
> done so far.

Yep, that's why I said: "to finish work on the saved changes" in the
comments. If it's not clear enough, I'm open for suggestions for
clarifications.

>
> But I'd agree that the new mode would fit in such a workflow.
>
>
>> +static int stash_staged(struct stash_info *info, const struct pathspec *ps,
>> +		       struct strbuf *out_patch, int quiet)
>> +{
>> +	int ret = 0;
>> +	struct child_process cp_diff_tree = CHILD_PROCESS_INIT;
>> +	struct index_state istate = { NULL };
>> +
>> + if (write_index_as_tree(&info->w_tree, &istate,
>> the_repository->index_file,
>> +				0, NULL)) {
>> +		ret = -1;
>> +		goto done;
>> +	}
>
> OK.  So what is currently in the index becomes the w-tree.
>
>> +	cp_diff_tree.git_cmd = 1;
>> +	strvec_pushl(&cp_diff_tree.args, "diff-tree", "-p", "-U1", "HEAD",
>> +		     oid_to_hex(&info->w_tree), "--", NULL);
>> +	if (pipe_command(&cp_diff_tree, NULL, 0, out_patch, 0, NULL, 0)) {
>> +		ret = -1;
>> +		goto done;
>> +	}
>> +
>> +	if (!out_patch->len) {
>> +		if (!quiet)
>> +			fprintf_ln(stderr, _("No changes selected"));
>> +		ret = 1;
>> +	}
>
> This seems to have been taken from the "stash_patch()" flow,

Yep, exactly. I'm not familiar enough with the code to easily write it
by myself, so it's basically a copy-paste of stash_patch() and getting
rid of all the unneeded stuff.

> but
> unlike the "stash -p" that goes interactive to let the user pick
> hunks, in which context "oh, no, you did not SELECT anything" makes
> perfect sense as an error message, this message would be confusing
> to users who weren't offered a chance to select.

It seems to me that it makes sense to leave this warning as is, in case
the user invoked "stash --staged" without anything staged. I'm OK to
change this if you have something better in mind.

>
>> +done:
>> +	discard_index(&istate);
>> +	return ret;
>> +}
>> +
>
> Also, as stash_staged() and stash_patch() are _so_ close, I suspect
> that we might want to make a common helper out of the original
> stash_patch() and make stash_patch() a thin wrapper around it in the
> step #1 of the series, and then add stash_staged() as a second
> caller to the common helper.  That might result in a cleaner end
> result.  And optionally the "do we really need to spawn diff-tree"
> optimization can be done on top after the dust settles [*].

I do see a few of opportunities to improve code quality of "git stash",
even before this patch, but that is was not an aim of this patch.

What I aimed for is to keep all the existing code as intact as possible
to minimize probability of unintended breakage.

[...]

>>  static int do_create_stash(const struct pathspec *ps, struct strbuf
>> *stash_msg_buf,
>> -			   int include_untracked, int patch_mode,
>> + int include_untracked, int patch_mode, int only_staged,
>
> Let's not keep adding a parameter to represent just a bit (or less).
> Can't we at least treat this as an auxiliary bit in the "patch_mode"
> flag?  The traditional patch_mode may be variant #1 while the one
> that does the same thing but skips the interactive hunk selection
> (i.e. only-staged) becomes the variant #2, or something?

I don't want to mix --patch and --staged, as I believe --patch, after
introduction of --staged, becomes almost useless. One can instead use
"git add --interactive" directly and then run "stash --staged" on the
result.

That said, to get rid of these function arguments bloating, the code
should better be rewritten to use structure to hold all the parameters,
but that's again not the material of this patch.

>
>> @@ -1321,6 +1353,16 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b
>>  		} else if (ret > 0) {
>>  			goto done;
>>  		}
>> +	} else if (only_staged) {
>> +		ret = stash_staged(info, ps, patch, quiet);
>> +		if (ret < 0) {
>> +			if (!quiet)
>> +				fprintf_ln(stderr, _("Cannot save the current "
>> +						     "staged state"));
>> +			goto done;
>> +		} else if (ret > 0) {
>> +			goto done;
>> +		}
>
> ... which would reduce the need to add yet another else-if to this
> cascade.

These else-if cascades could *all* be reduced. The code does need actual
re-factoring, independently of this patch.

>
>> @@ -1379,7 +1421,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);
>
> and no need to touch this hunk.

Yep, if they all were in parameters structure in the first place, it'd
be unneeded.

>
>> @@ -1389,7 +1431,7 @@ 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
>> only_staged)
>>  {
>
> nor this one.

Yep, but that's the generic problem with the code. Mixing "only_staged"
into "patch_mode" would only worsen the code.

>
> I think I can agree with the motivation now (thanks for the
> discussion); the code may want a bit more cleaning up.

I do thing the code of "git stash" needs cleanup. Independently of the
patch. If somebody gets time to cleanup the original code, I'll happily
adapt the patch to the changes.

Otherwise, maybe I'll find time later to refactor the resulting code of
the "git stash" myself, but I'd like the patch to get its place first.

If you insist, I can provide a follow-up patch that factors-out minor
bit of common code from --patch and --staged, but I really don't want to
mix "only_staged" flag into the "patch_mode" flag, sorry.

Thanks,
-- Sergey Organov
Junio C Hamano Oct. 15, 2021, 7:22 p.m. UTC | #3
Sergey Organov <sorganov@gmail.com> writes:

>> but
>> unlike the "stash -p" that goes interactive to let the user pick
>> hunks, in which context "oh, no, you did not SELECT anything" makes
>> perfect sense as an error message, this message would be confusing
>> to users who weren't offered a chance to select.
>
> It seems to me that it makes sense to leave this warning as is, in case
> the user invoked "stash --staged" without anything staged. I'm OK to
> change this if you have something better in mind.

I am not questioning the presense of the warning.  It is just the
phrasing of the warning---"You have nothing staged" would make a
good message, but "You didn't select anything", when we do not offer
them a chance to select in the first place, would not work well.
Sergey Organov Oct. 15, 2021, 8:14 p.m. UTC | #4
Junio C Hamano <gitster@pobox.com> writes:

> Sergey Organov <sorganov@gmail.com> writes:
>
>>> but
>>> unlike the "stash -p" that goes interactive to let the user pick
>>> hunks, in which context "oh, no, you did not SELECT anything" makes
>>> perfect sense as an error message, this message would be confusing
>>> to users who weren't offered a chance to select.
>>
>> It seems to me that it makes sense to leave this warning as is, in case
>> the user invoked "stash --staged" without anything staged. I'm OK to
>> change this if you have something better in mind.
>
> I am not questioning the presense of the warning.  It is just the
> phrasing of the warning---"You have nothing staged" would make a
> good message, but "You didn't select anything", when we do not offer
> them a chance to select in the first place, would not work well.

Ah, I agree, your phrasing is much better, -- will fix.

Thanks,
-- Sergey Organov
Sergey Organov Oct. 15, 2021, 8:21 p.m. UTC | #5
Sergey Organov <sorganov@gmail.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Sergey Organov <sorganov@gmail.com> writes:
>>
>>>> but
>>>> unlike the "stash -p" that goes interactive to let the user pick
>>>> hunks, in which context "oh, no, you did not SELECT anything" makes
>>>> perfect sense as an error message, this message would be confusing
>>>> to users who weren't offered a chance to select.
>>>
>>> It seems to me that it makes sense to leave this warning as is, in case
>>> the user invoked "stash --staged" without anything staged. I'm OK to
>>> change this if you have something better in mind.
>>
>> I am not questioning the presense of the warning.  It is just the
>> phrasing of the warning---"You have nothing staged" would make a
>> good message, but "You didn't select anything", when we do not offer
>> them a chance to select in the first place, would not work well.
>
> Ah, I agree, your phrasing is much better, -- will fix.

Changed to "No staged changes" for the next re-roll. Looks OK?

Thanks,
-- Sergey Organov
diff mbox series

Patch

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index be6084ccefbe..6e15f4752576 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -13,7 +13,7 @@  SYNOPSIS
 'git stash' drop [-q|--quiet] [<stash>]
 'git stash' ( pop | apply ) [--index] [-q|--quiet] [<stash>]
 'git stash' branch <branchname> [<stash>]
-'git stash' [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
+'git stash' [push [-p|--patch] [-S|--staged] [-k|--[no-]keep-index] [-q|--quiet]
 	     [-u|--include-untracked] [-a|--all] [-m|--message <message>]
 	     [--pathspec-from-file=<file> [--pathspec-file-nul]]
 	     [--] [<pathspec>...]]
@@ -47,7 +47,7 @@  stash index (e.g. the integer `n` is equivalent to `stash@{n}`).
 COMMANDS
 --------
 
-push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [-m|--message <message>] [--pathspec-from-file=<file> [--pathspec-file-nul]] [--] [<pathspec>...]::
+push [-p|--patch] [-S|--staged] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [-m|--message <message>] [--pathspec-from-file=<file> [--pathspec-file-nul]] [--] [<pathspec>...]::
 
 	Save your local modifications to a new 'stash entry' and roll them
 	back to HEAD (in the working tree and in the index).
@@ -60,7 +60,7 @@  subcommand from making an unwanted stash entry.  The two exceptions to this
 are `stash -p` which acts as alias for `stash push -p` and pathspec elements,
 which are allowed after a double hyphen `--` for disambiguation.
 
-save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [<message>]::
+save [-p|--patch] [-S|--staged] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [<message>]::
 
 	This option is deprecated in favour of 'git stash push'.  It
 	differs from "stash push" in that it cannot take pathspec.
@@ -205,6 +205,16 @@  to learn how to operate the `--patch` mode.
 The `--patch` option implies `--keep-index`.  You can use
 `--no-keep-index` to override this.
 
+-S::
+--staged::
+	This option is only valid for `push` and `save` commands.
++
+Stash only the changes that are currently staged. This is similar to
+basic `git commit` except the state is committed to the stash instead
+of current branch.
++
+The `--patch` option has priority over this one.
+
 --pathspec-from-file=<file>::
 	This option is only valid for `push` command.
 +
@@ -341,6 +351,24 @@  $ edit/build/test remaining parts
 $ git commit foo -m 'Remaining parts'
 ----------------------------------------------------------------
 
+Saving unrelated changes for future use::
+
+When you are in the middle of massive changes and you find some
+unrelated issue that you don't want to forget to fix, you can do the
+change(s), stage them, and use `git stash push --staged` to stash them
+out for future use. This is similar to committing the staged changes,
+only the commit ends-up being in the stash and not on the current branch.
++
+----------------------------------------------------------------
+# ... hack hack hack ...
+$ git add --patch foo           # add unrelated changes to the index
+$ git stash push --staged       # save these changes to the stash
+# ... hack hack hack, finish curent changes ...
+$ git commit -m 'Massive'       # commit fully tested changes
+$ git switch fixup-branch       # switch to another branch
+$ git stash pop                 # to finish work on the saved changes
+----------------------------------------------------------------
+
 Recovering stash entries that were cleared/dropped erroneously::
 
 If you mistakenly drop or clear stash entries, they cannot be recovered
diff --git a/builtin/stash.c b/builtin/stash.c
index 8f42360ca913..cdc142f16602 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -27,11 +27,11 @@  static const char * const git_stash_usage[] = {
 	N_("git stash ( pop | apply ) [--index] [-q|--quiet] [<stash>]"),
 	N_("git stash branch <branchname> [<stash>]"),
 	"git stash clear",
-	N_("git stash [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n"
+	N_("git stash [push [-p|--patch] [-S|--staged] [-k|--[no-]keep-index] [-q|--quiet]\n"
 	   "          [-u|--include-untracked] [-a|--all] [-m|--message <message>]\n"
 	   "          [--pathspec-from-file=<file> [--pathspec-file-nul]]\n"
 	   "          [--] [<pathspec>...]]"),
-	N_("git stash save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n"
+	N_("git stash save [-p|--patch] [-S|--staged] [-k|--[no-]keep-index] [-q|--quiet]\n"
 	   "          [-u|--include-untracked] [-a|--all] [<message>]"),
 	NULL
 };
@@ -1116,6 +1116,38 @@  static int save_untracked_files(struct stash_info *info, struct strbuf *msg,
 	return ret;
 }
 
+static int stash_staged(struct stash_info *info, const struct pathspec *ps,
+		       struct strbuf *out_patch, int quiet)
+{
+	int ret = 0;
+	struct child_process cp_diff_tree = CHILD_PROCESS_INIT;
+	struct index_state istate = { NULL };
+
+	if (write_index_as_tree(&info->w_tree, &istate, the_repository->index_file,
+				0, NULL)) {
+		ret = -1;
+		goto done;
+	}
+
+	cp_diff_tree.git_cmd = 1;
+	strvec_pushl(&cp_diff_tree.args, "diff-tree", "-p", "-U1", "HEAD",
+		     oid_to_hex(&info->w_tree), "--", NULL);
+	if (pipe_command(&cp_diff_tree, NULL, 0, out_patch, 0, NULL, 0)) {
+		ret = -1;
+		goto done;
+	}
+
+	if (!out_patch->len) {
+		if (!quiet)
+			fprintf_ln(stderr, _("No changes selected"));
+		ret = 1;
+	}
+
+done:
+	discard_index(&istate);
+	return ret;
+}
+
 static int stash_patch(struct stash_info *info, const struct pathspec *ps,
 		       struct strbuf *out_patch, int quiet)
 {
@@ -1242,7 +1274,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 patch_mode, int only_staged,
 			   struct stash_info *info, struct strbuf *patch,
 			   int quiet)
 {
@@ -1321,6 +1353,16 @@  static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b
 		} else if (ret > 0) {
 			goto done;
 		}
+	} else if (only_staged) {
+		ret = stash_staged(info, ps, patch, quiet);
+		if (ret < 0) {
+			if (!quiet)
+				fprintf_ln(stderr, _("Cannot save the current "
+						     "staged state"));
+			goto done;
+		} else if (ret > 0) {
+			goto done;
+		}
 	} else {
 		if (stash_working_tree(info, ps)) {
 			if (!quiet)
@@ -1379,7 +1421,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));
@@ -1389,7 +1431,7 @@  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 only_staged)
 {
 	int ret = 0;
 	struct stash_info info;
@@ -1407,6 +1449,17 @@  static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q
 		goto done;
 	}
 
+	/* --patch overrides --staged */
+	if (patch_mode)
+		only_staged = 0;
+
+	if (only_staged && include_untracked) {
+		fprintf_ln(stderr, _("Can't use --staged and --include-untracked"
+				     " or --all at the same time"));
+		ret = -1;
+		goto done;
+	}
+
 	read_cache_preload(NULL);
 	if (!include_untracked && ps->nr) {
 		int i;
@@ -1447,7 +1500,7 @@  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,
+	if (do_create_stash(ps, &stash_msg_buf, include_untracked, patch_mode, only_staged,
 			    &info, &patch, quiet)) {
 		ret = -1;
 		goto done;
@@ -1464,7 +1517,7 @@  static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q
 		printf_ln(_("Saved working directory and index state %s"),
 			  stash_msg_buf.buf);
 
-	if (!patch_mode) {
+	if (!(patch_mode || only_staged)) {
 		if (include_untracked && !ps->nr) {
 			struct child_process cp = CHILD_PROCESS_INIT;
 
@@ -1581,6 +1634,7 @@  static int push_stash(int argc, const char **argv, const char *prefix,
 {
 	int force_assume = 0;
 	int keep_index = -1;
+	int only_staged = 0;
 	int patch_mode = 0;
 	int include_untracked = 0;
 	int quiet = 0;
@@ -1591,6 +1645,8 @@  static int push_stash(int argc, const char **argv, const char *prefix,
 	struct option options[] = {
 		OPT_BOOL('k', "keep-index", &keep_index,
 			 N_("keep index")),
+		OPT_BOOL('S', "staged", &only_staged,
+			 N_("stash staged changes only")),
 		OPT_BOOL('p', "patch", &patch_mode,
 			 N_("stash in patch mode")),
 		OPT__QUIET(&quiet, N_("quiet mode")),
@@ -1629,6 +1685,9 @@  static int push_stash(int argc, const char **argv, const char *prefix,
 		if (patch_mode)
 			die(_("--pathspec-from-file is incompatible with --patch"));
 
+		if (only_staged)
+			die(_("--pathspec-from-file is incompatible with --staged"));
+
 		if (ps.nr)
 			die(_("--pathspec-from-file is incompatible with pathspec arguments"));
 
@@ -1640,12 +1699,13 @@  static int push_stash(int argc, const char **argv, const char *prefix,
 	}
 
 	return do_push_stash(&ps, stash_msg, quiet, keep_index, patch_mode,
-			     include_untracked);
+			     include_untracked, only_staged);
 }
 
 static int save_stash(int argc, const char **argv, const char *prefix)
 {
 	int keep_index = -1;
+	int only_staged = 0;
 	int patch_mode = 0;
 	int include_untracked = 0;
 	int quiet = 0;
@@ -1656,6 +1716,8 @@  static int save_stash(int argc, const char **argv, const char *prefix)
 	struct option options[] = {
 		OPT_BOOL('k', "keep-index", &keep_index,
 			 N_("keep index")),
+		OPT_BOOL('S', "staged", &only_staged,
+			 N_("stash staged changes only")),
 		OPT_BOOL('p', "patch", &patch_mode,
 			 N_("stash in patch mode")),
 		OPT__QUIET(&quiet, N_("quiet mode")),
@@ -1677,7 +1739,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, only_staged);
 
 	strbuf_release(&stash_msg_buf);
 	return ret;
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 873aa56e359d..18ea885412b8 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -288,6 +288,17 @@  test_expect_success 'stash --no-keep-index' '
 	test bar,bar2 = $(cat file),$(cat file2)
 '
 
+test_expect_success 'stash --staged' '
+	echo bar3 >file &&
+	echo bar4 >file2 &&
+	git add file2 &&
+	git stash --staged &&
+	test bar3,bar2 = $(cat file),$(cat file2) &&
+	git reset --hard &&
+	git stash pop &&
+	test bar,bar4 = $(cat file),$(cat file2)
+'
+
 test_expect_success 'dont assume push with non-option args' '
 	test_must_fail git stash -q drop 2>err &&
 	test_i18ngrep -e "subcommand wasn'\''t specified; '\''push'\'' can'\''t be assumed due to unexpected token '\''drop'\''" err