Re: [PATCH RFC v1] stash: implement '--staged' option for 'push' and 'save'

4 messages, 3 authors, 2021-10-12 · open the first message on its own page

Re: [PATCH RFC v1] stash: implement '--staged' option for 'push' and 'save'

From: Sergey Organov <hidden>
Date: 2021-10-11 21:55:38

Eric Sunshine [off-list ref] writes:
/On Mon, Oct 11, 2021 at 4:17 PM Sergey Organov [off-list ref] wrote:
quoted
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 using of any tool to
select the changes to stash-out, including, but not limited to 'git
add --interactive'.
s/using of any/use of any/
...or...
s/using of any/using any/
Will fix, thanks!
quoted
Signed-off-by: Sergey Organov <redacted>
---
diff --git a/builtin/stash.c b/builtin/stash.c
@@ -1656,6 +1716,8 @@ static int save_stash(int argc, const char **argv, const char *prefix)
+               OPT_BOOL('S', "staged", &only_staged,
+                        N_("stash in patch mode")),
                OPT_BOOL('p', "patch", &patch_mode,
                         N_("stash in patch mode")),
                OPT__QUIET(&quiet, N_("quiet mode")),
Copy/paste error in new help/description string?
Yep.

Thanks,

-- Sergey Organov

Re: [PATCH RFC v1] stash: implement '--staged' option for 'push' and 'save'

From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2021-10-12 09:25:10

On Tue, Oct 12 2021, Sergey Organov wrote:
Eric Sunshine [off-list ref] writes:
quoted
/On Mon, Oct 11, 2021 at 4:17 PM Sergey Organov [off-list ref] wrote:
quoted
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 using of any tool to
select the changes to stash-out, including, but not limited to 'git
add --interactive'.
s/using of any/use of any/
...or...
s/using of any/using any/
Will fix, thanks!
quoted
quoted
Signed-off-by: Sergey Organov <redacted>
---
diff --git a/builtin/stash.c b/builtin/stash.c
@@ -1656,6 +1716,8 @@ static int save_stash(int argc, const char **argv, const char *prefix)
+               OPT_BOOL('S', "staged", &only_staged,
+                        N_("stash in patch mode")),
                OPT_BOOL('p', "patch", &patch_mode,
                         N_("stash in patch mode")),
                OPT__QUIET(&quiet, N_("quiet mode")),
Copy/paste error in new help/description string?
Yep.

Thanks,
I very much like this option, I've sometimes missed it in "git stash",
and was always going to dig into if there was some way to do it.

The one thing I'm a bit iffy on is if this is consistent with the
--staged options in other commands (with some taking --cached and/or
--staged), I think so, and this is a good name.

But is the -S option used as a shorthand for --staged somewhere else?
*Checks*, ah yes, for "git restore", then we use "stage" for
checkout-index/ls-files, the latter of which has a 's' (not
capital-letter 'S') shorthand.

I *think* that just -s/--stage would make more sense here, but I've only
looked at it briefly, but getting options consistent if possible is in
general quite nice for users, so we should think about it...

Re: [PATCH RFC v1] stash: implement '--staged' option for 'push' and 'save'

From: Junio C Hamano <hidden>
Date: 2021-10-12 12:04:35

Ævar Arnfjörð Bjarmason [off-list ref] writes:
The one thing I'm a bit iffy on is if this is consistent with the
--staged options in other commands (with some taking --cached and/or
--staged), I think so, and this is a good name.
We clearly define contrasts between "--cached" and "--index", but
the "--staged", which is a confusing synonym for nothing, does not
get in the contrast between the two, so I do not think you need to
worry about "which one is it?" in this case.

If something works only on the contents in the index, then it should
use "--cached".  If it works both on the index and the working tree,
then it should use "--index".  If you call it "--staged", it is
whatever it means ;-)

More importantly...

Whenever I think about a new "feature", I try to come up with a
story in which the feature effectively improves the end-user's life,
how it fits in the larger picture, and enables something that is
hard to do by combining other tools.

The kind of "story" I would aim for is like this.  Suppose we were
selling not "git stash -S" but "git stash -k".  The story would go
like this:

  Imagine that the change you have been working on started to take
  shape, and you estimate it would be a three-patch series in the
  end.  You also guess that so far you have enough to finish the
  first step fully, perhaps 40% of the second step and a little bit
  of the third step, all mixed together.

  You started to sifting the changes into the first step material
  and the rest, by using "add -p" etc., and you are reasonably sure
  that what you have in the index is in a good shape for the first
  commit.  But it is not easy to be sure, because what you can test
  is only in the working tree, so a mistake like having all the code
  already added in the index but forgetting to add to the index a
  declaration for a variable the code uses that is in the working
  tree is easy to make.  With "git stash -k", you can materialize
  only what is in the index to the working tree, while stashing away
  the changes in the working tree that haven't been added yet.  By
  checking the resulting working tree, you can be sure.

  - If the resulting working tree after "git stash -k" tests out
    OK, then you can make a commit and the "git stash pop" will
    give you the material for the second and the third step.
    You'd work to produce the remaining 60% of the second step and
    do "git stash -k" dance again before recording it.

  - It is possible that the resulting working tree does not work
    OK.  You may find that you forgot to "add -p" the declaration
    of a variable you used in the code that you already "add"ed.
    After "stash -k", the former is stashed away while the latter
    appears in the working tree, and the compiler complains.  In
    such a case, you can "git commit" the slightly broken state,
    "git stash pop" to recover the missed declaration, together
    with the material for the second and third step, into the
    working tree, use "add -p" and "stash -k" to prepare and
    verify the "fixup" commit for the first step.  Later you can
    "rebase -i" the first step into shape.

