Git development
 help / color / mirror / Atom feed
* [PATCH (GITK) 1/6] gitk: Kill back-end processes on window close.
From: Alexander Gavrilov @ 2008-07-27  6:18 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras
In-Reply-To: <200807271017.23272.angavrilov@gmail.com>

Date: Sat, 12 Jul 2008 16:09:28 +0400

When collecting commits for a rarely changed, or recently
created file or directory, rev-list may work for a noticeable
period of time without producing any output. Such processes
don't receive SIGPIPE for a while after gitk is closed, thus
becoming runaway CPU hogs.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---
 gitk |   35 +++++++++++++++++++++++++----------
 1 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/gitk b/gitk
index fddcb45..29d79d6 100755
--- a/gitk
+++ b/gitk
@@ -375,19 +375,33 @@ proc start_rev_list {view} {
     return 1
 }
 
+proc stop_instance {inst} {
+    global commfd leftover
+
+    set fd $commfd($inst)
+    catch {
+	set pid [pid $fd]
+	exec kill $pid
+    }
+    catch {close $fd}
+    nukefile $fd
+    unset commfd($inst)
+    unset leftover($inst)
+}
+
+proc stop_backends {} {
+    global commfd
+
+    foreach inst [array names commfd] {
+	stop_instance $inst
+    }
+}
+
 proc stop_rev_list {view} {
-    global commfd viewinstances leftover
+    global viewinstances
 
     foreach inst $viewinstances($view) {
-	set fd $commfd($inst)
-	catch {
-	    set pid [pid $fd]
-	    exec kill $pid
-	}
-	catch {close $fd}
-	nukefile $fd
-	unset commfd($inst)
-	unset leftover($inst)
+	stop_instance $inst
     }
     set viewinstances($view) {}
 }
@@ -2103,6 +2117,7 @@ proc makewindow {} {
     bind . <$M1B-minus> {incrfont -1}
     bind . <$M1B-KP_Subtract> {incrfont -1}
     wm protocol . WM_DELETE_WINDOW doquit
+    bind . <Destroy> {stop_backends}
     bind . <Button-1> "click %W"
     bind $fstring <Key-Return> {dofind 1 1}
     bind $sha1entry <Key-Return> gotocommit
-- 
1.5.6.3.18.gfe82

^ permalink raw reply related

* Re: git-scm.com
From: "Peter Valdemar Mørch (Lists)" @ 2008-07-27  6:19 UTC (permalink / raw)
  To: git
In-Reply-To: <7vsktwfu5z.fsf@gitster.siamese.dyndns.org>

As a contributer with a single commit I was happy to see myself appear 
shortly on the list (yeah!). Ok, so I realize it is vanity and a little 
silly... :-)

Junio C Hamano gitster-at-pobox.com |Lists| wrote:
> I have a mild suspicion that sorting that list in alphabetical order may
> actually make it much better.  It all depends on the purpose of that list,
> though.

To me it makes sense to sort the entire list according to commits. Its 
still easy to find anybody with search, and I find it appropriate that I 
be towards the end. The commit sorting encourages me to move up the 
list! :-D

> And for the "giving credit" purpose, I do not think truncating the list at
> 5 commits or less threshold, as suggested earlier and already done, makes
> much sense, either.

And why truncate the list? I'd personally like to be back on the list 
(vanity! - but true), bandwidth is relatively cheap, and there is 
nothing below the list. I also think it makes the community look healty 
and encourages contribution to see how many others contribute.

Peter
-- 
Peter Valdemar Mørch
http://www.morch.com

^ permalink raw reply

* [PATCH (GITK) 2/6] gitk: Register diff-files & diff-index in commfd, to ensure kill.
From: Alexander Gavrilov @ 2008-07-27  6:19 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras
In-Reply-To: <200807271018.22969.angavrilov@gmail.com>

Date: Sun, 13 Jul 2008 16:40:47 +0400

Local change analysis can take a noticeable amount of time on large
file sets, and produce no output if there are no changes. Register
the back-ends in commfd, so that they get properly killed on window
close.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---
 gitk |   39 +++++++++++++++++++++++----------------
 1 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/gitk b/gitk
index 29d79d6..b523c98 100755
--- a/gitk
+++ b/gitk
@@ -90,6 +90,15 @@ proc dorunq {} {
     }
 }
 
