From: David Aguilar <davvid@gmail.com>
Date: Thu, 28 May 2009 21:30:49 GMT
Subject: Re: [PATCH v2 2/2] diff: generate prettier filenames when using GIT_EXTERNAL_DIFF
Message-ID: <20090528213049.GA55167@gmail.com>
In-Reply-To: <20090528174436.GA12723@coredump.intra.peff.net>

On Thu, May 28, 2009 at 01:44:36PM -0400, Jeff King wrote:
> On Wed, May 27, 2009 at 11:11:17PM -0700, David Aguilar wrote:
> 
> > +int git_mkstemps(char *path, size_t n, const char *template, int suffix_len);
> 
> FWIW, I find this name not very descriptive. From the name, I would
> expect it to do the exact same thing as mkstemps, but be our own
> personal implementation. But it is actually a wrapper that behaves
> somewhat differently. So I wonder if "mkstemps_tmpdir" or something
> would be a better name.

It does exactly what git_mkstemp() does, plus the extra
suffix_len parameter.  If we rename this function we have to
rename both.


> Is there a reason _not_ to always just use the pretty filename? It looks
> like you turn it on for external diff, but off for textconv. I don't
> think there is a reason not to use it for textconv.

I was not aware of the other code paths and only wanted to
affect the one that I knew about.  I agree that making that the
default behavior would be great, meaning we could drop the
pretty_filename flag altogether.

If you and others agree that the user-friendly names are a good
thing to have by default then I can rework patch 2/2.

> However, I suspect that all callers should use pretty filenames, and
> then this bit would just go away.

I fully agree.


-- 
		David