Re: [PATCH v2 2/2] diff: generate prettier filenames when using GIT_EXTERNAL_DIFF

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:

quoted

+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