I unfortunately am coming up empty for "git stash -S".  And I do not
see a beginning of a good story at the stackoverflow entry you had
the URL for, either.  If we had one to support this feature, that
would help very much.

While I failed to come up with a good story for this new feature, I
however did come up with possible confusion and frustration that
end-users may feel while trying to use it:

 - I thought the result of "git add -p" was good, so I did "git
   stash -S", then after working further on, did another "git stash
   -S" a few times.  Now I have a handful of stash entries, but
   because all I can do is "git stash pop" them one by one,

   - I need to make commits for real, and 

   - because I wasn't given a chance to, these stash entries do not
     record material to write good log messages and I forgot why I
     did some of the changes in the way I did so.

   - My "add -p" seem to have missed some stuff that should have
     been added, but it is too late to correct, especially given
     that these stash entries cannot be "rebase -i"ed or "commit
     --amend"ed.

 - Also, how would I reorder these steps?  If I made real commits,
   instead of "stash -S", I am familiar with "rebase -i" to reorder,
   combine or split them, but because these are not real commits, I
   cannot use "rebase -i".

 - After making these "stash -S" entries, I popped a wrong one.  If
   I recorded them as real commits on a temporary work branch, its
   reflog would have helped me to recover from such a mistake, but
   because stash does not mix well with reflog, I am lost.

And I do not want to see us respond to these future end-user gripes
with "don't worry, we'll extend 'git stash save [-S]' with the '-e'
option to let you describe the change in detail, and enhance 'git
stash pop' with the '--commit' option to directly create a commit
using the message you wrote when you created the stash", or "don't
worry, we'll enhance 'rebase -i' to be capable of working on series
of stash entries".  These all look like complexity that only became
necessary because we added "git stash -S"---if the user committed
incrementally on a temporary work branch, none of the complication
would have been needed.

I think it is very possible that an answer to the above possible
end-user gripes is "no, this feature is not about sifting a big and
mixed changes in the working tree into multiple steps recorded in
the stash entries (instead of a series of commits on a branch), so
all the above end-user gripes are the result of using the tool for a
wrong job", and that is why I wanted to come up with a story in
which this feature effectively improves the end-user experience.
IOW, with the "frustration" story in the previous paragraph, I might
have been trying to drive screws into wood with this new feature,
which is a hammer and not a screwdriver.  If that is the case, then
I would want to see a story where the hammer is used to drive nails
instead, and nails do something good that screws don't.

I have a suspicion that this _might_ be coming from a hesitancy to
commit (e.g. a draconian commit hook that always pushes things out
immediately a commit is made), and somehow creating stashes is used
as a way to sidestep the real source of the problem (e.g. in Git,
commits on temporary branches are designed to be useful lightweight
tools to help advance the history recorded in the real branch, but
misguided hooks and policies prevent your branches to be used as
such).  I dunno.

Re: [PATCH RFC v1] stash: implement '--staged' option for 'push' and 'save'

From: Junio C Hamano <hidden>
Date: 2021-10-12 12:34:23

Junio C Hamano [off-list ref] writes:
More importantly...

Whenever I think about a new "feature", I try to come up with a
story in which the feature effectively improves the end-user's life,
how it fits in the larger picture, and enables something that is
hard to do by combining other tools.

The kind of "story" I would aim for is like this.  Suppose we were
selling not "git stash -S" but "git stash -k". ...
To answer my previous "question", I guess this is usable in the same
scenario where "git stash -k" is useful.  After creating a bunch of
stash entries created by "git stash -S", if you want to test any of
them (because what is in these stash entries did not exist without
other working tree changes, and couldn't have been tested in the
working tree standalone by definition), you can "git stash pop" such
a stash entry created by "git stash -S" and then "git stash -k" to
materialize what was in the stash alone in the working tree to test
_later_ (as opposed to testing _first_; in the "git stash -k"
workflow, you'd collect "good bits" in the index with "add -p"
first, then "clear the remaining cruft" with "git stash -k" to test
it first, and take the cruft back with "git stash pop").

So in short, I do not think I am strongly opposed to "git stash -S"
existing, since I did find one use case story that it could be used,
but I do think it is redundant and unnecessary.

IOW, "git stash -k" followed by "git stash" and "git stash pop" the
one created with "git stash -k" would be an equivalent operation to
this new "git stash -S".  But the price of being able to combine
these three operations into one is that the user cannot have the
state after "stash -k" in the working tree to inspect, and I cannot
shake the feeling that this new "feature" is like a tail wagging a
dog.  If the "goal" is to "create a stash entry out of what is in
the index", then "stash -S" is a one-step handy tool that directly
achieves that "goal", but that "goal" does not smell like a useful
"goal" in the first place.  To "create a commit by sifting mixed
changes in the working tree with 'add -p' and then gaining a chance
to do a clean and final testing" would be the "goal" of "stash -k",
and that I can see a clear benefit.  Contrasting to that, I am not
so sure about "stash -S".  It would be another way to eventually do
the same thing but along a more roundabout route.

So, I dunno.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help