+proc reg_instance {fd} {
+    global commfd leftover loginstance
+
+    set i [incr loginstance]
+    set commfd($i) $fd
+    set leftover($i) {}
+    return $i
+}
+
 proc unmerged_files {files} {
     global nr_unmerged
 
@@ -294,10 +303,10 @@ proc parseviewrevs {view revs} {
 # Start off a git log process and arrange to read its output
 proc start_rev_list {view} {
     global startmsecs commitidx viewcomplete curview
-    global commfd leftover tclencoding
+    global tclencoding
     global viewargs viewargscmd viewfiles vfilelimit
     global showlocalchanges commitinterest
-    global viewactive loginstance viewinstances vmergeonly
+    global viewactive viewinstances vmergeonly
     global pending_select mainheadid
     global vcanopt vflags vrevs vorigargs
 
@@ -354,10 +363,8 @@ proc start_rev_list {view} {
 	error_popup "[mc "Error executing git log:"] $err"
 	return 0
     }
-    set i [incr loginstance]
+    set i [reg_instance $fd]
     set viewinstances($view) [list $i]
-    set commfd($i) $fd
-    set leftover($i) {}
     if {$showlocalchanges && $mainheadid ne {}} {
 	lappend commitinterest($mainheadid) {dodiffindex}
     }
@@ -420,8 +427,8 @@ proc getcommits {} {
 
 proc updatecommits {} {
     global curview vcanopt vorigargs vfilelimit viewinstances
-    global viewactive viewcomplete loginstance tclencoding
-    global startmsecs commfd showneartags showlocalchanges leftover
+    global viewactive viewcomplete tclencoding
+    global startmsecs showneartags showlocalchanges
     global mainheadid pending_select
     global isworktree
     global varcid vposids vnegids vflags vrevs
@@ -482,10 +489,8 @@ proc updatecommits {} {
     if {$viewactive($view) == 0} {
 	set startmsecs [clock clicks -milliseconds]
     }
-    set i [incr loginstance]
+    set i [reg_instance $fd]
     lappend viewinstances($view) $i
-    set commfd($i) $fd
-    set leftover($i) {}
     fconfigure $fd -blocking 0 -translation lf -eofchar {}
     if {$tclencoding != {}} {
 	fconfigure $fd -encoding $tclencoding
@@ -4063,10 +4068,11 @@ proc dodiffindex {} {
     incr lserial
     set fd [open "|git diff-index --cached HEAD" r]
     fconfigure $fd -blocking 0
-    filerun $fd [list readdiffindex $fd $lserial]
+    set i [reg_instance $fd]
+    filerun $fd [list readdiffindex $fd $lserial $i]
 }
 
-proc readdiffindex {fd serial} {
+proc readdiffindex {fd serial inst} {
     global mainheadid nullid nullid2 curview commitinfo commitdata lserial
 
     set isdiff 1
@@ -4077,7 +4083,7 @@ proc readdiffindex {fd serial} {
 	set isdiff 0
     }
     # we only need to see one line and we don't really care what it says...
-    close $fd
+    stop_instance $inst
 
     if {$serial != $lserial} {
 	return 0
@@ -4086,7 +4092,8 @@ proc readdiffindex {fd serial} {
     # now see if there are any local changes not checked in to the index
     set fd [open "|git diff-files" r]
     fconfigure $fd -blocking 0
-    filerun $fd [list readdifffiles $fd $serial]
+    set i [reg_instance $fd]
+    filerun $fd [list readdifffiles $fd $serial $i]
 
     if {$isdiff && ![commitinview $nullid2 $curview]} {
 	# add the line for the changes in the index to the graph
@@ -4103,7 +4110,7 @@ proc readdiffindex {fd serial} {
     return 0
 }
 
-proc readdifffiles {fd serial} {
+proc readdifffiles {fd serial inst} {
     global mainheadid nullid nullid2 curview
     global commitinfo commitdata lserial
 
@@ -4115,7 +4122,7 @@ proc readdifffiles {fd serial} {
 	set isdiff 0
     }
     # we only need to see one line and we don't really care what it says...
-    close $fd
+    stop_instance $inst
 
     if {$serial != $lserial} {
 	return 0
-- 
1.5.6.3.18.gfe82

^ permalink raw reply related

* [PATCH (GITK) 3/6] gitk: On Windows use a Cygwin-specific flag for kill.
From: Alexander Gavrilov @ 2008-07-27  6:20 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras
In-Reply-To: <200807271019.17267.angavrilov@gmail.com>

Date: Tue, 15 Jul 2008 00:35:42 +0400

MSysGit compiles git binaries as native Windows executables,
so they cannot be killed unless a special flag is specified.

This flag is implemented by the Cygwin version of kill,
which is also included in MSysGit.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---
 gitk |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/gitk b/gitk
index b523c98..d7fea26 100755
--- a/gitk
+++ b/gitk
@@ -388,7 +388,12 @@ proc stop_instance {inst} {
     set fd $commfd($inst)
     catch {
 	set pid [pid $fd]
-	exec kill $pid
+
+	if {$::tcl_platform(platform) eq {windows}} {
+	    exec kill -f $pid
+	} else {
+	    exec kill $pid
+	}
     }
     catch {close $fd}
     nukefile $fd
-- 
1.5.6.3.18.gfe82

^ permalink raw reply related

* [PATCH (GITK) 4/6] gitk: Fixed broken exception handling in diff.
From: Alexander Gavrilov @ 2008-07-27  6:20 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras
In-Reply-To: <200807271020.02415.angavrilov@gmail.com>

Date: Sat, 26 Jul 2008 18:48:41 +0400

If the tree diff command failed to start for some
random reason, treepending remained set, and thus
no more diffs were shown after that.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---
 gitk |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/gitk b/gitk
index d7fea26..abb6542 100755
--- a/gitk
+++ b/gitk
@@ -6457,9 +6457,10 @@ proc diffcmd {ids flags} {
 proc gettreediffs {ids} {
     global treediff treepending
 
+    if {[catch {set gdtf [open [diffcmd $ids {--no-commit-id}] r]}]} return
+
     set treepending $ids
     set treediff {}
-    if {[catch {set gdtf [open [diffcmd $ids {--no-commit-id}] r]}]} return
     fconfigure $gdtf -blocking 0
     filerun $gdtf [list gettreediffline $gdtf $ids]
 }
-- 
1.5.6.3.18.gfe82

^ permalink raw reply related

* [PATCH (GITK) 5/6] gitk: Fixed automatic row selection during load.
From: Alexander Gavrilov @ 2008-07-27  6:21 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras
In-Reply-To: <200807271020.53171.angavrilov@gmail.com>

Date: Sat, 26 Jul 2008 20:13:45 +0400

- Switching views now actually preserves the selected commit.
- Reloading (also Edit View) preserves the currently selected commit.
- Initial selection does not produce weird scrolling.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---
 gitk |   41 ++++++++++++++++++++++++-----------------
 1 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/gitk b/gitk
index abb6542..5021437 100755
--- a/gitk
+++ b/gitk
@@ -307,7 +307,7 @@ proc start_rev_list {view} {
     global viewargs viewargscmd viewfiles vfilelimit
     global showlocalchanges commitinterest
     global viewactive viewinstances vmergeonly
-    global pending_select mainheadid
+    global mainheadid
     global vcanopt vflags vrevs vorigargs
 
     set startmsecs [clock clicks -milliseconds]
@@ -374,9 +374,6 @@ proc start_rev_list {view} {
     }
     filerun $fd [list getcommitlines $fd $i $view 0]
     nowbusy $view [mc "Reading"]
-    if {$view == $curview} {
-	set pending_select $mainheadid
-    }
     set viewcomplete($view) 0
     set viewactive($view) 1
     return 1
@@ -418,11 +415,22 @@ proc stop_rev_list {view} {
     set viewinstances($view) {}
 }
 
-proc getcommits {} {
+proc reset_pending_select {selid} {
+    global pending_select mainheadid
+
+    if {$selid ne {}} {
+	set pending_select $selid
+    } else {
+	set pending_select $mainheadid
+    }
+}
+
+proc getcommits {selid} {
     global canv curview need_redisplay viewactive
 
     initlayout
     if {[start_rev_list $curview]} {
+	reset_pending_select $selid
 	show_status [mc "Reading commits..."]
 	set need_redisplay 1
     } else {
@@ -503,7 +511,7 @@ proc updatecommits {} {
     filerun $fd [list getcommitlines $fd $i $view 1]
     incr viewactive($view)
     set viewcomplete($view) 0
-    set pending_select $mainheadid
+    reset_pending_select {}
     nowbusy $view "Reading"
     if {$showneartags} {
 	getallcommits
@@ -515,6 +523,11 @@ proc reloadcommits {} {
     global showneartags treediffs commitinterest cached_commitrow
     global targetid
 
+    set selid {}
+    if {$selectedline ne {}} {
+	set selid $currentid
+    }
+
     if {!$viewcomplete($curview)} {
 	stop_rev_list $curview
     }
@@ -533,7 +546,7 @@ proc reloadcommits {} {
     catch {unset cached_commitrow}
     catch {unset targetid}
     setcanvscroll
-    getcommits
+    getcommits $selid
     return 0
 }
 
@@ -3325,10 +3338,7 @@ proc showview {n} {
 
     run refill_reflist
     if {![info exists viewcomplete($n)]} {
-	if {$selid ne {}} {
-	    set pending_select $selid
-	}
-	getcommits
+	getcommits $selid
 	return
     }
 
@@ -3365,11 +3375,7 @@ proc showview {n} {
     } elseif {$mainheadid ne {} && [commitinview $mainheadid $curview]} {
 	selectline [rowofcommit $mainheadid] 1
     } elseif {!$viewcomplete($n)} {
-	if {$selid ne {}} {
-	    set pending_select $selid
-	} else {
-	    set pending_select $mainheadid
-	}
+	reset_pending_select $selid
     } else {
 	set row [first_real_row]
 	if {$row < $numcommits} {
@@ -4036,6 +4042,7 @@ proc layoutmore {} {
     }
     if {[info exists pending_select] &&
 	[commitinview $pending_select $curview]} {
+	update
 	selectline [rowofcommit $pending_select] 1
     }
     drawvisible
@@ -9973,4 +9980,4 @@ if {[info exists permviews]} {
 	addviewmenu $n
     }
 }
-getcommits
+getcommits {}
-- 
1.5.6.3.18.gfe82

^ permalink raw reply related

* [PATCH (GITK) 6/6] gitk: Fallback to selecting the head commit upon load.
From: Alexander Gavrilov @ 2008-07-27  6:22 UTC (permalink / raw)
  To: git; +Cc: Paul Mackerras
In-Reply-To: <200807271021.46661.angavrilov@gmail.com>

Date: Sat, 26 Jul 2008 20:15:54 +0400

Try selecting the head, if the previously selected commit
is not available in the new view.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---
 gitk |   25 ++++++++++++++++++-------
 1 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/gitk b/gitk
index 5021437..d093a39 100755
--- a/gitk
+++ b/gitk
@@ -1506,8 +1506,15 @@ proc chewcommits {} {
 	global numcommits startmsecs
 
 	if {[info exists pending_select]} {
-	    set row [first_real_row]
-	    selectline $row 1
+	    update
+	    reset_pending_select {}
+
+	    if {[commitinview $pending_select $curview]} {
+		selectline [rowofcommit $pending_select] 1
+	    } else {
+		set row [first_real_row]
+		selectline $row 1
+	    }
 	}
 	if {$commitidx($curview) > 0} {
 	    #set ms [expr {[clock clicks -milliseconds] - $startmsecs}]
@@ -3372,14 +3379,18 @@ proc showview {n} {
     drawvisible
     if {$row ne {}} {
 	selectline $row 0
-    } elseif {$mainheadid ne {} && [commitinview $mainheadid $curview]} {
-	selectline [rowofcommit $mainheadid] 1
     } elseif {!$viewcomplete($n)} {
 	reset_pending_select $selid
     } else {
-	set row [first_real_row]
-	if {$row < $numcommits} {
-	    selectline $row 0
+	reset_pending_select {}
+
+	if {[commitinview $pending_select $curview]} {
+	    selectline [rowofcommit $pending_select] 1
+	} else {
+	    set row [first_real_row]
+	    if {$row < $numcommits} {
+		selectline $row 0
+	    }
 	}
     }
     if {!$viewcomplete($n)} {
-- 
1.5.6.3.18.gfe82

^ permalink raw reply related

* [PATCH (GIT-GUI) 1/2] git-gui: Fix the Remote menu separator.
From: Alexander Gavrilov @ 2008-07-27  6:34 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git

It was positioned incorrectly (offset by one position)
if the menu had a tear-off handle.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---

	A couple of random fixes.

	-- Alexander

 git-gui.sh |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/git-gui.sh b/git-gui.sh
index 9d0627d..da903a1 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -2926,6 +2926,7 @@ if {[is_enabled transport]} {
 	populate_fetch_menu
 	set n [expr {[.mbar.remote index end] - $n}]
 	if {$n > 0} {
+		if {[.mbar.remote type 0] eq "tearoff"} { incr n }
 		.mbar.remote insert $n separator
 	}
 	unset n
-- 
1.5.6.3.18.gfe82

^ permalink raw reply related

* [PATCH (GIT-GUI) 2/2] git-gui: Preserve scroll position on reshow_diff.
From: Alexander Gavrilov @ 2008-07-27  6:35 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <200807271034.21833.angavrilov@gmail.com>

It is especially useful for Stage/Unstage Line, because
they invoke full state scan and diff reload, which originally
would reset the scroll position to the top of the file.

Signed-off-by: Alexander Gavrilov <angavrilov@gmail.com>
---
 lib/diff.tcl |   18 ++++++++++++++----
 1 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/lib/diff.tcl b/lib/diff.tcl
index 96ba949..1c1aa56 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -19,6 +19,7 @@ proc clear_diff {} {
 proc reshow_diff {} {
 	global file_states file_lists
 	global current_diff_path current_diff_side
+	global ui_diff
 
 	set p $current_diff_path
 	if {$p eq {}} {
@@ -28,7 +29,8 @@ proc reshow_diff {} {
 		|| [lsearch -sorted -exact $file_lists($current_diff_side) $p] == -1} {
 		clear_diff
 	} else {
-		show_diff $p $current_diff_side
+		set save_pos [lindex [$ui_diff yview] 0]
+		show_diff $p $current_diff_side {} $save_pos
 	}
 }
 
@@ -52,7 +54,7 @@ A rescan will be automatically started to find other files which may have the sa
 	rescan ui_ready 0
 }
 
-proc show_diff {path w {lno {}}} {
+proc show_diff {path w {lno {}} {scroll_pos {}}} {
 	global file_states file_lists
 	global is_3way_diff diff_active repo_config
 	global ui_diff ui_index ui_workdir
@@ -151,6 +153,10 @@ proc show_diff {path w {lno {}}} {
 		$ui_diff conf -state disabled
 		set diff_active 0
 		unlock_index
+		if {$scroll_pos ne {}} {
+			update
+			$ui_diff yview moveto $scroll_pos
+		}
 		ui_ready
 		return
 	}
@@ -190,10 +196,10 @@ proc show_diff {path w {lno {}}} {
 		-blocking 0 \
 		-encoding binary \
 		-translation binary
-	fileevent $fd readable [list read_diff $fd]
+	fileevent $fd readable [list read_diff $fd $scroll_pos]
 }
 
-proc read_diff {fd} {
+proc read_diff {fd scroll_pos} {
 	global ui_diff diff_active
 	global is_3way_diff current_diff_header
 
@@ -282,6 +288,10 @@ proc read_diff {fd} {
 		close $fd
 		set diff_active 0
 		unlock_index
+		if {$scroll_pos ne {}} {
+			update
+			$ui_diff yview moveto $scroll_pos
+		}
 		ui_ready
 
 		if {[$ui_diff index end] eq {2.0}} {
-- 
1.5.6.3.18.gfe82

^ permalink raw reply related

* Re: [StGit PATCH] Fixed default install location
From: Daniel White @ 2008-07-27  6:27 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git
In-Reply-To: <b0943d9e0807251446w1e9ed73erfa7c1638191d88a@mail.gmail.com>

"Catalin Marinas" <catalin.marinas@gmail.com> wrote:
> 
> I also use "python setup.py bdist_rpm" when releasing, I think it will
> get confused by a different prefix.
> 

Not being familiar with distutils, I didn't even see that use case.

Regardless, the instructions are incorrect and the behaviour surprising.
When I installed this in a cygwin environment, it went on to clobber
my system directories.

Would it be more useful to have the Makefile handle the general case and
setup.py for more specialised cases, such as generating an rpm?

I've thrown some patches together that does some of this.  I can tidy
these up and put them in a public repository if this sounds like a
reasonable plan of attack.

-- 
Daniel White

^ permalink raw reply

* Re: [StGit PATCH] Fixed default install location
From: Catalin Marinas @ 2008-07-27  8:21 UTC (permalink / raw)
  To: Daniel White; +Cc: git
In-Reply-To: <20080727162750.25b7cdf3@whitehouse.id.au>

2008/7/27 Daniel White <daniel@whitehouse.id.au>:
> "Catalin Marinas" <catalin.marinas@gmail.com> wrote:
>>
>> I also use "python setup.py bdist_rpm" when releasing, I think it will
>> get confused by a different prefix.
>
> Not being familiar with distutils, I didn't even see that use case.

I use "python setup.py bdist_rpm", though passing --prefix would
probably fix it.

> Regardless, the instructions are incorrect and the behaviour surprising.
> When I installed this in a cygwin environment, it went on to clobber
> my system directories.
>
> Would it be more useful to have the Makefile handle the general case and
> setup.py for more specialised cases, such as generating an rpm?

I agree.

> I've thrown some patches together that does some of this.  I can tidy
> these up and put them in a public repository if this sounds like a
> reasonable plan of attack.

Yes, it is. Please base them on my (or Karl's) latest git tree as I
already merged this patch.

Thanks.

-- 
Catalin

^ permalink raw reply

* Re: [StGit PATCH] Fix some remaining old-style stg id calls
From: Catalin Marinas @ 2008-07-27  8:24 UTC (permalink / raw)
  To: Karl Hasselström; +Cc: git
In-Reply-To: <20080725004430.12440.49829.stgit@yoghurt>

2008/7/25 Karl Hasselström <kha@treskal.com>:
> You'll want to add this (just squash it into your patch). The calls
> were failing, but since both sides produced the empty string on
> stdout, the test was happy anyway.

Thanks, that's why I haven't noticed it.

-- 
Catalin

^ permalink raw reply

* Re: StGit: kha/{stable,safe,experimental} updated
From: Catalin Marinas @ 2008-07-27  8:44 UTC (permalink / raw)
  To: Karl Hasselström; +Cc: git, Samuel Tardieu
In-Reply-To: <20080725013936.GA20959@diana.vm.bytemark.co.uk>

2008/7/25 Karl Hasselström <kha@treskal.com>:
> The big update since last time is support (in kha/experimental) for
> hidden patches in the new-infrastructure commands and stack log.
>
> Unless more problems are uncovered, I'll soon move all patches in
> experimental to safe (which means I'll be recommending that Catalin
> merge them).

I'll have a look at the new stack log format (my main worry) this week
but the other patches look OK.

I merged the safe and stable branches.

Thanks.

-- 
Catalin

^ permalink raw reply

* Re: [PATCH] Optional grouping of projects by category.
From: Jakub Narebski @ 2008-07-27  8:53 UTC (permalink / raw)
  To: Sebastien Cevey; +Cc: git
In-Reply-To: <8763qsi2mt.wl%seb@cine7.net>

Sebastien Cevey <seb@cine7.net> writes:

> This adds the GITWEB_GROUP_CATEGORIES option which, if enabled, will
> result in grouping projects by category on the project list page.  The
> category is specified for each project by the $GIT_DIR/category file
> or the 'category' variable in its configuration file.
> 
> The feature is inspired from Sham Chukoury's patch for the XMMS2
> gitweb, but has been rewritten for the current gitweb development
> HEAD.
> 
> Thanks to Florian Ragwitz for Perl tips.

Thanks a lot.  I don't know if it is a good time, with git being in
feature freeze, and GSoC project about adding caching to gitweb in the
works, but I'll take this patch and resend it if needed.  I'll try to
comment on it soon.

That said, I think that the subject (oneline commit summary) should
include 'gitweb', e.g.

  "gitweb: Optional grouping of projects by category"

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* [PATCH] Make use of stat.ctime configurable
From: Alex Riesen @ 2008-07-26 15:38 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Linus Torvalds, Junio C Hamano, Johannes Sixt, git
In-Reply-To: <alpine.DEB.1.00.0807260256030.11976@eeepc-johanness>

because there are situations where it produces too much false
positives. Like when file system crawlers keep changing it when
scanning and using the ctime for marking scanned files.

The default is to allow use of ctime.

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
Johannes Schindelin, Sat, Jul 26, 2008 02:57:25 +0200:
> On Fri, 25 Jul 2008, Alex Riesen wrote:
> > But, given the fact, that somewhere sometimes its very-very annoying to 
> > have even one (un)changed file, something must be done about it. Maybe 
> > just direct
> > 
> > [...]
> > 	trustctime = false
> 
> ... which is all Junio and I were asking all along: a separate way to ask 
> for ignoring ctime; not just DWIM it on top of the executable bit.

Oh...

 cache.h       |    1 +
 config.c      |    4 ++++
 environment.c |    1 +
 read-cache.c  |    2 +-
 4 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/cache.h b/cache.h
index 38985aa..00d02d3 100644
--- a/cache.h
+++ b/cache.h
@@ -421,6 +421,7 @@ extern int delete_ref(const char *, const unsigned char *sha1);
 
 /* Environment bits from configuration mechanism */
 extern int trust_executable_bit;
+extern int trust_file_ctime;
 extern int quote_path_fully;
 extern int has_symlinks;
 extern int ignore_case;
diff --git a/config.c b/config.c
index 1e066c7..860e8ab 100644
--- a/config.c
+++ b/config.c
@@ -341,6 +341,10 @@ static int git_default_core_config(const char *var, const char *value)
 		trust_executable_bit = git_config_bool(var, value);
 		return 0;
 	}
+	if (!strcmp(var, "core.filectime")) {
+		trust_file_ctime = git_config_bool(var, value);
+		return 0;
+	}
 
 	if (!strcmp(var, "core.quotepath")) {
 		quote_path_fully = git_config_bool(var, value);
diff --git a/environment.c b/environment.c
index 4a88a17..4982771 100644
--- a/environment.c
+++ b/environment.c
@@ -13,6 +13,7 @@ char git_default_email[MAX_GITNAME];
 char git_default_name[MAX_GITNAME];
 int user_ident_explicitly_given;
 int trust_executable_bit = 1;
+int trust_file_ctime = 1;
 int has_symlinks = 1;
 int ignore_case;
 int assume_unchanged;
diff --git a/read-cache.c b/read-cache.c
index a50a851..00d39dc 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -181,7 +181,7 @@ static int ce_match_stat_basic(struct cache_entry *ce, struct stat *st)
 	}
 	if (ce->ce_mtime != (unsigned int) st->st_mtime)
 		changed |= MTIME_CHANGED;
-	if (ce->ce_ctime != (unsigned int) st->st_ctime)
+	if (trust_file_ctime && ce->ce_ctime != (unsigned int) st->st_ctime)
 		changed |= CTIME_CHANGED;
 
 	if (ce->ce_uid != (unsigned int) st->st_uid ||
-- 
1.6.0.rc0.76.g581e

^ permalink raw reply related

* [PATCH] Documentation: clarify diff.external limitations
From: Anders Melchiorsen @ 2008-07-27 11:12 UTC (permalink / raw)
  To: git; +Cc: gitster, Anders Melchiorsen

The diff.external examples pass a flag to gnu-diff, but that is not
actually supported.

Without the flag, diff will still complain about an extra operand
because git is passing 7 parameters to the external command.

Both of these are fixed by suggesting a diff-wrapper and pointing to
the description of the parameters passed.

Signed-off-by: Anders Melchiorsen <mail@cup.kalibalik.dk>
---
 Documentation/config.txt     |   10 ++++++----
 Documentation/git-config.txt |    2 +-
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 798b551..1a13abc 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -92,7 +92,7 @@ Example
 
 	# Our diff algorithm
 	[diff]
-		external = "/usr/local/bin/gnu-diff -u"
+		external = /usr/local/bin/diff-wrapper
 		renames = true
 
 	[branch "devel"]
@@ -563,9 +563,11 @@ diff.autorefreshindex::
 diff.external::
 	If this config variable is set, diff generation is not
 	performed using the internal diff machinery, but using the
-	given command.  Note: if you want to use an external diff
-	program only on a subset of your files, you might want to
-	use linkgit:gitattributes[5] instead.
+	given command.  Can be overridden with the `GIT_EXTERNAL_DIFF'
+	environment variable.  The command is called with parameters
+	as described under "git Diffs" in linkgit:git[1].  Note: if
+	you want to use an external diff program only on a subset of
+	your files, you	might want to use linkgit:gitattributes[5] instead.
 
 diff.renameLimit::
 	The number of files to consider when performing the copy/rename
diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 697824c..28e1861 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -222,7 +222,7 @@ Given a .git/config like this:
 
 	; Our diff algorithm
 	[diff]
-		external = "/usr/local/bin/gnu-diff -u"
+		external = /usr/local/bin/diff-wrapper
 		renames = true
 
 	; Proxy settings
-- 
1.5.4.3

^ permalink raw reply related

* Re: [RFC] Git User's Survey 2008
From: Nguyen Thai Ngoc Duy @ 2008-07-27 11:24 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Stephan Beyer, Robin Rosenberg, Johannes Schindelin, git
In-Reply-To: <200807261734.35239.jnareb@gmail.com>

On 7/26/08, Jakub Narebski <jnareb@gmail.com> wrote:
> On Thu, 24 July 2008, Jakub Narębski wrote:
>
> > On Thu, 24 July 2008, Stephan Beyer wrote:
>  >> Jakub Narebski wrote:
>  >>> Dnia środa 23. lipca 2008 16:54, Robin Rosenberg napisał
>  >>>> onsdagen den 23 juli 2008 15.18.40 skrev Johannes Schindelin:
>  >>>>> On Wed, 23 Jul 2008, Jakub Narebski wrote:
>  >>>>>> On Wed, 23 Jul 2008, Johannes Schindelin wrote:
>  >>>>>>> On Wed, 23 Jul 2008, Jakub Narebski wrote:
>  >>>>>>>
>  >>>>>>>>    04. Which programming languages you are proficient with?
>  >>>>>>>>        (The choices include programming languages used by git)
>  >>>>>>>>        (zero or more: multiple choice)
>  >>>>>>>>      - C, shell, Perl, Python, Tcl/Tk
>  >>>>>>>>      + (should we include other languages, like C++, Java, PHP,
>  >>>>>>>>         Ruby,...?)
>  >> [...]
>  >
>
> > If we want to provide larger number of programming languages to
>  > chose from (with "other" as fallback), we could take for example
>  > top 10 from the TIOBE index, or similar sites:
>  >   http://www.tiobe.com/index.php/content/paperinfo/tpci/index.html (for July 2008)
>  >   http://lui.arbingersys.com/index.html (Language Usage Indicators, Jul 10, 2008)
>  >
>  > This would bring 'Visual Basic', and perhaps 'Assembly' and 'Lisp'
>  > to the list of choices.
>
>
> Perhaps also consider GitHub's list of most popular languages
>   http://github.com/blog/99-popular-languages
>  (as mentioned in Petr 'Pasky' Baudis somewhere in git-scm.com thread)
>  to take into account git popularity among web developers.
>
>  This would add 'ERB' (or is it just subset of 'Ruby' as eRuby
>  implementation?), and 'Common Lisp' (if 'Common Lisp', then
>  probably also 'Scheme'/'Guile').

I have an impression that this 'erb' is a file extension for html file
with embedding ruby, used in ruby on rails. If so it's not worth
adding. If you are going to add Scheme, please remove Guile or you
have to list other Scheme implementations too.

>  There is always free-form 'other'...
>
> --
>  Jakub Narebski
>  Poland
>  --
>  To unsubscribe from this list: send the line "unsubscribe git" in
>  the body of a message to majordomo@vger.kernel.org
>  More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


-- 
Duy

^ permalink raw reply

* Re: git-scm.com
From: Petr Baudis @ 2008-07-27 11:37 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Scott Chacon, git
In-Reply-To: <7vsktwfu5z.fsf@gitster.siamese.dyndns.org>

On Sat, Jul 26, 2008 at 10:10:32AM -0700, Junio C Hamano wrote:
> "Scott Chacon" <schacon@gmail.com> writes:
> 
> > On Fri, Jul 25, 2008 at 4:47 PM, Junio C Hamano <gitster@pobox.com> wrote:
> >> ...
> >> I find a tabular list like this list easier to read if it were sorted like
> >> this:
> >>
> >>        A       D       G
> >>        B       E       H
> >>        C       F
> >> ...
> >
> > I fixed the things you mentioned here, except for the list ordering,
> > only because I kinda think you big contributors should be at the top
> > there,...
> 
> If you are going to list 30 or so top contributors in 8 rows times 4
> columns, because visually the columns are much more distinct than the
> rows, it makes the result look more sorted.  This is the same reasoning
> hwo "git help --all" was fixed with 112d0ba (Make "git help" sort git
> commands in columns, 2005-12-18).

Actually, this is strange for me: I would never think about reading git
help --all by rows, and I would never think about reading the authors
list by columns! It's difficult for me to point out why, possibly
because the authors list has less items per row and the items are longer
(and multi-word), but that's just a speculation. Maybe cultural
background (Japanese books are written in columns, right?) plays some
role too, I don't know.

> The purpose of the list would most likely not to find somebody with high
> activity to contact for help (you would use the top list that is sorted by
> the commit count for that kind of thing).  It would primarily be to give
> credit to everybody, and perhaps so that people on the contributor list
> can point at their own name and say "I helped them", or find somebody else
> they happen to know in the list.
> 
> When a contributor used to have 8 commits and then adds 2 commits, that
> would move the name in the list by a dozen places or so with the current
> set of contributors.  It would be much easier to locate one's own name
> among a huge list if the names are alphabetically sorted, not by commit
> count.  When more people start to contribute, your name does not move so
> drastically.  If you are Adam, you are likely to find yourself near the
> beginning of the list, if you are Scott, you are likely to find yourself
> near one fourth from the end of the list.

I don't think locating is any issue; the find function of browser is
very easy to use nowadays. I guess the purpose of the list would be
to show "I helped them this much" (i.e. "I'm high on the list"). I think
this would actually motivate contributors to move up in the ladder -
people are competitive; you might get wary about this kind of
motivation, but I believe that it is no bad thing, inherently. Heck, I
admit it does motivate even me a little, safely in the "Primary Authors"
section. :-) (These guys with their tools merged into git have unfair
advantage! You should add up also, uh, git-homepage, cogito and, um...
repo.git! *baby cry*</vanity>)

> And for the "giving credit" purpose, I do not think truncating the list at
> 5 commits or less threshold, as suggested earlier and already done, makes
> much sense, either.

The point here is that the list is awfully long and also can contain
a lot of duplicates or people with broken unicode, etc. - it gets hard
to maintain, and it makes the about page too long. I would be of course
fine with a tiny link at the bottom "(show all contributors)".

-- 
				Petr "Pasky" Baudis
As in certain cults it is possible to kill a process if you know
its true name.  -- Ken Thompson and Dennis M. Ritchie

^ permalink raw reply

* Re: Official Git Homepage change? Re: git-scm.com
From: Petr Baudis @ 2008-07-27 12:22 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, Scott Chacon, git
In-Reply-To: <7v8wvok3f6.fsf@gitster.siamese.dyndns.org>

On Sat, Jul 26, 2008 at 09:37:01AM -0700, Junio C Hamano wrote:
> Petr Baudis <pasky@suse.cz> writes:
> 
> >   I don't know if this should have any immediate effect on how we
> > develop Git etc., but I think it is good to be aware of the fact that
> > silently, huge amount of "dark mass" Git projects is accumulating and
> > that Git is making headways in areas many of us were little aware of.
> 
> The developer community and "dark matter" community are totally separate
> entities that do not interact with each other very much, and they will go
> their separate ways?  I think it is inevitable for any project once it
> becomes popular enough.

  I don't think this is inevitable. I think we are getting into this
position two few simple reasons:

  (i) The traffic on the main list is simply too high for regular users
to keep up with. If we care to get more in touch with our users,
the solution might be to spread the word about the Git Users Google
Group more, and monitor it more actively.

  (ii) The tone on the mailing list seems frequently unnecessarily
harsh. This was mentioned by some of the "dark matter" people (not Scott
himself) as the reason why didn't they announce their work on the
mailing list; fear of being flamed. Especially at the beginning of
summer when I "returned" after quite a while of inactivity, I perceived
this rather unfriendly tone rather strongly as well (not at me
personally, but reading replies to other people's contributions), though
I got kind of used to it quickly again. If we care to encourage postings
by "external" developers to the mailing list, we should keep the usual
strength of our criticism, but try to make the tone more encouraging.

> Where does this observation lead us in this "Official" git homepage
> discussion?  Perhaps the conclusion would be that there does not have to
> be any _single_ official home?  I dunno.

  That is not good idea; this would only split the community further,
*and* create confusion as some people would be directed to homepage A,
others to homepage B, each would have its resources kept up-to-date
in different manner, ...

  Also, we need someplace to link at from git itself:

	README:Many Git online resources are accessible from http://git.or.cz/
	gitweb/gitweb.perl:our $logo_url = "http://git.or.cz/";

In case of README, we could add another link easily, in case of gitweb,
much less so.

  This ultimately comes down to what address would *you* write on
a piece of paper if someone walked to you on say, some conference
and asked you "I like Git, where can I learn more?" Or you could start
explaining how Git does not have a single homepage and start writing
multiple URLs noting the differences. Would that make good impression?

-- 
				Petr "Pasky" Baudis
As in certain cults it is possible to kill a process if you know
its true name.  -- Ken Thompson and Dennis M. Ritchie

^ permalink raw reply

* Re: Official Git Homepage change? Re: git-scm.com
From: Petr Baudis @ 2008-07-27 12:35 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Scott Chacon, git
In-Reply-To: <7v4p6dnv5k.fsf@gitster.siamese.dyndns.org>

On Fri, Jul 25, 2008 at 09:09:43PM -0700, Junio C Hamano wrote:
> For me personally, pages outside the wiki have never felt like "the
> official git homepage", not because the contents you prepared were
> inadequate, but because I did not see much community participation to help
> enrich it.

I agree; I think this might have been in part because the Wiki was so
easy to use and change; I did receive some patches, but not any
overwhelming amount.

> So I wish the new site success, but the definition of success from my
> point of view is not how many random visitors it will attract, but how
> well the site makes the contributors (both to git software itself, and to
> the site's contents) feel welcomed.  Maybe in time it will become
> successful enough by _my_ definition of success, and I may recommend
> kernel.org folks to point at it from http://git.kernel.org/ (link with
> text "overview") if/when that happens, and I may start mentioning them in
> the "Note".  We'll see.

The question is, however,is whether to make the _current_ overview link
target simply alias at the new site _now_, though. :-) It seems to be
a waste to maintain two websites in parallel, as well as actively harmful
and confusing for people interested in Git, as I tried to explain in
my other mail. That's why this subthread was born.

> >   The negatives section writeup is longer, but in fact I think the
> > positives win here; I also have a bit of bad conscience about not giving
> > git.or.cz the amount of time it would deserve...
> 
> Let me thank you for maintaining not just git.or.cz/ but also repo.or.cz/
> and the wiki.

Thanks, I appreciate this, though pretty much all of this sort of popped
up simply because I had root access to a bored server. ;-) Especially
wrt. the wiki, I think it's mainly Jakub Narebski who is keeping it
together content-wise and keeps bugging me if it falls apart
technically.

-- 
				Petr "Pasky" Baudis
As in certain cults it is possible to kill a process if you know
its true name.  -- Ken Thompson and Dennis M. Ritchie

^ permalink raw reply

* Re: [PATCHv2] git-mv: Keep moved index entries inact
From: Petr Baudis @ 2008-07-27 13:41 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v8wvpm9cl.fsf@gitster.siamese.dyndns.org>

On Fri, Jul 25, 2008 at 11:46:02PM -0700, Junio C Hamano wrote:
> Thanks.  I think I've managed to fix the rename_index_entry_at() in a
> satisfactory way, and also made builtin-mv to allow "mv -f symlink file"
> and "mv -f file symlink".

Oh, sorry, I didn't realize there were still problems with the original
one, I would try it on my own in that case.

> So my take on the above test piece is that after:
> 
> 	>moved
> 	mkdir dir
>         >dir/file
>         ln -s dir symlink
> 	git add moved dir symlink
> 
> This should fail, as it is an overwrite:
> 
> 	git mv moved symlink
> 
> and with "-f", the command should simply remove symlink and replace it
> with a regular file whose contents come from the original "moved".
> 
> IOW, what a symlink points at should not matter.

You convinced me, yes. (Especially since I started actually using
symlinks in some of my projects very recently and this would be the
exact semantic I would eventually expect as well.)

-- 
				Petr "Pasky" Baudis
As in certain cults it is possible to kill a process if you know
its true name.  -- Ken Thompson and Dennis M. Ritchie

^ permalink raw reply

* [PATCH] t/t7001-mv.sh: Propose ability to use git-mv on conflicting entries
From: Petr Baudis @ 2008-07-27 13:47 UTC (permalink / raw)
  To: git; +Cc: gitster
In-Reply-To: <20080727134142.GA10151@machine.or.cz>

Currently, git-mv will declare "not under source control" on an attempt
to move a conflicted index entry. This patch adds an expect_failure
testcase for this case, since this is an artificial restriction. (However,
the scenario is not critical enough for the author to fix right now.)

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

I don't really know if it is ok to make "feature requests" like this by
adding failing testcases...

 t/t7001-mv.sh |   27 +++++++++++++++++++++++++++
 1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
index 7e47931..241e9a2 100755
--- a/t/t7001-mv.sh
+++ b/t/t7001-mv.sh
@@ -173,6 +173,33 @@ test_expect_success 'git mv should not change sha1 of moved cache entry' '
 
 rm -f dirty dirty2
 
+cat >multistage <<EOT
+100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 1	staged
+100755 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 2	staged
+100755 d00491fd7e5bb6fa28c517a0bb32b8b506539d4d 3	staged
+EOT
+
+# Rationale: I cannot git mv around a conflicted file. This is unnecessary
+# restriction in case another part of conflict resolution requires me to
+# move the file around.
+test_expect_failure 'git mv should move all stages of cache entry' '
+
+	rm -fr .git &&
+	git init &&
+	# git mv requires object to exist in working tree (bug?)
+	touch staged &&
+	git update-index --index-info <multistage &&
+	git ls-files --stage >lsf_output &&
+	test_cmp multistage lsf_output &&
+	git mv staged staged-mv &&
+	sed "s/staged/staged-mv/" <multistage >multistage-mv &&
+	git ls-files --stage >lsf_output &&
+	test_cmp multistage-mv lsf_output
+
+'
+
+rm -f multistage multistage-mv lsf_output staged
+
 test_expect_failure 'git mv should overwrite symlink to a file' '
 
 	rm -fr .git &&

^ permalink raw reply related

* Running git gui on Windows.
From: Jurko Gospodnetić @ 2008-07-27 13:48 UTC (permalink / raw)
  To: git

   Hi.

   Is there a way to run git gui on Windows so it does not block the 
calling process?

   For example, when I run 'git gui' from the Windows Command Prompt, I 
would like to be able to continue using that same command prompt and not 
have it blocked until I exit the started git gui process.

   I am using msysgit on Windows XP SP3 and 'git --version' states 'git 
version 1.5.6.1.1071.g76fb'.

   Many thanks.

   Best regards,
     Jurko Gospodnetić

^ permalink raw reply

* Re: Running git gui on Windows.
From: Jurko Gospodnetić @ 2008-07-27 13:55 UTC (permalink / raw)
  To: git
In-Reply-To: <g6hubi$8op$1@ger.gmane.org>

   Hi.

>   Is there a way to run git gui on Windows so it does not block the 
> calling process?
> 
>   For example, when I run 'git gui' from the Windows Command Prompt, I 
> would like to be able to continue using that same command prompt and not 
> have it blocked until I exit the started git gui process.
> 
>   I am using msysgit on Windows XP SP3 and 'git --version' states 'git 
> version 1.5.6.1.1071.g76fb'.

   I now realized my question did not say exactly what I intended it to. 
I know I can start 'git gui' using:

   start "" /b cmd /c git gui

   from the command prompt and get the desired effect. I was wondering 
why git gui does not do this in the first place and whether it could be 
modified so that this is the default behaviour?

   Best regards,
     Jurko Gospodnetić

^ permalink raw reply

* Re: [RFC/PATCH v2] merge-base: teach "git merge-base" to accept more than 2 arguments
From: Johannes Schindelin @ 2008-07-27 14:38 UTC (permalink / raw)
  To: Christian Couder; +Cc: git, Junio C Hamano, Miklos Vajna
In-Reply-To: <20080727053324.b54fe48e.chriscool@tuxfamily.org>

Hi,

On Sun, 27 Jul 2008, Christian Couder wrote:

> diff --git a/builtin-merge-base.c b/builtin-merge-base.c
> index 1cb2925..f2c9756 100644
> --- a/builtin-merge-base.c
> +++ b/builtin-merge-base.c
> @@ -38,15 +48,22 @@ int cmd_merge_base(int argc, const char **argv, const char *prefix)
>  			usage(merge_base_usage);
>  		argc--; argv++;
>  	}
> -	if (argc != 3)
> +	if (argc < 3)
>  		usage(merge_base_usage);
> -	if (get_sha1(argv[1], rev1key))
> -		die("Not a valid object name %s", argv[1]);
> -	if (get_sha1(argv[2], rev2key))
> -		die("Not a valid object name %s", argv[2]);
> -	rev1 = lookup_commit_reference(rev1key);
> -	rev2 = lookup_commit_reference(rev2key);
> -	if (!rev1 || !rev2)
> +
> +	rev1 = get_commit_reference(argv[1]);
> +	if (!rev1)
>  		return 1;

Why do you special case rev1?  Is it so special?  Just handle it together 
with all of the other arguments!

IOW have one commit array, and do not call it "prev".

> -	return show_merge_base(rev1, rev2, show_all);
> +	argc--; argv++;
> +
> +	do {
> +		struct commit *rev2 = get_commit_reference(argv[1]);
> +		if (!rev2)
> +			return 1;
> +		ALLOC_GROW(prev2, prev2_nr + 1, prev2_alloc);
> +		prev2[prev2_nr++] = rev2;
> +		argc--; argv++;
> +	} while (argc > 1);

Now, this is ugly.  You know beforehand the _exact_ number of arguments, 
and yet you dynamically grow the array?  Also, why do you use a do { } 
while(), when a for () would be much, much clearer?

BTW I seem to recall that get_merge_bases_many() was _not_ the same as 
get_merge_octopus().  Could you please remind me what _many() does?

Ciao,
Dscho

^ permalink raw reply


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