Re: [GSoC][PATCH v8 00/20] Convert "git stash" to C builtin
Hi Paul, On Fri, 31 Aug 2018, Paul-Sebastian Ungureanu wrote: > This a new iteration of `stash.c`. Thank you for the pleasant read! I read through all of the patches, spending particularly some time with the `stash create` one. Apart from the few comments I had, I only have positive things to say ;-) Thanks, Dscho
Re: [GSoC][PATCH v8 00/20] Convert "git stash" to C builtin
Paul-Sebastian Ungureanu writes:
> This a new iteration of `stash.c`. What is new?
>
> * Some commits got squashed. The commit related to replacing
> `git apply` child process was dropped since it wasn't the best
> idea.
>
> * In v7, there was a bug [1] related to config `git stash show`
> The bug was fixed and a test file was added for this.
>
> * Fixed `git stash [push]` [2]. In v7, `git stash -q drop` would
> act like `git stash push -q drop`.
>
> * Fixed coding-style nits. Verified that messages are marked
> for translation and are going to the correct output stream.
>
> * Fixed one memory leak (related to `strbuf_detach`).
>
> * Simplified the code a little bit.
Also worth noting.
* Rebased on a recent 'master', in which the calling convention
for functions like dir_path_match() has been updated. It won't
compile when applied to anything older than dc0f6f9e ("Merge
branch 'nd/no-the-index'", 2018-08-20).
Re: [GSoC][PATCH v8 00/20] Convert "git stash" to C builtin
On Thu, Aug 30 2018, Paul-Sebastian Ungureanu wrote:
> Hello,
>
> This a new iteration of `stash.c`. What is new?
>
> * Some commits got squashed. The commit related to replacing
> `git apply` child process was dropped since it wasn't the best
> idea.
>
> * In v7, there was a bug [1] related to config `git stash show`
> The bug was fixed and a test file was added for this.
>
> * Fixed `git stash [push]` [2]. In v7, `git stash -q drop` would
> act like `git stash push -q drop`.
>
> * Fixed coding-style nits. Verified that messages are marked
> for translation and are going to the correct output stream.
>
> * Fixed one memory leak (related to `strbuf_detach`).
>
> * Simplified the code a little bit.
This looks good, I read this and a previous version. I'e tested this on
Linux, FreeBSD & AIX. All tests pass on all of them.
One style nit: These patches consistently indent wrapped lines not to
align with the opening "(" (as is our usual style), but just with
"\n\t". I.e. this patch on top would make it like what we usually do
(but should be squashed as appropriate):
diff --git a/builtin/stash.c b/builtin/stash.c
index dd1084afd4..6d849ed811 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -389,3 +389,3 @@ static int restore_untracked(struct object_id *u_tree)
static int do_apply_stash(const char *prefix, struct stash_info *info,
- int index)
+ int index)
{
@@ -408,3 +408,3 @@ static int do_apply_stash(const char *prefix, struct
stash_info *info,
if (!oidcmp(>b_tree, >i_tree) || !oidcmp(_tree,
- >i_tree)) {
+
>i_tree)) {
has_index = 0;
@@ -514,3 +514,3 @@ static int apply_stash(int argc, const char **argv,
const char *prefix)
OPT_BOOL(0, "index", ,
- N_("attempt to recreate the index")),
+N_("attempt to recreate the index")),
OPT_END()
@@ -610,3 +610,3 @@ static int pop_stash(int argc, const char **argv, const
char *prefix)
OPT_BOOL(0, "index", ,
- N_("attempt to recreate the index")),
+N_("attempt to recreate the index")),
OPT_END()
@@ -971,3 +971,3 @@ static int stash_patch(struct stash_info *info, struct
pathspec ps, int quiet)
argv_array_pushl(_add_i.args, "add--interactive", "--patch=stash",
- "--", NULL);
+"--", NULL);
for (i = 0; i < ps.nr; ++i)
@@ -1279,3 +1279,3 @@ static int do_push_stash(struct pathspec ps, const
char *stash_msg, int quiet,
printf_ln(_("Saved working directory and index state %s"),
- stash_msg);
+ stash_msg);
@@ -1413,5 +1413,5 @@ static int push_stash(int argc, const char **argv,
const char *prefix)
OPT_SET_INT('k', "keep-index", _index,
- N_("keep index"), 1),
+ N_("keep index"), 1),
OPT_BOOL('p', "patch", _mode,
- N_("stash in patch mode")),
+N_("stash in patch mode")),
OPT__QUIET(, N_("quiet mode")),
@@ -1422,3 +1422,3 @@ static int push_stash(int argc, const char **argv,
const char *prefix)
OPT_STRING('m', "message", _msg, N_("message"),
-N_("stash message")),
+ N_("stash message")),
OPT_END()
@@ -1450,5 +1450,5 @@ static int save_stash(int argc, const char **argv,
const char *prefix)
OPT_SET_INT('k', "keep-index", _index,
- N_("keep index"), 1),
+ N_("keep index"), 1),
OPT_BOOL('p', "patch", _mode,
- N_("stash in patch mode")),
+N_("stash in patch mode")),
OPT__QUIET(, N_("quiet mode")),
Tip: It's also better to get feedback by CC-ing people who've had
feedback on previous versions.
[GSoC][PATCH v8 00/20] Convert "git stash" to C builtin
Hello, This a new iteration of `stash.c`. What is new? * Some commits got squashed. The commit related to replacing `git apply` child process was dropped since it wasn't the best idea. * In v7, there was a bug [1] related to config `git stash show` The bug was fixed and a test file was added for this. * Fixed `git stash [push]` [2]. In v7, `git stash -q drop` would act like `git stash push -q drop`. * Fixed coding-style nits. Verified that messages are marked for translation and are going to the correct output stream. * Fixed one memory leak (related to `strbuf_detach`). * Simplified the code a little bit. [1]: https://public-inbox.org/git/20180815210142.gn2...@hank.intra.tgummerer.com/ [2]: https://public-inbox.org/git/20180818165102.gf11...@hank.intra.tgummerer.com/ Best regards, Paul Joel Teichroeb (5): stash: improve option parsing test coverage stash: convert apply to builtin stash: convert drop and clear to builtin stash: convert branch to builtin stash: convert pop to builtin Paul-Sebastian Ungureanu (15): sha1-name.c: add `get_oidf()` which acts like `get_oid()` stash: update test cases conform to coding guidelines stash: rename test cases to be more descriptive stash: add tests for `git stash show` config stash: convert list to builtin stash: convert show to builtin stash: mention options in `show` synopsis. stash: convert store to builtin stash: convert create to builtin stash: convert push to builtin stash: make push -q quiet stash: convert save to builtin stash: convert `stash--helper.c` into `stash.c` stash: optimize `get_untracked_files()` and `check_changes()` stash: replace all `write-tree` child processes with API calls Documentation/git-stash.txt |4 +- Makefile |2 +- builtin.h|1 + builtin/stash.c | 1563 ++ cache.h |1 + git-stash.sh | 752 git.c|1 + sha1-name.c | 19 + t/t3903-stash.sh | 192 +++-- t/t3907-stash-show-config.sh | 81 ++ 10 files changed, 1795 insertions(+), 821 deletions(-) create mode 100644 builtin/stash.c delete mode 100755 git-stash.sh create mode 100755 t/t3907-stash-show-config.sh -- 2.19.0.rc0.22.gc26283d74e
