Re: [PATCH v13 00/27] Convert "git stash" to C builtin
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Feb 26, 2019, 12:40 UTC
- Message-ID
- <nycvar.QRO.7.76.6.1902261330560.41@tvgsbejvaqbjf.bet>
- In-Reply-To
- <20190225231631.30507-1-t.gummerer@gmail.com>
Hi Thomas,
On Mon, 25 Feb 2019, Thomas Gummerer wrote:
> As I was advocating for this series to go into 'next' without a large > refactor of this series, I'll put my money were my mouth is and try to > make the cleanups and fixes required, though without trying to avoid > further external process calls, or changing the series around too much.
Thank you.
Show 27 quoted lines
> range-diff below:
>
> [...]
> 9: f6bbd78127 ! 10: 45670448e8 stash: convert apply to builtin
> @@ -17,7 +17,7 @@
>
> Signed-off-by: Joel Teichroeb <joel@teichroeb.net>
> Signed-off-by: Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com>
> - Signed-off-by: Junio C Hamano <gitster@pobox.com>
> + Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
>
> diff --git a/.gitignore b/.gitignore
> --- a/.gitignore
> @@ -96,7 +96,6 @@
> + * i_tree is set to the index tree
> + * u_tree is set to the untracked files tree
> + */
> -+
> +struct stash_info {
> + struct object_id w_commit;
> + struct object_id b_commit;
> @@ -357,7 +356,7 @@
> + if (refresh_cache(REFRESH_QUIET))
> + return -1;
> +
> -+ if (write_cache_as_tree(&c_tree, 0, NULL) || reset_tree(&c_tree, 0, 0))
> ++ if (write_cache_as_tree(&c_tree, 0, NULL))Thank you for picking this up.
Show 23 quoted lines
> + return error(_("cannot apply a stash in the middle of a merge"));
> +
> + if (index) {
>
> [...]
> 22: 51809c70ca ! 23: 56a5ce2aeb stash: convert `stash--helper.c` into `stash.c`
> @@ -13,7 +13,7 @@
> called directly and not by a shell script.
>
> Signed-off-by: Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com>
> - Signed-off-by: Junio C Hamano <gitster@pobox.com>
> + Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
>
> diff --git a/.gitignore b/.gitignore
> --- a/.gitignore
> @@ -273,9 +273,11 @@
> return 0;
>
> - strbuf_addstr(&stash_msg_buf, stash_msg);
> - if (!(ret = do_create_stash(ps, &stash_msg_buf, 0, 0, &info, NULL, 0)))
> +- ret = do_create_stash(ps, &stash_msg_buf, include_untracked, 0, &info,
> ++ ret = do_create_stash(ps, &stash_msg_buf, 0, 0, &info,
> + NULL, 0);I do not quite understand this change, though. The end result seems to look similar enough to the previous iteration (except for the kept line wrapping which I would have undone in this patch), but how come that the `include_untracked` crept into some earlier patch in this iteration?
The rest was obvious enough, thank you.
As I said before, I was very much in favor of getting this `stash-in-c` business moving again by advancing it to `next`.
From my perspective (which is not informed by any official statement about the role of `pu` or `next` because there is none as far as I know), the purpose of `pu` is to get patches into a shape where the original author is no longer the only one working on them. In my mind, that moment has long passed, and the fact that `ps/stash-in-c` is still stuck in `pu` really held me back on working more on this.
So if you manage to get this finally cooking again, all hail to you!
Thanks, Dscho