Re: [PATCH v3 4/4] stash: implement builtin stash

2017-05-31 Thread Jeff King
On Wed, May 31, 2017 at 08:29:43PM -0700, Joel Teichroeb wrote: > I'm running into a lot of trouble using argv_array_clear. It seems > that some of the builtin git cmd functions move the parameters around, > and write new pointers to argv. There's three options I have now, and > I'm not sure

Re: [PATCH v3 4/4] stash: implement builtin stash

2017-05-31 Thread Joel Teichroeb
I'm running into a lot of trouble using argv_array_clear. It seems that some of the builtin git cmd functions move the parameters around, and write new pointers to argv. There's three options I have now, and I'm not sure which is the best one. 1. Fix all the builtin cmd functions that I use to

Re: [PATCH v3 4/4] stash: implement builtin stash

2017-05-29 Thread Ævar Arnfjörð Bjarmason
On Mon, May 29, 2017 at 8:18 PM, Joel Teichroeb wrote: > Once I have all those leaks fixed, is there a way to make sure I'm not > missing any? I tried using valgrind with leak-check enabled, but there > are too many leaks from other git commands. I just used: valgrind

Re: [PATCH v3 4/4] stash: implement builtin stash

2017-05-29 Thread Joel Teichroeb
Once I have all those leaks fixed, is there a way to make sure I'm not missing any? I tried using valgrind with leak-check enabled, but there are too many leaks from other git commands. Joel

Re: [PATCH v3 4/4] stash: implement builtin stash

2017-05-29 Thread Junio C Hamano
Joel Teichroeb writes: > +int untracked_files(struct strbuf *out, int include_untracked, > + const char **argv) Does this need to be public? For a caller that wants to learn if there is _any_ untracked file, having a strbuf that holds all output is overkill.

Re: [PATCH v3 4/4] stash: implement builtin stash

2017-05-28 Thread Jeff King
On Sun, May 28, 2017 at 11:31:48AM -0700, Joel Teichroeb wrote: > >> + /* TODO: Improve this logic */ > >> + strbuf_addf(, "%s", REV); > >> + str = strstr(symbolic.buf, "@"); > > > > Could you elaborate on how this should be improved? > > I just figured there would be a builtin

Re: [PATCH v3 4/4] stash: implement builtin stash

2017-05-28 Thread Jeff King
On Sun, May 28, 2017 at 08:51:07PM +0200, Ævar Arnfjörð Bjarmason wrote: > On Sun, May 28, 2017 at 6:56 PM, Joel Teichroeb wrote: > > Implement all git stash functionality as a builtin command > > > > Signed-off-by: Joel Teichroeb > > --- > > General

Re: [PATCH v3 4/4] stash: implement builtin stash

2017-05-28 Thread Ævar Arnfjörð Bjarmason
On Sun, May 28, 2017 at 6:56 PM, Joel Teichroeb wrote: > Implement all git stash functionality as a builtin command > > Signed-off-by: Joel Teichroeb > --- General note on this that I missed in my first E-Mail, you have ~20 calls to argv_array_init() but

Re: [PATCH v3 4/4] stash: implement builtin stash

2017-05-28 Thread Joel Teichroeb
On Sun, May 28, 2017 at 11:26 AM, Ævar Arnfjörð Bjarmason wrote: > On Sun, May 28, 2017 at 6:56 PM, Joel Teichroeb wrote: >> Implement all git stash functionality as a builtin command > > First thanks for working on this, it's great. Applied it locally, >

Re: [PATCH v3 4/4] stash: implement builtin stash

2017-05-28 Thread Ævar Arnfjörð Bjarmason
On Sun, May 28, 2017 at 6:56 PM, Joel Teichroeb wrote: > Implement all git stash functionality as a builtin command First thanks for working on this, it's great. Applied it locally, passes all tests for me. A couple of comments Christian didn't cover > + info->has_u =

Re: [PATCH v3 4/4] stash: implement builtin stash

2017-05-28 Thread Christian Couder
On Sun, May 28, 2017 at 6:56 PM, Joel Teichroeb wrote: [...] > +int untracked_files(struct strbuf *out, int include_untracked, > + const char **argv) > +{ > + struct child_process cp = CHILD_PROCESS_INIT; > + cp.git_cmd = 1; > +

[PATCH v3 4/4] stash: implement builtin stash

2017-05-28 Thread Joel Teichroeb
Implement all git stash functionality as a builtin command Signed-off-by: Joel Teichroeb --- Makefile |2 +- builtin.h |1 + builtin/stash.c | 1280