Re: [PATCH] am: terminate state files with a newline

2015-08-24 Thread brian m. carlson
On Sun, Aug 23, 2015 at 01:50:53PM +0800, Paul Tan wrote: > Did we ever explictly allow external programs to poke around the > contents of the .git/rebase-apply directory? I think it may not be so > good, as it means that it may not be possible to switch the storage > format in the future (e.g. to

Re: [PATCH] am: terminate state files with a newline

2015-08-23 Thread Jeff King
On Sun, Aug 23, 2015 at 11:48:44PM -0700, Junio C Hamano wrote: > > I think the "if" here is redundant; strbuf_complete_line already handles > > it. > > True. And I like your write_state_bool() wrapper (which should be > "static void" to the builtin/am.c) very much. > > On top of that, I think

Re: [PATCH] am: terminate state files with a newline

2015-08-23 Thread Junio C Hamano
Jeff King writes: > FWIW, I had a similar thought when reading the original thread. I also > noted that all of the callers here pass "1" for the "fatal" parameter, > and that they are either bools or single strings. I wonder if: > > void write_state_bool(struct am_state *state, const char *name

Re: [PATCH] am: terminate state files with a newline

2015-08-23 Thread Jeff King
On Sun, Aug 23, 2015 at 12:05:32PM -0700, Junio C Hamano wrote: > > - write_file(am_path(state, "threeway"), 1, state->threeway ? "t" : "f"); > > + write_file(am_path(state, "threeway"), 1, "%s\n", state->threeway ? "t" > > : "f"); > > Stepping back a bit, after realizing that "write_file()"

Re: [PATCH] am: terminate state files with a newline

2015-08-23 Thread Junio C Hamano
Paul Tan writes: > Did we ever explictly allow external programs to poke around the > contents of the .git/rebase-apply directory? We tell users to take a peek into it when "am" fails, don't we, by naming $GIT_DIR/rebase-apply/patch? > - write_file(am_path(state, "threeway"), 1, state->thre

Re: [PATCH] am: terminate state files with a newline

2015-08-23 Thread SZEDER Gábor
Hi, Quoting Paul Tan : Did we ever explictly allow external programs to poke around the contents of the .git/rebase-apply directory? I think it may not be so good, as it means that it may not be possible to switch the storage format in the future (e.g. to allow atomic modifications, maybe?) :-/

[PATCH] am: terminate state files with a newline

2015-08-22 Thread Paul Tan
se-apply directory? I think it may not be so good, as it means that it may not be possible to switch the storage format in the future (e.g. to allow atomic modifications, maybe?) :-/ . Regards, Paul -- >8 -- Subject: [PATCH] am: terminate state files with a newline Since builtin/am.c replaced