Re: [PATCH v2] mergetool: use more conservative temporary filenames

From: David Aguilar <davvid@gmail.com>
Date: Oct 10, 2014, 18:48 UTC
Message-ID: <20141010184834.GB78117@gmail.com>
In-Reply-To: <5437C0CC.7030102@gmail.com>

On Fri, Oct 10, 2014 at 01:19:40PM +0200, Jakub Narębski wrote:

Show 33 quoted lines

> David Aguilar wrote:
> >Avoid filenames with multiple dots so that overly-picky tools do
> >not misinterpret their extension.
> >
> >Previously, foo/bar.ext in the worktree would result in e.g.
> >
> >	./foo/bar.ext.BASE.1234.ext
> >
> >This can be improved by having only a single .ext and using
> >underscore instead of dot so that the extension cannot be
> >misinterpreted.  The resulting path becomes:
> >
> >	./foo/bar_BASE_1234.ext
> >
> >Suggested-by: Sergio Ferrero <sferrero@ensoftcorp.com>
> >Helped-by: Junio C Hamano <gitster@pobox.com>
> >Signed-off-by: David Aguilar <davvid@gmail.com>
> >---
> 
> >+	if BASE=$(expr "$MERGED" : '\(.*\)\.[^/]*$')
> >+	then
> >+		ext=$(expr "$MERGED" : '.*\(\.[^/]*\)$')
> >+	else
> >+		BASE=$MERGED
> >+		ext=
> >+	fi
> 
> Why use expr and not POSIX shell parameter substitution?
> 
> BASE=${MERGED%.*}
> ext=.${MERGED##*.}
> 
> Or something like that...

Thanks for the sug.

My POSIX shell parameter expansion-fu is not super advanced, but if you can help me rework it I'd be happy to reroll.

It does seem simple and robust with expr, though. Extending the parameter expansion approach to work in all cases may end up with more complexity than with the expr method, it seems.

Here are the use cases:

	$ MERGED=foo.bar.baz && echo ${MERGED%.*} ${MERGED##*.}
	foo.bar baz

Good.

	$ MERGED=foo && echo ${MERGED%.*} ${MERGED##*.}
	foo foo

Bad. There's no extension and the substitution doesn't handle it.

	$ MERGED=foo.bar/baz && echo ${MERGED%.*} ${MERGED##*.}
	foo bar/baz

Bad. There's no extension but the substitution thinks the parent directory's extension-less name is the basename, and thinks that bar/baz is the extension.

I am curious to know whether there's a nice and elegant way to do it with shell expansions. Let me know what you think.

cheers,

-- 
David