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

2015-08-24 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 the right

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

2015-08-24 Thread Junio C Hamano
Jeff King p...@peff.net 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

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 Junio C Hamano
Paul Tan pyoka...@gmail.com 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,

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

2015-08-23 Thread SZEDER Gábor
Hi, Quoting Paul Tan pyoka...@gmail.com: 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

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() is a short-hand for I

[PATCH] am: terminate state files with a newline

2015-08-22 Thread Paul Tan
, 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 git-am.sh in 783d7e8 (builtin-am: remove redirection