Re: [PATCH v2 2/6] builtin/am: make sure state files are text

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 25, 2015, 18:41 UTC
Message-ID: <xmqq8u8zyzvk.fsf@gitster.dls.corp.google.com>
In-Reply-To: <20150825164701.GA10060@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 17 quoted lines

> On Tue, Aug 25, 2015 at 09:19:13AM -0700, Junio C Hamano wrote:
>
>> As to "flags exposed to callers" vs "with and without gently", when
>> we change the system to allow new modes of operations (e.g. somebody
>> wants to write a binary file, or allocate more flag bits for their
>> special case), I'd expect that we'd add a more general and verbose
>> "write_file_with_options(path, flags, fmt, ...)"), gain experience
>> with that function, and then possibly introduce canned thin wrappers
>> (e.g. write_binary_file() that is a synonym to passing BINARY but
>> not GENTLY) if the new thing proves widely useful, just like I left
>> write_file() and write_file_gently() in as fairly common things to
>> do.
>
> Yeah, that works. It is a bit of a gamble to me. If we never add a lot
> more options, the end result is much nicer (callers do not deal with the
> flag option at all). But if we do, we end up with the mess that
> get_sha1_with_* and add_pending_object() got into.

Yeah. I do not know. Perhaps a good intermim solution for now would be to make

  - write_file_l(path, flags, fmt, ...);

the low-level helper, with a single convenience wrapper:

  - write_file(path, fmt, ...)

for everybody other than two "gently" ones to use. Two "gently" ones can call write_file_l(path, WRITE_FILE_GENTLY, fmt,...).

You are right about binary stuff. You could do fmt="...%c..." and pass '\0' to corresponding place if your NUL is in the fixed part of the data you are generating, but otherwise write_file() interface is a very useful way to handle binary.