Re: [PATCH v3] diff: generate prettier filenames when using GIT_EXTERNAL_DIFF
- From
David Aguilar <davvid@gmail.com>
- Date
- May 26, 2009, 17:37 UTC
- Message-ID
- <20090526173726.GA5528@gmail.com>
- In-Reply-To
- <4A1B85B8.1050907@gmail.com>
On Mon, May 25, 2009 at 11:01:28PM -0700, Stephen Boyd wrote:
Show 23 quoted lines
> David Aguilar wrote:
> > - fd = git_mkstemp(temp->tmp_path, PATH_MAX, ".diff_XXXXXX");
> > + if (pretty_filename) {
> > + struct strbuf pretty_name = STRBUF_INIT;
> > + char *pathdup = xstrdup(path);
> > + char *base = basename(pathdup);
> > + char *dot = strchr(base, '.');
> > + int suffix_len = 0;
> > +
> > + if (dot) {
> > + /* path has an extension, e.g. "foo.txt";
> > + * generate "foo.XXXX.txt".
> > + */
> > + *dot = '\0';
> > + strbuf_addstr(&pretty_name, base);
> > + *dot = '.';
> > + strbuf_addstr(&pretty_name, ".XXXXXX");
> > + suffix_len = strlen(dot);
> > + strbuf_addstr(&pretty_name, dot);
>
> This *dot business annoys me. Would it be better to use strbuf_add()
> with some pointer math thrown in? Also, what happens with files such as
> "foo.bar.txt"? Do we want "foo.XXXXX.bar.txt"?That was intentional; "foo.tar.gz" becomes "foo.XXXXXX.tar.gz", which is what I considered better behavior when writing it.
Thanks for the note about using strbuf_add(). I'll see if we can simplify things by using it instead of strbuf_addstr().
I should probably split the "add compat/mkstemps" and "use it in git_mkstemps()" parts into separate commits since we're going to want to use the native mkstemps implementation if it's available.
-- David