Git development
 help / color / mirror / Atom feed
* [PATCH] git-rebase.sh: Use POSIX/Susv command substitution instead of backticks
@ 2008-02-05 22:08 Jari Aalto
  2008-02-05 22:27 ` Johannes Schindelin
  0 siblings, 1 reply; 7+ messages in thread
From: Jari Aalto @ 2008-02-05 22:08 UTC (permalink / raw)
  To: git

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(-)

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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] git-rebase.sh: Use POSIX/Susv command substitution instead of backticks
  2008-02-05 22:08 [PATCH] git-rebase.sh: Use POSIX/Susv command substitution instead of backticks Jari Aalto
@ 2008-02-05 22:27 ` Johannes Schindelin
  2008-02-05 22:53   ` Jari Aalto
  0 siblings, 1 reply; 7+ messages in thread
From: Johannes Schindelin @ 2008-02-05 22:27 UTC (permalink / raw)
  To: Jari Aalto; +Cc: git

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] git-rebase.sh: Use POSIX/Susv command substitution instead of backticks
  2008-02-05 22:27 ` Johannes Schindelin
@ 2008-02-05 22:53   ` Jari Aalto
  2008-02-05 23:06     ` Johannes Schindelin
  0 siblings, 1 reply; 7+ messages in thread
From: Jari Aalto @ 2008-02-05 22:53 UTC (permalink / raw)
  To: git

* Tue 2008-02-05 Johannes Schindelin <Johannes.Schindelin@gmx.de>
* Message-Id: alpine.LSU.1.00.0802052226340.8543@racer.site
> 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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] git-rebase.sh: Use POSIX/Susv command substitution instead of backticks
  2008-02-05 22:53   ` Jari Aalto
@ 2008-02-05 23:06     ` Johannes Schindelin
  2008-02-06  0:59       ` Junio C Hamano
  2008-02-06  9:23       ` Ralf Wildenhues
  0 siblings, 2 replies; 7+ messages in thread
From: Johannes Schindelin @ 2008-02-05 23:06 UTC (permalink / raw)
  To: Jari Aalto; +Cc: git

Hi,

On Wed, 6 Feb 2008, Jari Aalto wrote:

> * 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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] git-rebase.sh: Use POSIX/Susv command substitution instead of backticks
  2008-02-05 23:06     ` Johannes Schindelin
@ 2008-02-06  0:59       ` Junio C Hamano
  2008-02-06  2:03         ` Junio C Hamano
  2008-02-06  9:23       ` Ralf Wildenhues
  1 sibling, 1 reply; 7+ messages in thread
From: Junio C Hamano @ 2008-02-06  0:59 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Jari Aalto, git

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.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] git-rebase.sh: Use POSIX/Susv command substitution instead of backticks
  2008-02-06  0:59       ` Junio C Hamano
@ 2008-02-06  2:03         ` Junio C Hamano
  0 siblings, 0 replies; 7+ messages in thread
From: Junio C Hamano @ 2008-02-06  2:03 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Jari Aalto, git

Junio C Hamano <gitster@pobox.com> writes:

> 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.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] git-rebase.sh: Use POSIX/Susv command substitution instead of backticks
  2008-02-05 23:06     ` Johannes Schindelin
  2008-02-06  0:59       ` Junio C Hamano
@ 2008-02-06  9:23       ` Ralf Wildenhues
  1 sibling, 0 replies; 7+ messages in thread
From: Ralf Wildenhues @ 2008-02-06  9:23 UTC (permalink / raw)
  To: git

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2008-02-06  9:25 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-02-05 22:08 [PATCH] git-rebase.sh: Use POSIX/Susv command substitution instead of backticks Jari Aalto
2008-02-05 22:27 ` Johannes Schindelin
2008-02-05 22:53   ` Jari Aalto
2008-02-05 23:06     ` Johannes Schindelin
2008-02-06  0:59       ` Junio C Hamano
2008-02-06  2:03         ` Junio C Hamano
2008-02-06  9:23       ` Ralf Wildenhues

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox