git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Aguilar <davvid@gmail.com>
To: "Jakub Narębski" <jnareb@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	git@vger.kernel.org, Sergio Ferrero <sferrero@ensoftcorp.com>,
	Charles Bailey <charles@hashpling.org>
Subject: Re: [PATCH v2] mergetool: use more conservative temporary filenames
Date: Fri, 10 Oct 2014 11:48:36 -0700	[thread overview]
Message-ID: <20141010184834.GB78117@gmail.com> (raw)
In-Reply-To: <5437C0CC.7030102@gmail.com>

On Fri, Oct 10, 2014 at 01:19:40PM +0200, Jakub Narębski wrote:
> 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

  reply	other threads:[~2014-10-10 18:48 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-10  8:19 [PATCH v2] mergetool: use more conservative temporary filenames David Aguilar
2014-10-10 11:19 ` Jakub Narębski
2014-10-10 18:48   ` David Aguilar [this message]
2014-10-10 19:54     ` Johannes Sixt
2014-10-10 21:16   ` Junio C Hamano
2014-10-13 19:30 ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141010184834.GB78117@gmail.com \
    --to=davvid@gmail.com \
    --cc=charles@hashpling.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jnareb@gmail.com \
    --cc=sferrero@ensoftcorp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).