Re: [PATCH 1/6] rebase -i: Add the "ref" command

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH 1/6] rebase -i: Add the "ref" command

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:51:31

Greg Price [off-list ref] writes:
...
+		if ! grep -Fq " $refname" "$state_dir"/oldrefs 2>/dev/null
+		then
+			echo "$sha1 $refname" >> "$state_dir"/oldrefs
(Style) Extra SP between ">>" and "$state_dir/oldrefs"
quoted hunk
diff --git a/git-rebase.sh b/git-rebase.sh
index d7855ea..1bfe6a8 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -118,6 +118,8 @@ read_basic_state () {
 		strategy_opts="$(cat "$state_dir"/strategy_opts)"
 	test -f "$state_dir"/allow_rerere_autoupdate &&
 		allow_rerere_autoupdate="$(cat "$state_dir"/allow_rerere_autoupdate)"
+	test -f "$state_dir"/oldrefs &&
+		oldrefs="$(cat "$state_dir"/oldrefs)"
 }
 
 write_basic_state () {
@@ -332,6 +334,15 @@ skip)
 abort)
 	git rerere clear
 	read_basic_state
+	[ -n "$oldrefs" ] && echo "$oldrefs" | while read sha1 ref
(Style) I think almost everybody else spells out "test".  Also please
break line before the while, like this:

	test -n "$oldrefs" &&
	echo "$oldrefs" |
	while read sha1 ref
        do
        	...
+	do
+		if test "(null)" = $sha1
Who is giving you "(null)"???

Re: [PATCH 1/6] rebase -i: Add the "ref" command

From: Greg Price <hidden>
Date: 2016-06-15 22:51:31

Thanks for the review.

On Mon, Jun 27, 2011 at 11:46:52AM -0700, Junio C Hamano wrote:
Greg Price [off-list ref] writes:
quoted
...
+		if ! grep -Fq " $refname" "$state_dir"/oldrefs 2>/dev/null
+		then
+			echo "$sha1 $refname" >> "$state_dir"/oldrefs
(Style) Extra SP between ">>" and "$state_dir/oldrefs"
Hmm -- it looks like the prevalent style in the codebase is actually
to include the space:

greg@gouda:~/w/git$ git grep -c '>>[^ ]' v1.7.6 git-*.sh
v1.7.6:git-bisect.sh:3
v1.7.6:git-instaweb.sh:2
v1.7.6:git-rebase--interactive.sh:2
v1.7.6:git-stash.sh:1
greg@gouda:~/w/git$ git grep -c '>> ' v1.7.6 git-*.sh
v1.7.6:git-am.sh:1
v1.7.6:git-filter-branch.sh:1
v1.7.6:git-instaweb.sh:8
v1.7.6:git-rebase--interactive.sh:10
v1.7.6:git-rebase--merge.sh:1

and in particular in git-rebase--interactive.sh.  But I could do it
either way.


quoted
@@ -332,6 +334,15 @@ skip)
 abort)
 	git rerere clear
 	read_basic_state
+	[ -n "$oldrefs" ] && echo "$oldrefs" | while read sha1 ref
(Style) I think almost everybody else spells out "test".  Also please
break line before the while, like this:

	test -n "$oldrefs" &&
	echo "$oldrefs" |
	while read sha1 ref
        do
        	...
Sure, done.


quoted
+	do
+		if test "(null)" = $sha1
Who is giving you "(null)"???
I am, myself -- it's what the 'ref' implementation in
git-rebase--interactive.sh uses to indicate that a ref had not existed
and should be deleted on abort.

+       ref)
+               mark_action_done
+               refname=$sha1
+               sha1=$(git rev-parse --quiet --verify "$refname" \
+                       || echo "(null)")
+               if ! grep -Fq " $refname" "$state_dir"/oldrefs 2>/dev/null
+               then
+                       echo "$sha1 $refname" >> "$state_dir"/oldrefs
+               fi

I could change it to something like "-".  It needs to be something
that the 'read' builtin, as used at the top of the loop, treats as a
word.

Greg
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help