threads / patch / 11905

patchgit-rebase.sh: Use POSIX/Susv command substitution instead of backticks

Subject: [PATCH] git-rebase.sh: Use POSIX/Susv command substitution instead of backticks

## tl;dr

7 messages between Feb 5, 2008 and Feb 6, 2008. Diffs are folded; open one to read it.

replies: 6people: 4as markdown or json

Jari Aalto· Feb 5, 2008, 22:08 UTC · lore

Use redable $(<command>) syntax instead of backtics in code. See The Open Group Base Specifications Issue 6, IEEE Std 1003.1, 2004 Edition

Signed-off-by: Jari Aalto <jari.aalto AT cante.net>
---
 See http://www.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html#tag_02_06_03
 git-rebase.sh |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)
Show changes to git-rebase.sh +7 −7
diff --git a/git-rebase.sh b/git-rebase.sh
index bdcea0e..d2b329f 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -60,7 +60,7 @@ continue_merge () {
 		die "$RESOLVEMSG"
 	fi
 
-	cmt=`cat "$dotest/current"`
+	cmt=$(cat "$dotest/current")
 	if ! git diff-index --quiet HEAD --
 	then
 		if ! git-commit -C "$cmt"
@@ -75,7 +75,7 @@ continue_merge () {
 	fi
 	git rev-list --pretty=oneline -1 "$cmt" | sed -e 's/^[^ ]* //'
 
-	prev_head=`git rev-parse HEAD^0`
+	prev_head=$(git rev-parse HEAD^0)
 	# save the resulting commit so we can read-tree on it later
 	echo "$prev_head" > "$dotest/prev_head"
 
@@ -233,7 +233,7 @@ do
 	-s|--s|--st|--str|--stra|--strat|--strate|--strateg|--strategy)
 		case "$#,$1" in
 		*,*=*)
-			strategy=`expr "z$1" : 'z-[^=]*=\(.*\)'` ;;
+			strategy=$(expr "z$1" : 'z-[^=]*=\(.*\)') ;;
 		1,*)
 			usage ;;
 		*)
@@ -295,7 +295,7 @@ esac
 
 # The upstream head must be given.  Make sure it is valid.
 upstream_name="$1"
-upstream=`git rev-parse --verify "${upstream_name}^0"` ||
+upstream=$(git rev-parse --verify "${upstream_name}^0") ||
     die "invalid upstream $upstream_name"
 
 # Make sure the branch to rebase onto is valid.
@@ -318,9 +318,9 @@ case "$#" in
 	git-checkout "$2" || usage
 	;;
 *)
-	if branch_name=`git symbolic-ref -q HEAD`
+	if branch_name=$(git symbolic-ref -q HEAD)
 	then
-		branch_name=`expr "z$branch_name" : 'zrefs/heads/\(.*\)'`
+		branch_name=$(expr "z$branch_name" : 'zrefs/heads/\(.*\)')
 	else
 		branch_name=HEAD ;# detached
 	fi
@@ -399,7 +399,7 @@ echo "$orig_head" > "$dotest/orig-head"
 echo "$head_name" > "$dotest/head-name"
 
 msgnum=0
-for cmt in `git rev-list --reverse --no-merges "$upstream"..ORIG_HEAD`
+for cmt in $(git rev-list --reverse --no-merges "$upstream"..ORIG_HEAD)
 do
 	msgnum=$(($msgnum + 1))
 	echo "$cmt" > "$dotest/cmt.$msgnum"

-- 
1.5.4-rc5.GIT-dirty


-- 
Welcome to FOSS revolution: we fix and modify until it shines
Johannes Schindelin· Feb 5, 2008, 22:27 UTC · re: Jari Aalto · lore

Re: [PATCH] git-rebase.sh: Use POSIX/Susv command substitution instead of backticks

Hi,
On Wed, 6 Feb 2008, Jari Aalto wrote:
> Use redable $(<command>) syntax instead of backtics in code.  See The 
> Open Group Base Specifications Issue 6, IEEE Std 1003.1, 2004 Edition

Sorry, I am not quite sure if this patch is worth it: either you want to clean this up in _all_ of our shell scripts, or you leave it. Or you fix it in those parts that you touch anyway, but again leave the rest as-are.

Ciao, Dscho

Jari Aalto· Feb 5, 2008, 22:53 UTC · re: Johannes Schindelin · lore

Re: [PATCH] git-rebase.sh: Use POSIX/Susv command substitution instead of backticks

* Tue 2008-02-05 Johannes Schindelin <Johannes.Schindelin@gmx.de>
* Message-Id: alpine.LSU.1.00.0802052226340.8543@racer.site
Show 9 quoted lines
> Hi,
>
> On Wed, 6 Feb 2008, Jari Aalto wrote:
>
>> Use redable $(<command>) syntax instead of backtics in code.  See The 
>> Open Group Base Specifications Issue 6, IEEE Std 1003.1, 2004 Edition
>
> Sorry, I am not quite sure if this patch is worth it: either you want to 
> clean this up in _all_ of our shell scripts, or you leave it.  
Yes, that's the plan. This is just a start.
Jari
-- 
Welcome to FOSS revolution: we fix and modify until it shines
Johannes Schindelin· Feb 5, 2008, 23:06 UTC · re: Jari Aalto · lore

Re: [PATCH] git-rebase.sh: Use POSIX/Susv command substitution instead of backticks

Hi,
On Wed, 6 Feb 2008, Jari Aalto wrote:
Show 12 quoted lines
> * Tue 2008-02-05 Johannes Schindelin <Johannes.Schindelin@gmx.de>
> * Message-Id: alpine.LSU.1.00.0802052226340.8543@racer.site
>
> > On Wed, 6 Feb 2008, Jari Aalto wrote:
> >
> >> Use redable $(<command>) syntax instead of backtics in code.  See The 
> >> Open Group Base Specifications Issue 6, IEEE Std 1003.1, 2004 Edition
> >
> > Sorry, I am not quite sure if this patch is worth it: either you want 
> > to clean this up in _all_ of our shell scripts, or you leave it.
> 
> Yes, that's the plan. This is just a start.

Again, you failed to address me. FYI my email address is johannes.schindelin@gmx.de. Thank you very much.

"This is just a start" is not good enough. First, you have to defend why this is a good change. It does not really fix anything.

And then you have to do it for all scripts in one go. Mind you, it is not really complicated: just one call to perl.

Hth, Dscho

Junio C Hamano· Feb 6, 2008, 00:59 UTC · re: Johannes Schindelin · lore

Re: [PATCH] git-rebase.sh: Use POSIX/Susv command substitution instead of backticks

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> And then you have to do it for all scripts in one go.  Mind you, it is not 
> really complicated: just one call to perl.

Please do not do this. If other people have pending changes, "cleanup for clean-up's sake" would create conflicts for no good reason.

There are only two cases such a clean-up patch is good:
 (1) When the maintainer is not yet accepting any patches after
     a release-freeze and there is no pending patches from the
     community, and/or if you can convince people with pending
     patches to rebase on top of the clean-up because the
     current codebase is so unmaintainably bad, then a
     whole-tree clean-up patch should go in before anything
     else, forcing everybody to rebase on top of it;
 (2) If you will be working on the code in an area, you may want
     to have the first one in the series a "pure clean-up and
     nothing else" of the whole area, and then build your real
     changes on top.  You still need to coordinate with people
     whose patches may get hit by your clean-ups, but you have
     to do this anyway because you will have conflicts from your
     "real changes".

Any other "clean-up patch" would result in a not-so-appreciated code churn. Please don't encourage it.

Junio C Hamano· Feb 6, 2008, 02:03 UTC · re: Junio C Hamano · lore

Re: [PATCH] git-rebase.sh: Use POSIX/Susv command substitution instead of backticks

Junio C Hamano <gitster@pobox.com> writes:
Show 29 quoted lines
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>> And then you have to do it for all scripts in one go.  Mind you, it is not 
>> really complicated: just one call to perl.
>
> Please do not do this.  If other people have pending changes,
> "cleanup for clean-up's sake" would create conflicts for no good
> reason.
>
> There are only two cases such a clean-up patch is good:
>
>  (1) When the maintainer is not yet accepting any patches after
>      a release-freeze and there is no pending patches from the
>      community, and/or if you can convince people with pending
>      patches to rebase on top of the clean-up because the
>      current codebase is so unmaintainably bad, then a
>      whole-tree clean-up patch should go in before anything
>      else, forcing everybody to rebase on top of it;
>
>  (2) If you will be working on the code in an area, you may want
>      to have the first one in the series a "pure clean-up and
>      nothing else" of the whole area, and then build your real
>      changes on top.  You still need to coordinate with people
>      whose patches may get hit by your clean-ups, but you have
>      to do this anyway because you will have conflicts from your
>      "real changes".
>
> Any other "clean-up patch" would result in a not-so-appreciated
> code churn.  Please don't encourage it.
Just to make sure Jari does not get a wrong idea,

My "Please don't" is meant against Johannes's "Do it all if you do it".

If Jari did the patch as the first step of making real changes to "git rebase" (making -i not forcing -m, perhaps), it is the right thing to have a clean-up patch as a preparatory step, before starting the real work in later patches in the series. And such a clean-up patch should not inflict useless code churn on other commands.

So a single patch only to git-rebase is acceptable if that is what Jari is planning to do: preparatory clean-up before bringing a real improvement in.

Ralf Wildenhues· Feb 6, 2008, 09:23 UTC · re: Johannes Schindelin · lore

Re: [PATCH] git-rebase.sh: Use POSIX/Susv command substitution instead of backticks

Johannes Schindelin <Johannes.Schindelin <at> gmx.de> writes:
> > > On Wed, 6 Feb 2008, Jari Aalto wrote:
> > >
> > >> Use redable $(<command>) syntax instead of backtics in code.  See The 
> > >> Open Group Base Specifications Issue 6, IEEE Std 1003.1, 2004 Edition
> And then you have to do it for all scripts in one go.  Mind you, it is not 
> really complicated: just one call to perl.

You still have to check all backslashes within the changed command; some you may now have to remove.

Cheers, Ralf

← back to recent threads