* [PATCH] diff-delta.c: rename {a,}{entry,hash} to {,u}{entry,hash}
From: David Kastrup @ 2007-09-08 8:54 UTC (permalink / raw)
To: git
In-Reply-To: <0cd39105dcd57a60eca290db598613aafcc8c577.1189243702.git.dak@gnu.org>
The variables for the packed entries are now called just entry and
hash rather than aentry+ahash, and those for the unpacked entries have
been renamed to uentry and uhash from the original entry and hash.
While this makes the diff to the unchanged code larger, it matches the
type declarations better.
Signed-off-by: David Kastrup <dak@gnu.org>
---
diff-delta.c | 65 ++++++++++++++++++++++++++++-----------------------------
1 files changed, 32 insertions(+), 33 deletions(-)
diff --git a/diff-delta.c b/diff-delta.c
index 1b4b1c1..e7c33aa 100644
--- a/diff-delta.c
+++ b/diff-delta.c
@@ -135,8 +135,8 @@ struct delta_index * create_delta_index(const void *buf, unsigned long bufsize)
unsigned int i, hsize, hmask, entries, prev_val, *hash_count;
const unsigned char *data, *buffer = buf;
struct delta_index *index;
- struct unpacked_index_entry *entry, **hash;
- struct index_entry *aentry, **ahash;
+ struct unpacked_index_entry *uentry, **uhash;
+ struct index_entry *entry, **hash;
void *mem;
unsigned long memsize;
@@ -153,21 +153,21 @@ struct delta_index * create_delta_index(const void *buf, unsigned long bufsize)
hmask = hsize - 1;
/* allocate lookup index */
- memsize = sizeof(*hash) * hsize +
- sizeof(*entry) * entries;
+ memsize = sizeof(*uhash) * hsize +
+ sizeof(*uentry) * entries;
mem = malloc(memsize);
if (!mem)
return NULL;
- hash = mem;
- mem = hash + hsize;
- entry = mem;
+ uhash = mem;
+ mem = uhash + hsize;
+ uentry = mem;
- memset(hash, 0, hsize * sizeof(*hash));
+ memset(uhash, 0, hsize * sizeof(*uhash));
/* allocate an array to count hash entries */
hash_count = calloc(hsize, sizeof(*hash_count));
if (!hash_count) {
- free(hash);
+ free(uhash);
return NULL;
}
@@ -181,15 +181,15 @@ struct delta_index * create_delta_index(const void *buf, unsigned long bufsize)
val = ((val << 8) | data[i]) ^ T[val >> RABIN_SHIFT];
if (val == prev_val) {
/* keep the lowest of consecutive identical blocks */
- entry[-1].entry.ptr = data + RABIN_WINDOW;
+ uentry[-1].entry.ptr = data + RABIN_WINDOW;
--entries;
} else {
prev_val = val;
i = val & hmask;
- entry->entry.ptr = data + RABIN_WINDOW;
- entry->entry.val = val;
- entry->next = hash[i];
- hash[i] = entry++;
+ uentry->entry.ptr = data + RABIN_WINDOW;
+ uentry->entry.val = val;
+ uentry->next = uhash[i];
+ uhash[i] = uentry++;
hash_count[i]++;
}
}
@@ -209,17 +209,17 @@ struct delta_index * create_delta_index(const void *buf, unsigned long bufsize)
for (i = 0; i < hsize; i++) {
if (hash_count[i] < HASH_LIMIT)
continue;
- entry = hash[i];
+ uentry = uhash[i];
do {
- struct unpacked_index_entry *keep = entry;
+ struct unpacked_index_entry *keep = uentry;
int skip = hash_count[i] / HASH_LIMIT;
do {
--entries;
- entry = entry->next;
- } while(--skip && entry);
+ uentry = uentry->next;
+ } while(--skip && uentry);
++entries;
- keep->next = entry;
- } while(entry);
+ keep->next = uentry;
+ } while(uentry);
}
free(hash_count);
@@ -227,13 +227,12 @@ struct delta_index * create_delta_index(const void *buf, unsigned long bufsize)
* linked lists */
memsize = sizeof(*index)
- + sizeof(*ahash) * (hsize+1)
- + sizeof(*aentry) * entries;
-
+ + sizeof(*hash) * (hsize+1)
+ + sizeof(*entry) * entries;
mem = malloc(memsize);
if (!mem) {
- free(hash);
+ free(uhash);
return NULL;
}
@@ -244,25 +243,25 @@ struct delta_index * create_delta_index(const void *buf, unsigned long bufsize)
index->hash_mask = hmask;
mem = index + 1;
- ahash = mem;
- mem = ahash + (hsize+1);
- aentry = mem;
+ hash = mem;
+ mem = hash + (hsize+1);
+ entry = mem;
/* Coalesce all entries belonging to one linked list into
* consecutive array entries */
for (i = 0; i < hsize; i++) {
- ahash[i] = aentry;
- for (entry = hash[i]; entry; entry = entry->next)
- *aentry++ = entry->entry;
+ hash[i] = entry;
+ for (uentry = uhash[i]; uentry; uentry = uentry->next)
+ *entry++ = uentry->entry;
}
/* Sentinel value to indicate the length of the last hash
* bucket */
- ahash[hsize] = aentry;
- assert(aentry - (struct index_entry *)mem == entries);
- free(hash);
+ hash[hsize] = entry;
+ assert(entry - (struct index_entry *)mem == entries);
+ free(uhash);
return index;
}
--
1.5.3.GIT
^ permalink raw reply related
* git-gui: fix selection of fonts under X11
From: Simon Sasburg @ 2007-09-08 9:14 UTC (permalink / raw)
To: spearce; +Cc: git
This patch fixes selection of fonts under X11 by using combobox widget
instead of tk_optionmenu widget
The tk_optionmenu widget previously used has a bug in X11 that
prevents you from scrolling it, and thus prevents you from selecting
fonts beyond those that fit on the screen.
We work around this by using a 3rd party combobox widget that does
allow scrolling.
I'm not too sure this is the right solution (using 3rd party widget),
but it does work, and it does solve the problem.
The combobox widget comes from here:
http://www1.clearlight.com/~oakley/tcl/combobox/index.html
---
git-gui/lib/combobox.tcl | 2188 ++++++++++++++++++++++++++++++++++++++++++++++
git-gui/lib/option.tcl | 19 +-
2 files changed, 2204 insertions(+), 3 deletions(-)
create mode 100755 git-gui/lib/combobox.tcl
diff --git a/git-gui/lib/combobox.tcl b/git-gui/lib/combobox.tcl
new file mode 100755
index 0000000..7e11d39
--- /dev/null
+++ b/git-gui/lib/combobox.tcl
@@ -0,0 +1,2188 @@
+# Copyright (c) 1998-2003, Bryan Oakley
+# All Rights Reservered
+#
+# Bryan Oakley
+# oakley@bardo.clearlight.com
+#
+# combobox v2.3 August 16, 2003
+#
+# a combobox / dropdown listbox (pick your favorite name) widget
+# written in pure tcl
+#
+# this code is freely distributable without restriction, but is
+# provided as-is with no warranty expressed or implied.
+#
+# thanks to the following people who provided beta test support or
+# patches to the code (in no particular order):
+#
+# Scott Beasley Alexandre Ferrieux Todd Helfter
+# Matt Gushee Laurent Duperval John Jackson
+# Fred Rapp Christopher Nelson
+# Eric Galluzzo Jean-Francois Moine Oliver Bienert
+#
+# A special thanks to Martin M. Hunt who provided several good ideas,
+# and always with a patch to implement them. Jean-Francois Moine,
+# Todd Helfter and John Jackson were also kind enough to send in some
+# code patches.
+#
+# ... and many others over the years.
+
+package require Tk 8.0
+package provide combobox 2.3
+
+namespace eval ::combobox {
+
+ # this is the public interface
+ namespace export combobox
+
+ # these contain references to available options
+ variable widgetOptions
+
+ # these contain references to available commands and subcommands
+ variable widgetCommands
+ variable scanCommands
+ variable listCommands
+}
+
+# ::combobox::combobox --
+#
+# This is the command that gets exported. It creates a new
+# combobox widget.
+#
+# Arguments:
+#
+# w path of new widget to create
+# args additional option/value pairs (eg: -background white, etc.)
+#
+# Results:
+#
+# It creates the widget and sets up all of the default bindings
+#
+# Returns:
+#
+# The name of the newly create widget
+
+proc ::combobox::combobox {w args} {
+ variable widgetOptions
+ variable widgetCommands
+ variable scanCommands
+ variable listCommands
+
+ # perform a one time initialization
+ if {![info exists widgetOptions]} {
+ Init
+ }
+
+ # build it...
+ eval Build $w $args
+
+ # set some bindings...
+ SetBindings $w
+
+ # and we are done!
+ return $w
+}
+
+
+# ::combobox::Init --
+#
+# Initialize the namespace variables. This should only be called
+# once, immediately prior to creating the first instance of the
+# widget
+#
+# Arguments:
+#
+# none
+#
+# Results:
+#
+# All state variables are set to their default values; all of
+# the option database entries will exist.
+#
+# Returns:
+#
+# empty string
+
+proc ::combobox::Init {} {
+ variable widgetOptions
+ variable widgetCommands
+ variable scanCommands
+ variable listCommands
+ variable defaultEntryCursor
+
+ array set widgetOptions [list \
+ -background {background Background} \
+ -bd -borderwidth \
+ -bg -background \
+ -borderwidth {borderWidth BorderWidth} \
+ -buttonbackground {buttonBackground Background} \
+ -command {command Command} \
+ -commandstate {commandState State} \
+ -cursor {cursor Cursor} \
+ -disabledbackground {disabledBackground DisabledBackground} \
+ -disabledforeground {disabledForeground DisabledForeground} \
+ -dropdownwidth {dropdownWidth DropdownWidth} \
+ -editable {editable Editable} \
+ -elementborderwidth {elementBorderWidth BorderWidth} \
+ -fg -foreground \
+ -font {font Font} \
+ -foreground {foreground Foreground} \
+ -height {height Height} \
+ -highlightbackground {highlightBackground HighlightBackground} \
+ -highlightcolor {highlightColor HighlightColor} \
+ -highlightthickness {highlightThickness HighlightThickness} \
+ -image {image Image} \
+ -listvar {listVariable Variable} \
+ -maxheight {maxHeight Height} \
+ -opencommand {opencommand Command} \
+ -relief {relief Relief} \
+ -selectbackground {selectBackground Foreground} \
+ -selectborderwidth {selectBorderWidth BorderWidth} \
+ -selectforeground {selectForeground Background} \
+ -state {state State} \
+ -takefocus {takeFocus TakeFocus} \
+ -textvariable {textVariable Variable} \
+ -value {value Value} \
+ -width {width Width} \
+ -xscrollcommand {xScrollCommand ScrollCommand} \
+ ]
+
+
+ set widgetCommands [list \
+ bbox cget configure curselection \
+ delete get icursor index \
+ insert list scan selection \
+ xview select toggle open \
+ close subwidget \
+ ]
+
+ set listCommands [list \
+ delete get \
+ index insert size \
+ ]
+
+ set scanCommands [list mark dragto]
+
+ # why check for the Tk package? This lets us be sourced into
+ # an interpreter that doesn't have Tk loaded, such as the slave
+ # interpreter used by pkg_mkIndex. In theory it should have no
+ # side effects when run
+ if {[lsearch -exact [package names] "Tk"] != -1} {
+
+ ##################################################################
+ #- this initializes the option database. Kinda gross, but it works
+ #- (I think).
+ ##################################################################
+
+ # the image used for the button...
+ if {$::tcl_platform(platform) == "windows"} {
+ image create bitmap ::combobox::bimage -data {
+ #define down_arrow_width 12
+ #define down_arrow_height 12
+ static char down_arrow_bits[] = {
+ 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,
+ 0xfc,0xf1,0xf8,0xf0,0x70,0xf0,0x20,0xf0,
+ 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00;
+ }
+ }
+ } else {
+ image create bitmap ::combobox::bimage -data {
+ #define down_arrow_width 15
+ #define down_arrow_height 15
+ static char down_arrow_bits[] = {
+ 0x00,0x80,0x00,0x80,0x00,0x80,0x00,0x80,
+ 0x00,0x80,0xf8,0x8f,0xf0,0x87,0xe0,0x83,
+ 0xc0,0x81,0x80,0x80,0x00,0x80,0x00,0x80,
+ 0x00,0x80,0x00,0x80,0x00,0x80
+ }
+ }
+ }
+
+ # compute a widget name we can use to create a temporary widget
+ set tmpWidget ".__tmp__"
+ set count 0
+ while {[winfo exists $tmpWidget] == 1} {
+ set tmpWidget ".__tmp__$count"
+ incr count
+ }
+
+ # get the scrollbar width. Because we try to be clever and draw our
+ # own button instead of using a tk widget, we need to know what size
+ # button to create. This little hack tells us the width of a scroll
+ # bar.
+ #
+ # NB: we need to be sure and pick a window that doesn't already
+ # exist...
+ scrollbar $tmpWidget
+ set sb_width [winfo reqwidth $tmpWidget]
+ set bbg [$tmpWidget cget -background]
+ destroy $tmpWidget
+
+ # steal options from the entry widget
+ # we want darn near all options, so we'll go ahead and do
+ # them all. No harm done in adding the one or two that we
+ # don't use.
+ entry $tmpWidget
+ foreach foo [$tmpWidget configure] {
+ # the cursor option is special, so we'll save it in
+ # a special way
+ if {[lindex $foo 0] == "-cursor"} {
+ set defaultEntryCursor [lindex $foo 4]
+ }
+ if {[llength $foo] == 5} {
+ set option [lindex $foo 1]
+ set value [lindex $foo 4]
+ option add *Combobox.$option $value widgetDefault
+
+ # these options also apply to the dropdown listbox
+ if {[string compare $option "foreground"] == 0 \
+ || [string compare $option "background"] == 0 \
+ || [string compare $option "font"] == 0} {
+ option add *Combobox*ComboboxListbox.$option $value \
+ widgetDefault
+ }
+ }
+ }
+ destroy $tmpWidget
+
+ # these are unique to us...
+ option add *Combobox.elementBorderWidth 1 widgetDefault
+ option add *Combobox.buttonBackground $bbg widgetDefault
+ option add *Combobox.dropdownWidth {} widgetDefault
+ option add *Combobox.openCommand {} widgetDefault
+ option add *Combobox.cursor {} widgetDefault
+ option add *Combobox.commandState normal widgetDefault
+ option add *Combobox.editable 1 widgetDefault
+ option add *Combobox.maxHeight 10 widgetDefault
+ option add *Combobox.height 0
+ }
+
+ # set class bindings
+ SetClassBindings
+}
+
+# ::combobox::SetClassBindings --
+#
+# Sets up the default bindings for the widget class
+#
+# this proc exists since it's The Right Thing To Do, but
+# I haven't had the time to figure out how to do all the
+# binding stuff on a class level. The main problem is that
+# the entry widget must have focus for the insertion cursor
+# to be visible. So, I either have to have the entry widget
+# have the Combobox bindtag, or do some fancy juggling of
+# events or some such. What a pain.
+#
+# Arguments:
+#
+# none
+#
+# Returns:
+#
+# empty string
+
+proc ::combobox::SetClassBindings {} {
+
+ # make sure we clean up after ourselves...
+ bind Combobox <Destroy> [list ::combobox::DestroyHandler %W]
+
+ # this will (hopefully) close (and lose the grab on) the
+ # listbox if the user clicks anywhere outside of it. Note
+ # that on Windows, you can click on some other app and
+ # the listbox will still be there, because tcl won't see
+ # that button click
+ set this {[::combobox::convert %W -W]}
+ bind Combobox <Any-ButtonPress> "$this close"
+ bind Combobox <Any-ButtonRelease> "$this close"
+
+ # this helps (but doesn't fully solve) focus issues. The general
+ # idea is, whenever the frame gets focus it gets passed on to
+ # the entry widget
+ bind Combobox <FocusIn> {::combobox::tkTabToWindow \
+ [::combobox::convert %W -W].entry}
+
+ # this closes the listbox if we get hidden
+ bind Combobox <Unmap> {[::combobox::convert %W -W] close}
+
+ return ""
+}
+
+# ::combobox::SetBindings --
+#
+# here's where we do most of the binding foo. I think there's probably
+# a few bindings I ought to add that I just haven't thought
+# about...
+#
+# I'm not convinced these are the proper bindings. Ideally all
+# bindings should be on "Combobox", but because of my juggling of
+# bindtags I'm not convinced thats what I want to do. But, it all
+# seems to work, its just not as robust as it could be.
+#
+# Arguments:
+#
+# w widget pathname
+#
+# Returns:
+#
+# empty string
+
+proc ::combobox::SetBindings {w} {
+ upvar ::combobox::${w}::widgets widgets
+ upvar ::combobox::${w}::options options
+
+ # juggle the bindtags. The basic idea here is to associate the
+ # widget name with the entry widget, so if a user does a bind
+ # on the combobox it will get handled properly since it is
+ # the entry widget that has keyboard focus.
+ bindtags $widgets(entry) \
+ [concat $widgets(this) [bindtags $widgets(entry)]]
+
+ bindtags $widgets(button) \
+ [concat $widgets(this) [bindtags $widgets(button)]]
+
+ # override the default bindings for tab and shift-tab. The
+ # focus procs take a widget as their only parameter and we
+ # want to make sure the right window gets used (for shift-
+ # tab we want it to appear as if the event was generated
+ # on the frame rather than the entry.
+ bind $widgets(entry) <Tab> \
+ "::combobox::tkTabToWindow \[tk_focusNext $widgets(entry)\]; break"
+ bind $widgets(entry) <Shift-Tab> \
+ "::combobox::tkTabToWindow \[tk_focusPrev $widgets(this)\]; break"
+
+ # this makes our "button" (which is actually a label)
+ # do the right thing
+ bind $widgets(button) <ButtonPress-1> [list $widgets(this) toggle]
+
+ # this lets the autoscan of the listbox work, even if they
+ # move the cursor over the entry widget.
+ bind $widgets(entry) <B1-Enter> "break"
+
+ bind $widgets(listbox) <ButtonRelease-1> \
+ "::combobox::Select [list $widgets(this)] \
+ \[$widgets(listbox) nearest %y\]; break"
+
+ bind $widgets(vsb) <ButtonPress-1> {continue}
+ bind $widgets(vsb) <ButtonRelease-1> {continue}
+
+ bind $widgets(listbox) <Any-Motion> {
+ %W selection clear 0 end
+ %W activate @%x,%y
+ %W selection anchor @%x,%y
+ %W selection set @%x,%y @%x,%y
+ # need to do a yview if the cursor goes off the top
+ # or bottom of the window... (or do we?)
+ }
+
+ # these events need to be passed from the entry widget
+ # to the listbox, or otherwise need some sort of special
+ # handling.
+ foreach event [list <Up> <Down> <Tab> <Return> <Escape> \
+ <Next> <Prior> <Double-1> <1> <Any-KeyPress> \
+ <FocusIn> <FocusOut>] {
+ bind $widgets(entry) $event \
+ [list ::combobox::HandleEvent $widgets(this) $event]
+ }
+
+ # like the other events, <MouseWheel> needs to be passed from
+ # the entry widget to the listbox. However, in this case we
+ # need to add an additional parameter
+ catch {
+ bind $widgets(entry) <MouseWheel> \
+ [list ::combobox::HandleEvent $widgets(this) <MouseWheel> %D]
+ }
+}
+
+# ::combobox::Build --
+#
+# This does all of the work necessary to create the basic
+# combobox.
+#
+# Arguments:
+#
+# w widget name
+# args additional option/value pairs
+#
+# Results:
+#
+# Creates a new widget with the given name. Also creates a new
+# namespace patterened after the widget name, as a child namespace
+# to ::combobox
+#
+# Returns:
+#
+# the name of the widget
+
+proc ::combobox::Build {w args } {
+ variable widgetOptions
+
+ if {[winfo exists $w]} {
+ error "window name \"$w\" already exists"
+ }
+
+ # create the namespace for this instance, and define a few
+ # variables
+ namespace eval ::combobox::$w {
+
+ variable ignoreTrace 0
+ variable oldFocus {}
+ variable oldGrab {}
+ variable oldValue {}
+ variable options
+ variable this
+ variable widgets
+
+ set widgets(foo) foo ;# coerce into an array
+ set options(foo) foo ;# coerce into an array
+
+ unset widgets(foo)
+ unset options(foo)
+ }
+
+ # import the widgets and options arrays into this proc so
+ # we don't have to use fully qualified names, which is a
+ # pain.
+ upvar ::combobox::${w}::widgets widgets
+ upvar ::combobox::${w}::options options
+
+ # this is our widget -- a frame of class Combobox. Naturally,
+ # it will contain other widgets. We create it here because
+ # we need it in order to set some default options.
+ set widgets(this) [frame $w -class Combobox -takefocus 0]
+ set widgets(entry) [entry $w.entry -takefocus 1]
+ set widgets(button) [label $w.button -takefocus 0]
+
+ # this defines all of the default options. We get the
+ # values from the option database. Note that if an array
+ # value is a list of length one it is an alias to another
+ # option, so we just ignore it
+ foreach name [array names widgetOptions] {
+ if {[llength $widgetOptions($name)] == 1} continue
+
+ set optName [lindex $widgetOptions($name) 0]
+ set optClass [lindex $widgetOptions($name) 1]
+
+ set value [option get $w $optName $optClass]
+ set options($name) $value
+ }
+
+ # a couple options aren't available in earlier versions of
+ # tcl, so we'll set them to sane values. For that matter, if
+ # they exist but are empty, set them to sane values.
+ if {[string length $options(-disabledforeground)] == 0} {
+ set options(-disabledforeground) $options(-foreground)
+ }
+ if {[string length $options(-disabledbackground)] == 0} {
+ set options(-disabledbackground) $options(-background)
+ }
+
+ # if -value is set to null, we'll remove it from our
+ # local array. The assumption is, if the user sets it from
+ # the option database, they will set it to something other
+ # than null (since it's impossible to determine the difference
+ # between a null value and no value at all).
+ if {[info exists options(-value)] \
+ && [string length $options(-value)] == 0} {
+ unset options(-value)
+ }
+
+ # we will later rename the frame's widget proc to be our
+ # own custom widget proc. We need to keep track of this
+ # new name, so we'll define and store it here...
+ set widgets(frame) ::combobox::${w}::$w
+
+ # gotta do this sooner or later. Might as well do it now
+ pack $widgets(button) -side right -fill y -expand no
+ pack $widgets(entry) -side left -fill both -expand yes
+
+ # I should probably do this in a catch, but for now it's
+ # good enough... What it does, obviously, is put all of
+ # the option/values pairs into an array. Make them easier
+ # to handle later on...
+ array set options $args
+
+ # now, the dropdown list... the same renaming nonsense
+ # must go on here as well...
+ set widgets(dropdown) [toplevel $w.top]
+ set widgets(listbox) [listbox $w.top.list]
+ set widgets(vsb) [scrollbar $w.top.vsb]
+
+ pack $widgets(listbox) -side left -fill both -expand y
+
+ # fine tune the widgets based on the options (and a few
+ # arbitrary values...)
+
+ # NB: we are going to use the frame to handle the relief
+ # of the widget as a whole, so the entry widget will be
+ # flat. This makes the button which drops down the list
+ # to appear "inside" the entry widget.
+
+ $widgets(vsb) configure \
+ -borderwidth 1 \
+ -command "$widgets(listbox) yview" \
+ -highlightthickness 0
+
+ $widgets(button) configure \
+ -background $options(-buttonbackground) \
+ -highlightthickness 0 \
+ -borderwidth $options(-elementborderwidth) \
+ -relief raised \
+ -width [expr {[winfo reqwidth $widgets(vsb)] - 2}]
+
+ $widgets(entry) configure \
+ -borderwidth 0 \
+ -relief flat \
+ -highlightthickness 0
+
+ $widgets(dropdown) configure \
+ -borderwidth $options(-elementborderwidth) \
+ -relief sunken
+
+ $widgets(listbox) configure \
+ -selectmode browse \
+ -background [$widgets(entry) cget -bg] \
+ -yscrollcommand "$widgets(vsb) set" \
+ -exportselection false \
+ -borderwidth 0
+
+
+# trace variable ::combobox::${w}::entryTextVariable w \
+# [list ::combobox::EntryTrace $w]
+
+ # do some window management foo on the dropdown window
+ wm overrideredirect $widgets(dropdown) 1
+ wm transient $widgets(dropdown) [winfo toplevel $w]
+ wm group $widgets(dropdown) [winfo parent $w]
+ wm resizable $widgets(dropdown) 0 0
+ wm withdraw $widgets(dropdown)
+
+ # this moves the original frame widget proc into our
+ # namespace and gives it a handy name
+ rename ::$w $widgets(frame)
+
+ # now, create our widget proc. Obviously (?) it goes in
+ # the global namespace. All combobox widgets will actually
+ # share the same widget proc to cut down on the amount of
+ # bloat.
+ proc ::$w {command args} \
+ "eval ::combobox::WidgetProc $w \$command \$args"
+
+
+ # ok, the thing exists... let's do a bit more configuration.
+ if {[catch "::combobox::Configure [list $widgets(this)] [array
get options]" error]} {
+ catch {destroy $w}
+ error "internal error: $error"
+ }
+
+ return ""
+
+}
+
+# ::combobox::HandleEvent --
+#
+# this proc handles events from the entry widget that we want
+# handled specially (typically, to allow navigation of the list
+# even though the focus is in the entry widget)
+#
+# Arguments:
+#
+# w widget pathname
+# event a string representing the event (not necessarily an
+# actual event)
+# args additional arguments required by particular events
+
+proc ::combobox::HandleEvent {w event args} {
+ upvar ::combobox::${w}::widgets widgets
+ upvar ::combobox::${w}::options options
+ upvar ::combobox::${w}::oldValue oldValue
+
+ # for all of these events, if we have a special action we'll
+ # do that and do a "return -code break" to keep additional
+ # bindings from firing. Otherwise we'll let the event fall
+ # on through.
+ switch $event {
+
+ "<MouseWheel>" {
+ if {[winfo ismapped $widgets(dropdown)]} {
+ set D [lindex $args 0]
+ # the '120' number in the following expression has
+ # it's genesis in the tk bind manpage, which suggests
+ # that the smallest value of %D for mousewheel events
+ # will be 120. The intent is to scroll one line at a time.
+ $widgets(listbox) yview scroll [expr {-($D/120)}] units
+ }
+ }
+
+ "<Any-KeyPress>" {
+ # if the widget is editable, clear the selection.
+ # this makes it more obvious what will happen if the
+ # user presses <Return> (and helps our code know what
+ # to do if the user presses return)
+ if {$options(-editable)} {
+ $widgets(listbox) see 0
+ $widgets(listbox) selection clear 0 end
+ $widgets(listbox) selection anchor 0
+ $widgets(listbox) activate 0
+ }
+ }
+
+ "<FocusIn>" {
+ set oldValue [$widgets(entry) get]
+ }
+
+ "<FocusOut>" {
+ if {![winfo ismapped $widgets(dropdown)]} {
+ # did the value change?
+ set newValue [$widgets(entry) get]
+ if {$oldValue != $newValue} {
+ CallCommand $widgets(this) $newValue
+ }
+ }
+ }
+
+ "<1>" {
+ set editable [::combobox::GetBoolean $options(-editable)]
+ if {!$editable} {
+ if {[winfo ismapped $widgets(dropdown)]} {
+ $widgets(this) close
+ return -code break;
+
+ } else {
+ if {$options(-state) != "disabled"} {
+ $widgets(this) open
+ return -code break;
+ }
+ }
+ }
+ }
+
+ "<Double-1>" {
+ if {$options(-state) != "disabled"} {
+ $widgets(this) toggle
+ return -code break;
+ }
+ }
+
+ "<Tab>" {
+ if {[winfo ismapped $widgets(dropdown)]} {
+ ::combobox::Find $widgets(this) 0
+ return -code break;
+ } else {
+ ::combobox::SetValue $widgets(this) [$widgets(this) get]
+ }
+ }
+
+ "<Escape>" {
+# $widgets(entry) delete 0 end
+# $widgets(entry) insert 0 $oldValue
+ if {[winfo ismapped $widgets(dropdown)]} {
+ $widgets(this) close
+ return -code break;
+ }
+ }
+
+ "<Return>" {
+ # did the value change?
+ set newValue [$widgets(entry) get]
+ if {$oldValue != $newValue} {
+ CallCommand $widgets(this) $newValue
+ }
+
+ if {[winfo ismapped $widgets(dropdown)]} {
+ ::combobox::Select $widgets(this) \
+ [$widgets(listbox) curselection]
+ return -code break;
+ }
+
+ }
+
+ "<Next>" {
+ $widgets(listbox) yview scroll 1 pages
+ set index [$widgets(listbox) index @0,0]
+ $widgets(listbox) see $index
+ $widgets(listbox) activate $index
+ $widgets(listbox) selection clear 0 end
+ $widgets(listbox) selection anchor $index
+ $widgets(listbox) selection set $index
+
+ }
+
+ "<Prior>" {
+ $widgets(listbox) yview scroll -1 pages
+ set index [$widgets(listbox) index @0,0]
+ $widgets(listbox) activate $index
+ $widgets(listbox) see $index
+ $widgets(listbox) selection clear 0 end
+ $widgets(listbox) selection anchor $index
+ $widgets(listbox) selection set $index
+ }
+
+ "<Down>" {
+ if {[winfo ismapped $widgets(dropdown)]} {
+ ::combobox::tkListboxUpDown $widgets(listbox) 1
+ return -code break;
+
+ } else {
+ if {$options(-state) != "disabled"} {
+ $widgets(this) open
+ return -code break;
+ }
+ }
+ }
+ "<Up>" {
+ if {[winfo ismapped $widgets(dropdown)]} {
+ ::combobox::tkListboxUpDown $widgets(listbox) -1
+ return -code break;
+
+ } else {
+ if {$options(-state) != "disabled"} {
+ $widgets(this) open
+ return -code break;
+ }
+ }
+ }
+ }
+
+ return ""
+}
+
+# ::combobox::DestroyHandler {w} --
+#
+# Cleans up after a combobox widget is destroyed
+#
+# Arguments:
+#
+# w widget pathname
+#
+# Results:
+#
+# The namespace that was created for the widget is deleted,
+# and the widget proc is removed.
+
+proc ::combobox::DestroyHandler {w} {
+
+ catch {
+ # if the widget actually being destroyed is of class Combobox,
+ # remove the namespace and associated proc.
+ if {[string compare [winfo class $w] "Combobox"] == 0} {
+ # delete the namespace and the proc which represents
+ # our widget
+ namespace delete ::combobox::$w
+ rename $w {}
+ }
+ }
+ return ""
+}
+
+# ::combobox::Find
+#
+# finds something in the listbox that matches the pattern in the
+# entry widget and selects it
+#
+# N.B. I'm not convinced this is working the way it ought to. It
+# works, but is the behavior what is expected? I've also got a gut
+# feeling that there's a better way to do this, but I'm too lazy to
+# figure it out...
+#
+# Arguments:
+#
+# w widget pathname
+# exact boolean; if true an exact match is desired
+#
+# Returns:
+#
+# Empty string
+
+proc ::combobox::Find {w {exact 0}} {
+ upvar ::combobox::${w}::widgets widgets
+ upvar ::combobox::${w}::options options
+
+ ## *sigh* this logic is rather gross and convoluted. Surely
+ ## there is a more simple, straight-forward way to implement
+ ## all this. As the saying goes, I lack the time to make it
+ ## shorter...
+
+ # use what is already in the entry widget as a pattern
+ set pattern [$widgets(entry) get]
+
+ if {[string length $pattern] == 0} {
+ # clear the current selection
+ $widgets(listbox) see 0
+ $widgets(listbox) selection clear 0 end
+ $widgets(listbox) selection anchor 0
+ $widgets(listbox) activate 0
+ return
+ }
+
+ # we're going to be searching this list...
+ set list [$widgets(listbox) get 0 end]
+
+ # if we are doing an exact match, try to find,
+ # well, an exact match
+ set exactMatch -1
+ if {$exact} {
+ set exactMatch [lsearch -exact $list $pattern]
+ }
+
+ # search for it. We'll try to be clever and not only
+ # search for a match for what they typed, but a match for
+ # something close to what they typed. We'll keep removing one
+ # character at a time from the pattern until we find a match
+ # of some sort.
+ set index -1
+ while {$index == -1 && [string length $pattern]} {
+ set index [lsearch -glob $list "$pattern*"]
+ if {$index == -1} {
+ regsub {.$} $pattern {} pattern
+ }
+ }
+
+ # this is the item that most closely matches...
+ set thisItem [lindex $list $index]
+
+ # did we find a match? If so, do some additional munging...
+ if {$index != -1} {
+
+ # we need to find the part of the first item that is
+ # unique WRT the second... I know there's probably a
+ # simpler way to do this...
+
+ set nextIndex [expr {$index + 1}]
+ set nextItem [lindex $list $nextIndex]
+
+ # we don't really need to do much if the next
+ # item doesn't match our pattern...
+ if {[string match $pattern* $nextItem]} {
+ # ok, the next item matches our pattern, too
+ # now the trick is to find the first character
+ # where they *don't* match...
+ set marker [string length $pattern]
+ while {$marker <= [string length $pattern]} {
+ set a [string index $thisItem $marker]
+ set b [string index $nextItem $marker]
+ if {[string compare $a $b] == 0} {
+ append pattern $a
+ incr marker
+ } else {
+ break
+ }
+ }
+ } else {
+ set marker [string length $pattern]
+ }
+
+ } else {
+ set marker end
+ set index 0
+ }
+
+ # ok, we know the pattern and what part is unique;
+ # update the entry widget and listbox appropriately
+ if {$exact && $exactMatch == -1} {
+ # this means we didn't find an exact match
+ $widgets(listbox) selection clear 0 end
+ $widgets(listbox) see $index
+
+ } elseif {!$exact} {
+ # this means we found something, but it isn't an exact
+ # match. If we find something that *is* an exact match we
+ # don't need to do the following, since it would merely
+ # be replacing the data in the entry widget with itself
+ set oldstate [$widgets(entry) cget -state]
+ $widgets(entry) configure -state normal
+ $widgets(entry) delete 0 end
+ $widgets(entry) insert end $thisItem
+ $widgets(entry) selection clear
+ $widgets(entry) selection range $marker end
+ $widgets(listbox) activate $index
+ $widgets(listbox) selection clear 0 end
+ $widgets(listbox) selection anchor $index
+ $widgets(listbox) selection set $index
+ $widgets(listbox) see $index
+ $widgets(entry) configure -state $oldstate
+ }
+}
+
+# ::combobox::Select --
+#
+# selects an item from the list and sets the value of the combobox
+# to that value
+#
+# Arguments:
+#
+# w widget pathname
+# index listbox index of item to be selected
+#
+# Returns:
+#
+# empty string
+
+proc ::combobox::Select {w index} {
+ upvar ::combobox::${w}::widgets widgets
+ upvar ::combobox::${w}::options options
+
+ # the catch is because I'm sloppy -- presumably, the only time
+ # an error will be caught is if there is no selection.
+ if {![catch {set data [$widgets(listbox) get [lindex $index 0]]}]} {
+ ::combobox::SetValue $widgets(this) $data
+
+ $widgets(listbox) selection clear 0 end
+ $widgets(listbox) selection anchor $index
+ $widgets(listbox) selection set $index
+
+ }
+ $widgets(entry) selection range 0 end
+ $widgets(entry) icursor end
+
+ $widgets(this) close
+
+ return ""
+}
+
+# ::combobox::HandleScrollbar --
+#
+# causes the scrollbar of the dropdown list to appear or disappear
+# based on the contents of the dropdown listbox
+#
+# Arguments:
+#
+# w widget pathname
+# action the action to perform on the scrollbar
+#
+# Returns:
+#
+# an empty string
+
+proc ::combobox::HandleScrollbar {w {action "unknown"}} {
+ upvar ::combobox::${w}::widgets widgets
+ upvar ::combobox::${w}::options options
+
+ if {$options(-height) == 0} {
+ set hlimit $options(-maxheight)
+ } else {
+ set hlimit $options(-height)
+ }
+
+ switch $action {
+ "grow" {
+ if {$hlimit > 0 && [$widgets(listbox) size] > $hlimit} {
+ pack forget $widgets(listbox)
+ pack $widgets(vsb) -side right -fill y -expand n
+ pack $widgets(listbox) -side left -fill both -expand y
+ }
+ }
+
+ "shrink" {
+ if {$hlimit > 0 && [$widgets(listbox) size] <= $hlimit} {
+ pack forget $widgets(vsb)
+ }
+ }
+
+ "crop" {
+ # this means the window was cropped and we definitely
+ # need a scrollbar no matter what the user wants
+ pack forget $widgets(listbox)
+ pack $widgets(vsb) -side right -fill y -expand n
+ pack $widgets(listbox) -side left -fill both -expand y
+ }
+
+ default {
+ if {$hlimit > 0 && [$widgets(listbox) size] > $hlimit} {
+ pack forget $widgets(listbox)
+ pack $widgets(vsb) -side right -fill y -expand n
+ pack $widgets(listbox) -side left -fill both -expand y
+ } else {
+ pack forget $widgets(vsb)
+ }
+ }
+ }
+
+ return ""
+}
+
+# ::combobox::ComputeGeometry --
+#
+# computes the geometry of the dropdown list based on the size of the
+# combobox...
+#
+# Arguments:
+#
+# w widget pathname
+#
+# Returns:
+#
+# the desired geometry of the listbox
+
+proc ::combobox::ComputeGeometry {w} {
+ upvar ::combobox::${w}::widgets widgets
+ upvar ::combobox::${w}::options options
+
+ if {$options(-height) == 0 && $options(-maxheight) != "0"} {
+ # if this is the case, count the items and see if
+ # it exceeds our maxheight. If so, set the listbox
+ # size to maxheight...
+ set nitems [$widgets(listbox) size]
+ if {$nitems > $options(-maxheight)} {
+ # tweak the height of the listbox
+ $widgets(listbox) configure -height $options(-maxheight)
+ } else {
+ # un-tweak the height of the listbox
+ $widgets(listbox) configure -height 0
+ }
+ update idletasks
+ }
+
+ # compute height and width of the dropdown list
+ set bd [$widgets(dropdown) cget -borderwidth]
+ set height [expr {[winfo reqheight $widgets(dropdown)] + $bd + $bd}]
+ if {[string length $options(-dropdownwidth)] == 0 ||
+ $options(-dropdownwidth) == 0} {
+ set width [winfo width $widgets(this)]
+ } else {
+ set m [font measure [$widgets(listbox) cget -font] "m"]
+ set width [expr {$options(-dropdownwidth) * $m}]
+ }
+
+ # figure out where to place it on the screen, trying to take into
+ # account we may be running under some virtual window manager
+ set screenWidth [winfo screenwidth $widgets(this)]
+ set screenHeight [winfo screenheight $widgets(this)]
+ set rootx [winfo rootx $widgets(this)]
+ set rooty [winfo rooty $widgets(this)]
+ set vrootx [winfo vrootx $widgets(this)]
+ set vrooty [winfo vrooty $widgets(this)]
+
+ # the x coordinate is simply the rootx of our widget, adjusted for
+ # the virtual window. We won't worry about whether the window will
+ # be offscreen to the left or right -- we want the illusion that it
+ # is part of the entry widget, so if part of the entry widget is off-
+ # screen, so will the list. If you want to change the behavior,
+ # simply change the if statement... (and be sure to update this
+ # comment!)
+ set x [expr {$rootx + $vrootx}]
+ if {0} {
+ set rightEdge [expr {$x + $width}]
+ if {$rightEdge > $screenWidth} {
+ set x [expr {$screenWidth - $width}]
+ }
+ if {$x < 0} {set x 0}
+ }
+
+ # the y coordinate is the rooty plus vrooty offset plus
+ # the height of the static part of the widget plus 1 for a
+ # tiny bit of visual separation...
+ set y [expr {$rooty + $vrooty + [winfo reqheight $widgets(this)] + 1}]
+ set bottomEdge [expr {$y + $height}]
+
+ if {$bottomEdge >= $screenHeight} {
+ # ok. Fine. Pop it up above the entry widget isntead of
+ # below.
+ set y [expr {($rooty - $height - 1) + $vrooty}]
+
+ if {$y < 0} {
+ # this means it extends beyond our screen. How annoying.
+ # Now we'll try to be real clever and either pop it up or
+ # down, depending on which way gives us the biggest list.
+ # then, we'll trim the list to fit and force the use of
+ # a scrollbar
+
+ # (sadly, for windows users this measurement doesn't
+ # take into consideration the height of the taskbar,
+ # but don't blame me -- there isn't any way to detect
+ # it or figure out its dimensions. The same probably
+ # applies to any window manager with some magic windows
+ # glued to the top or bottom of the screen)
+
+ if {$rooty > [expr {$screenHeight / 2}]} {
+ # we are in the lower half of the screen --
+ # pop it up. Y is zero; that parts easy. The height
+ # is simply the y coordinate of our widget, minus
+ # a pixel for some visual separation. The y coordinate
+ # will be the topof the screen.
+ set y 1
+ set height [expr {$rooty - 1 - $y}]
+
+ } else {
+ # we are in the upper half of the screen --
+ # pop it down
+ set y [expr {$rooty + $vrooty + \
+ [winfo reqheight $widgets(this)] + 1}]
+ set height [expr {$screenHeight - $y}]
+
+ }
+
+ # force a scrollbar
+ HandleScrollbar $widgets(this) crop
+ }
+ }
+
+ if {$y < 0} {
+ # hmmm. Bummer.
+ set y 0
+ set height $screenheight
+ }
+
+ set geometry [format "=%dx%d+%d+%d" $width $height $x $y]
+
+ return $geometry
+}
+
+# ::combobox::DoInternalWidgetCommand --
+#
+# perform an internal widget command, then mung any error results
+# to look like it came from our megawidget. A lot of work just to
+# give the illusion that our megawidget is an atomic widget
+#
+# Arguments:
+#
+# w widget pathname
+# subwidget pathname of the subwidget
+# command subwidget command to be executed
+# args arguments to the command
+#
+# Returns:
+#
+# The result of the subwidget command, or an error
+
+proc ::combobox::DoInternalWidgetCommand {w subwidget command args} {
+ upvar ::combobox::${w}::widgets widgets
+ upvar ::combobox::${w}::options options
+
+ set subcommand $command
+ set command [concat $widgets($subwidget) $command $args]
+ if {[catch $command result]} {
+ # replace the subwidget name with the megawidget name
+ regsub $widgets($subwidget) $result $widgets(this) result
+
+ # replace specific instances of the subwidget command
+ # with our megawidget command
+ switch $subwidget,$subcommand {
+ listbox,index {regsub "index" $result "list index" result}
+ listbox,insert {regsub "insert" $result "list insert" result}
+ listbox,delete {regsub "delete" $result "list delete" result}
+ listbox,get {regsub "get" $result "list get" result}
+ listbox,size {regsub "size" $result "list size" result}
+ }
+ error $result
+
+ } else {
+ return $result
+ }
+}
+
+
+# ::combobox::WidgetProc --
+#
+# This gets uses as the widgetproc for an combobox widget.
+# Notice where the widget is created and you'll see that the
+# actual widget proc merely evals this proc with all of the
+# arguments intact.
+#
+# Note that some widget commands are defined "inline" (ie:
+# within this proc), and some do most of their work in
+# separate procs. This is merely because sometimes it was
+# easier to do it one way or the other.
+#
+# Arguments:
+#
+# w widget pathname
+# command widget subcommand
+# args additional arguments; varies with the subcommand
+#
+# Results:
+#
+# Performs the requested widget command
+
+proc ::combobox::WidgetProc {w command args} {
+ upvar ::combobox::${w}::widgets widgets
+ upvar ::combobox::${w}::options options
+ upvar ::combobox::${w}::oldFocus oldFocus
+ upvar ::combobox::${w}::oldFocus oldGrab
+
+ set command [::combobox::Canonize $w command $command]
+
+ # this is just shorthand notation...
+ set doWidgetCommand \
+ [list ::combobox::DoInternalWidgetCommand $widgets(this)]
+
+ if {$command == "list"} {
+ # ok, the next argument is a list command; we'll
+ # rip it from args and append it to command to
+ # create a unique internal command
+ #
+ # NB: because of the sloppy way we are doing this,
+ # we'll also let the user enter our secret command
+ # directly (eg: listinsert, listdelete), but we
+ # won't document that fact
+ set command "list-[lindex $args 0]"
+ set args [lrange $args 1 end]
+ }
+
+ set result ""
+
+ # many of these commands are just synonyms for specific
+ # commands in one of the subwidgets. We'll get them out
+ # of the way first, then do the custom commands.
+ switch $command {
+ bbox -
+ delete -
+ get -
+ icursor -
+ index -
+ insert -
+ scan -
+ selection -
+ xview {
+ set result [eval $doWidgetCommand entry $command $args]
+ }
+ list-get {set result [eval $doWidgetCommand listbox get $args]}
+ list-index {set result [eval $doWidgetCommand listbox index $args]}
+ list-size {set result [eval $doWidgetCommand listbox size $args]}
+
+ select {
+ if {[llength $args] == 1} {
+ set index [lindex $args 0]
+ set result [Select $widgets(this) $index]
+ } else {
+ error "usage: $w select index"
+ }
+ }
+
+ subwidget {
+ set knownWidgets [list button entry listbox dropdown vsb]
+ if {[llength $args] == 0} {
+ return $knownWidgets
+ }
+
+ set name [lindex $args 0]
+ if {[lsearch $knownWidgets $name] != -1} {
+ set result $widgets($name)
+ } else {
+ error "unknown subwidget $name"
+ }
+ }
+
+ curselection {
+ set result [eval $doWidgetCommand listbox curselection]
+ }
+
+ list-insert {
+ eval $doWidgetCommand listbox insert $args
+ set result [HandleScrollbar $w "grow"]
+ }
+
+ list-delete {
+ eval $doWidgetCommand listbox delete $args
+ set result [HandleScrollbar $w "shrink"]
+ }
+
+ toggle {
+ # ignore this command if the widget is disabled...
+ if {$options(-state) == "disabled"} return
+
+ # pops down the list if it is not, hides it
+ # if it is...
+ if {[winfo ismapped $widgets(dropdown)]} {
+ set result [$widgets(this) close]
+ } else {
+ set result [$widgets(this) open]
+ }
+ }
+
+ open {
+
+ # if this is an editable combobox, the focus should
+ # be set to the entry widget
+ if {$options(-editable)} {
+ focus $widgets(entry)
+ $widgets(entry) select range 0 end
+ $widgets(entry) icursor end
+ }
+
+ # if we are disabled, we won't allow this to happen
+ if {$options(-state) == "disabled"} {
+ return 0
+ }
+
+ # if there is a -opencommand, execute it now
+ if {[string length $options(-opencommand)] > 0} {
+ # hmmm... should I do a catch, or just let the normal
+ # error handling handle any errors? For now, the latter...
+ uplevel \#0 $options(-opencommand)
+ }
+
+ # compute the geometry of the window to pop up, and set
+ # it, and force the window manager to take notice
+ # (even if it is not presently visible).
+ #
+ # this isn't strictly necessary if the window is already
+ # mapped, but we'll go ahead and set the geometry here
+ # since its harmless and *may* actually reset the geometry
+ # to something better in some weird case.
+ set geometry [::combobox::ComputeGeometry $widgets(this)]
+ wm geometry $widgets(dropdown) $geometry
+ update idletasks
+
+ # if we are already open, there's nothing else to do
+ if {[winfo ismapped $widgets(dropdown)]} {
+ return 0
+ }
+
+ # save the widget that currently has the focus; we'll restore
+ # the focus there when we're done
+ set oldFocus [focus]
+
+ # ok, tweak the visual appearance of things and
+ # make the list pop up
+ $widgets(button) configure -relief sunken
+ wm deiconify $widgets(dropdown)
+ update idletasks
+ raise $widgets(dropdown)
+
+ # force focus to the entry widget so we can handle keypress
+ # events for traversal
+ focus -force $widgets(entry)
+
+ # select something by default, but only if its an
+ # exact match...
+ ::combobox::Find $widgets(this) 1
+
+ # save the current grab state for the display containing
+ # this widget. We'll restore it when we close the dropdown
+ # list
+ set status "none"
+ set grab [grab current $widgets(this)]
+ if {$grab != ""} {set status [grab status $grab]}
+ set oldGrab [list $grab $status]
+ unset grab status
+
+ # *gasp* do a global grab!!! Mom always told me not to
+ # do things like this, but sometimes a man's gotta do
+ # what a man's gotta do.
+ grab -global $widgets(this)
+
+ # fake the listbox into thinking it has focus. This is
+ # necessary to get scanning initialized properly in the
+ # listbox.
+ event generate $widgets(listbox) <B1-Enter>
+
+ return 1
+ }
+
+ close {
+ # if we are already closed, don't do anything...
+ if {![winfo ismapped $widgets(dropdown)]} {
+ return 0
+ }
+
+ # restore the focus and grab, but ignore any errors...
+ # we're going to be paranoid and release the grab before
+ # trying to set any other grab because we really really
+ # really want to make sure the grab is released.
+ catch {focus $oldFocus} result
+ catch {grab release $widgets(this)}
+ catch {
+ set status [lindex $oldGrab 1]
+ if {$status == "global"} {
+ grab -global [lindex $oldGrab 0]
+ } elseif {$status == "local"} {
+ grab [lindex $oldGrab 0]
+ }
+ unset status
+ }
+
+ # hides the listbox
+ $widgets(button) configure -relief raised
+ wm withdraw $widgets(dropdown)
+
+ # select the data in the entry widget. Not sure
+ # why, other than observation seems to suggest that's
+ # what windows widgets do.
+ set editable [::combobox::GetBoolean $options(-editable)]
+ if {$editable} {
+ $widgets(entry) selection range 0 end
+ $widgets(button) configure -relief raised
+ }
+
+
+ # magic tcl stuff (see tk.tcl in the distribution
+ # lib directory)
+ ::combobox::tkCancelRepeat
+
+ return 1
+ }
+
+ cget {
+ if {[llength $args] != 1} {
+ error "wrong # args: should be $w cget option"
+ }
+ set opt [::combobox::Canonize $w option [lindex $args 0]]
+
+ if {$opt == "-value"} {
+ set result [$widgets(entry) get]
+ } else {
+ set result $options($opt)
+ }
+ }
+
+ configure {
+ set result [eval ::combobox::Configure {$w} $args]
+ }
+
+ default {
+ error "bad option \"$command\""
+ }
+ }
+
+ return $result
+}
+
+# ::combobox::Configure --
+#
+# Implements the "configure" widget subcommand
+#
+# Arguments:
+#
+# w widget pathname
+# args zero or more option/value pairs (or a single option)
+#
+# Results:
+#
+# Performs typcial "configure" type requests on the widget
+
+proc ::combobox::Configure {w args} {
+ variable widgetOptions
+ variable defaultEntryCursor
+
+ upvar ::combobox::${w}::widgets widgets
+ upvar ::combobox::${w}::options options
+
+ if {[llength $args] == 0} {
+ # hmmm. User must be wanting all configuration information
+ # note that if the value of an array element is of length
+ # one it is an alias, which needs to be handled slightly
+ # differently
+ set results {}
+ foreach opt [lsort [array names widgetOptions]] {
+ if {[llength $widgetOptions($opt)] == 1} {
+ set alias $widgetOptions($opt)
+ set optName $widgetOptions($alias)
+ lappend results [list $opt $optName]
+ } else {
+ set optName [lindex $widgetOptions($opt) 0]
+ set optClass [lindex $widgetOptions($opt) 1]
+ set default [option get $w $optName $optClass]
+ if {[info exists options($opt)]} {
+ lappend results [list $opt $optName $optClass \
+ $default $options($opt)]
+ } else {
+ lappend results [list $opt $optName $optClass \
+ $default ""]
+ }
+ }
+ }
+
+ return $results
+ }
+
+ # one argument means we are looking for configuration
+ # information on a single option
+ if {[llength $args] == 1} {
+ set opt [::combobox::Canonize $w option [lindex $args 0]]
+
+ set optName [lindex $widgetOptions($opt) 0]
+ set optClass [lindex $widgetOptions($opt) 1]
+ set default [option get $w $optName $optClass]
+ set results [list $opt $optName $optClass \
+ $default $options($opt)]
+ return $results
+ }
+
+ # if we have an odd number of values, bail.
+ if {[expr {[llength $args]%2}] == 1} {
+ # hmmm. An odd number of elements in args
+ error "value for \"[lindex $args end]\" missing"
+ }
+
+ # Great. An even number of options. Let's make sure they
+ # are all valid before we do anything. Note that Canonize
+ # will generate an error if it finds a bogus option; otherwise
+ # it returns the canonical option name
+ foreach {name value} $args {
+ set name [::combobox::Canonize $w option $name]
+ set opts($name) $value
+ }
+
+ # process all of the configuration options
+ # some (actually, most) options require us to
+ # do something, like change the attributes of
+ # a widget or two. Here's where we do that...
+ #
+ # note that the handling of disabledforeground and
+ # disabledbackground is a little wonky. First, we have
+ # to deal with backwards compatibility (ie: tk 8.3 and below
+ # didn't have such options for the entry widget), and
+ # we have to deal with the fact we might want to disable
+ # the entry widget but use the normal foreground/background
+ # for when the combobox is not disabled, but not editable either.
+
+ set updateVisual 0
+ foreach option [array names opts] {
+ set newValue $opts($option)
+ if {[info exists options($option)]} {
+ set oldValue $options($option)
+ }
+
+ switch -- $option {
+ -buttonbackground {
+ $widgets(button) configure -background $newValue
+ }
+ -background {
+ set updateVisual 1
+ set options($option) $newValue
+ }
+
+ -borderwidth {
+ $widgets(frame) configure -borderwidth $newValue
+ set options($option) $newValue
+ }
+
+ -command {
+ # nothing else to do...
+ set options($option) $newValue
+ }
+
+ -commandstate {
+ # do some value checking...
+ if {$newValue != "normal" && $newValue != "disabled"} {
+ set options($option) $oldValue
+ set message "bad state value \"$newValue\";"
+ append message " must be normal or disabled"
+ error $message
+ }
+ set options($option) $newValue
+ }
+
+ -cursor {
+ $widgets(frame) configure -cursor $newValue
+ $widgets(entry) configure -cursor $newValue
+ $widgets(listbox) configure -cursor $newValue
+ set options($option) $newValue
+ }
+
+ -disabledforeground {
+ set updateVisual 1
+ set options($option) $newValue
+ }
+
+ -disabledbackground {
+ set updateVisual 1
+ set options($option) $newValue
+ }
+
+ -dropdownwidth {
+ set options($option) $newValue
+ }
+
+ -editable {
+ set updateVisual 1
+ if {$newValue} {
+ # it's editable...
+ $widgets(entry) configure \
+ -state normal \
+ -cursor $defaultEntryCursor
+ } else {
+ $widgets(entry) configure \
+ -state disabled \
+ -cursor $options(-cursor)
+ }
+ set options($option) $newValue
+ }
+
+ -elementborderwidth {
+ $widgets(button) configure -borderwidth $newValue
+ $widgets(vsb) configure -borderwidth $newValue
+ $widgets(dropdown) configure -borderwidth $newValue
+ set options($option) $newValue
+ }
+
+ -font {
+ $widgets(entry) configure -font $newValue
+ $widgets(listbox) configure -font $newValue
+ set options($option) $newValue
+ }
+
+ -foreground {
+ set updateVisual 1
+ set options($option) $newValue
+ }
+
+ -height {
+ $widgets(listbox) configure -height $newValue
+ HandleScrollbar $w
+ set options($option) $newValue
+ }
+
+ -highlightbackground {
+ $widgets(frame) configure -highlightbackground $newValue
+ set options($option) $newValue
+ }
+
+ -highlightcolor {
+ $widgets(frame) configure -highlightcolor $newValue
+ set options($option) $newValue
+ }
+
+ -highlightthickness {
+ $widgets(frame) configure -highlightthickness $newValue
+ set options($option) $newValue
+ }
+
+ -image {
+ if {[string length $newValue] > 0} {
+ puts "old button width: [$widgets(button) cget -width]"
+ $widgets(button) configure \
+ -image $newValue \
+ -width [expr {[image width $newValue] + 2}]
+ puts "new button width: [$widgets(button) cget -width]"
+
+ } else {
+ $widgets(button) configure -image ::combobox::bimage
+ }
+ set options($option) $newValue
+ }
+
+ -listvar {
+ if {[catch {$widgets(listbox) cget -listvar}]} {
+ return -code error \
+ "-listvar not supported with this version of tk"
+ }
+ $widgets(listbox) configure -listvar $newValue
+ set options($option) $newValue
+ }
+
+ -maxheight {
+ # ComputeGeometry may dork with the actual height
+ # of the listbox, so let's undork it
+ $widgets(listbox) configure -height $options(-height)
+ HandleScrollbar $w
+ set options($option) $newValue
+ }
+
+ -opencommand {
+ # nothing else to do...
+ set options($option) $newValue
+ }
+
+ -relief {
+ $widgets(frame) configure -relief $newValue
+ set options($option) $newValue
+ }
+
+ -selectbackground {
+ $widgets(entry) configure -selectbackground $newValue
+ $widgets(listbox) configure -selectbackground $newValue
+ set options($option) $newValue
+ }
+
+ -selectborderwidth {
+ $widgets(entry) configure -selectborderwidth $newValue
+ $widgets(listbox) configure -selectborderwidth $newValue
+ set options($option) $newValue
+ }
+
+ -selectforeground {
+ $widgets(entry) configure -selectforeground $newValue
+ $widgets(listbox) configure -selectforeground $newValue
+ set options($option) $newValue
+ }
+
+ -state {
+ if {$newValue == "normal"} {
+ set updateVisual 1
+ # it's enabled
+
+ set editable [::combobox::GetBoolean \
+ $options(-editable)]
+ if {$editable} {
+ $widgets(entry) configure -state normal
+ $widgets(entry) configure -takefocus 1
+ }
+
+ # note that $widgets(button) is actually a label,
+ # not a button. And being able to disable labels
+ # wasn't possible until tk 8.3. (makes me wonder
+ # why I chose to use a label, but that answer is
+ # lost to antiquity)
+ if {[info patchlevel] >= 8.3} {
+ $widgets(button) configure -state normal
+ }
+
+ } elseif {$newValue == "disabled"} {
+ set updateVisual 1
+ # it's disabled
+ $widgets(entry) configure -state disabled
+ $widgets(entry) configure -takefocus 0
+ # note that $widgets(button) is actually a label,
+ # not a button. And being able to disable labels
+ # wasn't possible until tk 8.3. (makes me wonder
+ # why I chose to use a label, but that answer is
+ # lost to antiquity)
+ if {$::tcl_version >= 8.3} {
+ $widgets(button) configure -state disabled
+ }
+
+ } else {
+ set options($option) $oldValue
+ set message "bad state value \"$newValue\";"
+ append message " must be normal or disabled"
+ error $message
+ }
+
+ set options($option) $newValue
+ }
+
+ -takefocus {
+ $widgets(entry) configure -takefocus $newValue
+ set options($option) $newValue
+ }
+
+ -textvariable {
+ $widgets(entry) configure -textvariable $newValue
+ set options($option) $newValue
+ }
+
+ -value {
+ ::combobox::SetValue $widgets(this) $newValue
+ set options($option) $newValue
+ }
+
+ -width {
+ $widgets(entry) configure -width $newValue
+ $widgets(listbox) configure -width $newValue
+ set options($option) $newValue
+ }
+
+ -xscrollcommand {
+ $widgets(entry) configure -xscrollcommand $newValue
+ set options($option) $newValue
+ }
+ }
+
+ if {$updateVisual} {UpdateVisualAttributes $w}
+ }
+}
+
+# ::combobox::UpdateVisualAttributes --
+#
+# sets the visual attributes (foreground, background mostly)
+# based on the current state of the widget (normal/disabled,
+# editable/non-editable)
+#
+# why a proc for such a simple thing? Well, in addition to the
+# various states of the widget, we also have to consider the
+# version of tk being used -- versions from 8.4 and beyond have
+# the notion of disabled foreground/background options for various
+# widgets. All of the permutations can get nasty, so we encapsulate
+# it all in one spot.
+#
+# note also that we don't handle all visual attributes here; just
+# the ones that depend on the state of the widget. The rest are
+# handled on a case by case basis
+#
+# Arguments:
+# w widget pathname
+#
+# Returns:
+# empty string
+
+proc ::combobox::UpdateVisualAttributes {w} {
+
+ upvar ::combobox::${w}::widgets widgets
+ upvar ::combobox::${w}::options options
+
+ if {$options(-state) == "normal"} {
+
+ set foreground $options(-foreground)
+ set background $options(-background)
+
+ } elseif {$options(-state) == "disabled"} {
+
+ set foreground $options(-disabledforeground)
+ set background $options(-disabledbackground)
+ }
+
+ $widgets(entry) configure -foreground $foreground -background $background
+ $widgets(listbox) configure -foreground $foreground -background $background
+ $widgets(button) configure -foreground $foreground
+ $widgets(vsb) configure -background $background -troughcolor
$background
+ $widgets(frame) configure -background $background
+
+ # we need to set the disabled colors in case our widget is disabled.
+ # We could actually check for disabled-ness, but we also need to
+ # check whether we're enabled but not editable, in which case the
+ # entry widget is disabled but we still want the enabled colors. It's
+ # easier just to set everything and be done with it.
+
+ if {$::tcl_version >= 8.4} {
+ $widgets(entry) configure \
+ -disabledforeground $foreground \
+ -disabledbackground $background
+ $widgets(button) configure -disabledforeground $foreground
+ $widgets(listbox) configure -disabledforeground $foreground
+ }
+}
+
+# ::combobox::SetValue --
+#
+# sets the value of the combobox and calls the -command,
+# if defined
+#
+# Arguments:
+#
+# w widget pathname
+# newValue the new value of the combobox
+#
+# Returns
+#
+# Empty string
+
+proc ::combobox::SetValue {w newValue} {
+
+ upvar ::combobox::${w}::widgets widgets
+ upvar ::combobox::${w}::options options
+ upvar ::combobox::${w}::ignoreTrace ignoreTrace
+ upvar ::combobox::${w}::oldValue oldValue
+
+ if {[info exists options(-textvariable)] \
+ && [string length $options(-textvariable)] > 0} {
+ set variable ::$options(-textvariable)
+ set $variable $newValue
+ } else {
+ set oldstate [$widgets(entry) cget -state]
+ $widgets(entry) configure -state normal
+ $widgets(entry) delete 0 end
+ $widgets(entry) insert 0 $newValue
+ $widgets(entry) configure -state $oldstate
+ }
+
+ # set our internal textvariable; this will cause any public
+ # textvariable (ie: defined by the user) to be updated as
+ # well
+# set ::combobox::${w}::entryTextVariable $newValue
+
+ # redefine our concept of the "old value". Do it before running
+ # any associated command so we can be sure it happens even
+ # if the command somehow fails.
+ set oldValue $newValue
+
+
+ # call the associated command. The proc will handle whether or
+ # not to actually call it, and with what args
+ CallCommand $w $newValue
+
+ return ""
+}
+
+# ::combobox::CallCommand --
+#
+# calls the associated command, if any, appending the new
+# value to the command to be called.
+#
+# Arguments:
+#
+# w widget pathname
+# newValue the new value of the combobox
+#
+# Returns
+#
+# empty string
+
+proc ::combobox::CallCommand {w newValue} {
+ upvar ::combobox::${w}::widgets widgets
+ upvar ::combobox::${w}::options options
+
+ # call the associated command, if defined and -commandstate is
+ # set to "normal"
+ if {$options(-commandstate) == "normal" && \
+ [string length $options(-command)] > 0} {
+ set args [list $widgets(this) $newValue]
+ uplevel \#0 $options(-command) $args
+ }
+}
+
+
+# ::combobox::GetBoolean --
+#
+# returns the value of a (presumably) boolean string (ie: it should
+# do the right thing if the string is "yes", "no", "true", 1, etc
+#
+# Arguments:
+#
+# value value to be converted
+# errorValue a default value to be returned in case of an error
+#
+# Returns:
+#
+# a 1 or zero, or the value of errorValue if the string isn't
+# a proper boolean value
+
+proc ::combobox::GetBoolean {value {errorValue 1}} {
+ if {[catch {expr {([string trim $value])?1:0}} res]} {
+ return $errorValue
+ } else {
+ return $res
+ }
+}
+
+# ::combobox::convert --
+#
+# public routine to convert %x, %y and %W binding substitutions.
+# Given an x, y and or %W value relative to a given widget, this
+# routine will convert the values to be relative to the combobox
+# widget. For example, it could be used in a binding like this:
+#
+# bind .combobox <blah> {doSomething [::combobox::convert %W -x %x]}
+#
+# Note that this procedure is *not* exported, but is intended for
+# public use. It is not exported because the name could easily
+# clash with existing commands.
+#
+# Arguments:
+#
+# w a widget path; typically the actual result of a %W
+# substitution in a binding. It should be either a
+# combobox widget or one of its subwidgets
+#
+# args should one or more of the following arguments or
+# pairs of arguments:
+#
+# -x <x> will convert the value <x>; typically <x> will
+# be the result of a %x substitution
+# -y <y> will convert the value <y>; typically <y> will
+# be the result of a %y substitution
+# -W (or -w) will return the name of the combobox widget
+# which is the parent of $w
+#
+# Returns:
+#
+# a list of the requested values. For example, a single -w will
+# result in a list of one items, the name of the combobox widget.
+# Supplying "-x 10 -y 20 -W" (in any order) will return a list of
+# three values: the converted x and y values, and the name of
+# the combobox widget.
+
+proc ::combobox::convert {w args} {
+ set result {}
+ if {![winfo exists $w]} {
+ error "window \"$w\" doesn't exist"
+ }
+
+ while {[llength $args] > 0} {
+ set option [lindex $args 0]
+ set args [lrange $args 1 end]
+
+ switch -exact -- $option {
+ -x {
+ set value [lindex $args 0]
+ set args [lrange $args 1 end]
+ set win $w
+ while {[winfo class $win] != "Combobox"} {
+ incr value [winfo x $win]
+ set win [winfo parent $win]
+ if {$win == "."} break
+ }
+ lappend result $value
+ }
+
+ -y {
+ set value [lindex $args 0]
+ set args [lrange $args 1 end]
+ set win $w
+ while {[winfo class $win] != "Combobox"} {
+ incr value [winfo y $win]
+ set win [winfo parent $win]
+ if {$win == "."} break
+ }
+ lappend result $value
+ }
+
+ -w -
+ -W {
+ set win $w
+ while {[winfo class $win] != "Combobox"} {
+ set win [winfo parent $win]
+ if {$win == "."} break;
+ }
+ lappend result $win
+ }
+ }
+ }
+ return $result
+}
+
+# ::combobox::Canonize --
+#
+# takes a (possibly abbreviated) option or command name and either
+# returns the canonical name or an error
+#
+# Arguments:
+#
+# w widget pathname
+# object type of object to canonize; must be one of "command",
+# "option", "scan command" or "list command"
+# opt the option (or command) to be canonized
+#
+# Returns:
+#
+# Returns either the canonical form of an option or command,
+# or raises an error if the option or command is unknown or
+# ambiguous.
+
+proc ::combobox::Canonize {w object opt} {
+ variable widgetOptions
+ variable columnOptions
+ variable widgetCommands
+ variable listCommands
+ variable scanCommands
+
+ switch $object {
+ command {
+ if {[lsearch -exact $widgetCommands $opt] >= 0} {
+ return $opt
+ }
+
+ # command names aren't stored in an array, and there
+ # isn't a way to get all the matches in a list, so
+ # we'll stuff the commands in a temporary array so
+ # we can use [array names]
+ set list $widgetCommands
+ foreach element $list {
+ set tmp($element) ""
+ }
+ set matches [array names tmp ${opt}*]
+ }
+
+ {list command} {
+ if {[lsearch -exact $listCommands $opt] >= 0} {
+ return $opt
+ }
+
+ # command names aren't stored in an array, and there
+ # isn't a way to get all the matches in a list, so
+ # we'll stuff the commands in a temporary array so
+ # we can use [array names]
+ set list $listCommands
+ foreach element $list {
+ set tmp($element) ""
+ }
+ set matches [array names tmp ${opt}*]
+ }
+
+ {scan command} {
+ if {[lsearch -exact $scanCommands $opt] >= 0} {
+ return $opt
+ }
+
+ # command names aren't stored in an array, and there
+ # isn't a way to get all the matches in a list, so
+ # we'll stuff the commands in a temporary array so
+ # we can use [array names]
+ set list $scanCommands
+ foreach element $list {
+ set tmp($element) ""
+ }
+ set matches [array names tmp ${opt}*]
+ }
+
+ option {
+ if {[info exists widgetOptions($opt)] \
+ && [llength $widgetOptions($opt)] == 2} {
+ return $opt
+ }
+ set list [array names widgetOptions]
+ set matches [array names widgetOptions ${opt}*]
+ }
+
+ }
+
+ if {[llength $matches] == 0} {
+ set choices [HumanizeList $list]
+ error "unknown $object \"$opt\"; must be one of $choices"
+
+ } elseif {[llength $matches] == 1} {
+ set opt [lindex $matches 0]
+
+ # deal with option aliases
+ switch $object {
+ option {
+ set opt [lindex $matches 0]
+ if {[llength $widgetOptions($opt)] == 1} {
+ set opt $widgetOptions($opt)
+ }
+ }
+ }
+
+ return $opt
+
+ } else {
+ set choices [HumanizeList $list]
+ error "ambiguous $object \"$opt\"; must be one of $choices"
+ }
+}
+
+# ::combobox::HumanizeList --
+#
+# Returns a human-readable form of a list by separating items
+# by columns, but separating the last two elements with "or"
+# (eg: foo, bar or baz)
+#
+# Arguments:
+#
+# list a valid tcl list
+#
+# Results:
+#
+# A string which as all of the elements joined with ", " or
+# the word " or "
+
+proc ::combobox::HumanizeList {list} {
+
+ if {[llength $list] == 1} {
+ return [lindex $list 0]
+ } else {
+ set list [lsort $list]
+ set secondToLast [expr {[llength $list] -2}]
+ set most [lrange $list 0 $secondToLast]
+ set last [lindex $list end]
+
+ return "[join $most {, }] or $last"
+ }
+}
+
+# This is some backwards-compatibility code to handle TIP 44
+# (http://purl.org/tcl/tip/44.html). For all private tk commands
+# used by this widget, we'll make duplicates of the procs in the
+# combobox namespace.
+#
+# I'm not entirely convinced this is the right thing to do. I probably
+# shouldn't even be using the private commands. Then again, maybe the
+# private commands really should be public. Oh well; it works so it
+# must be OK...
+foreach command {TabToWindow CancelRepeat ListboxUpDown} {
+ if {[llength [info commands ::combobox::tk$command]] == 1} break;
+
+ set tmp [info commands tk$command]
+ set proc ::combobox::tk$command
+ if {[llength [info commands tk$command]] == 1} {
+ set command [namespace which [lindex $tmp 0]]
+ proc $proc {args} "uplevel $command \$args"
+ } else {
+ if {[llength [info commands ::tk::$command]] == 1} {
+ proc $proc {args} "uplevel ::tk::$command \$args"
+ }
+ }
+}
+
+# end of combobox.tcl
+
diff --git a/git-gui/lib/option.tcl b/git-gui/lib/option.tcl
index aa9f783..66d0d36 100644
--- a/git-gui/lib/option.tcl
+++ b/git-gui/lib/option.tcl
@@ -256,9 +256,22 @@ proc do_options {} {
frame $w.global.$name
label $w.global.$name.l -text "$text:"
pack $w.global.$name.l -side left -anchor w -fill x
- eval tk_optionMenu $w.global.$name.family \
- global_config_new(gui.$font^^family) \
- $all_fonts
+
+ eval ::combobox::combobox $w.global.$name.family \
+ -borderwidth 1 \
+ -textvariable global_config_new(gui.$font^^family) \
+ -editable false \
+ -highlightthickness 1
+
+ set widest 0
+ foreach family $all_fonts {
+ if {[set length [string length $family]] > $widest} {
+ set widest $length
+ }
+ $w.global.$name.family list insert end $family
+ }
+ $w.global.$name.family configure -width $widest
+
spinbox $w.global.$name.size \
-textvariable global_config_new(gui.$font^^size) \
-from 2 -to 80 -increment 1 \
--
1.5.3.GIT
^ permalink raw reply related
* Re: [PATCH] diff-delta.c: pack the index structure
From: David Kastrup @ 2007-09-08 6:48 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: git
In-Reply-To: <alpine.LFD.0.9999.0709072215420.21186@xanadu.home>
Nicolas Pitre <nico@cam.org> writes:
> On Sat, 8 Sep 2007, David Kastrup wrote:
>
>> In normal use cases, the performance wins are not overly impressive:
>> we get something like 5-10% due to the slightly better locality of
>> memory accesses using the packed structure.
>
> The gain is probably counterbalanced by the fact that you're copying
> the whole index when packing it, which is unfortunate.
It was a design choice (I don't particularly like it myself). An
index is created once, used a dozen times. Doing the packing in-place
implies using realloc on the finished index. I consider the
likelihood of permanent memory fragmentation higher when doing that
rather than when allocating a fresh area of the same size.
Also a repack in-place is going to cost more operations and have quite
more complicated code.
> Also, could you provide actual test results backing your performance
> claim? 5-10% is still not negligible.
I did in my git repository
for i in .git/objects/[0-9a-f][0-9a-f]/[0-9a-f]*;do echo $i;done|sed 's+.*ts/\(..\)/+\1+' > /tmp/objlist
and then something like
dak@lola:/usr/local/tmp/git$ time ./git-pack-objects </tmp/objlist --stdout|dd of=/dev/null
4099+2 records in
4100+1 records out
2099205 bytes (2.1 MB) copied, 3.99295 seconds, 526 kB/s
real 0m4.023s
user 0m3.812s
sys 0m0.168s
dak@lola:/usr/local/tmp/git$ time git-pack-objects </tmp/objlist --stdout|dd of=/dev/null
4099+2 records in
4100+1 records out
2099205 bytes (2.1 MB) copied, 4.18734 seconds, 501 kB/s
real 0m4.218s
user 0m4.012s
sys 0m0.160s
dak@lola:/usr/local/tmp/git$
repeatedly on a warm cache. Results were pretty much comparable:
consistently my version was faster, but never more than 10%.
>> - struct index_entry *entry, **hash;
>> + struct unpacked_index_entry *entry, **hash;
>> + struct index_entry *aentry, **ahash;
>
> What does the "a" stand for?
array (as opposed to linked list). Was the first thing coming into my
mind. Maybe I should have gone for "p" for packed, but I shied from
it because it often is meant to imply "pointer".
Alternatively I could replace "entry" with "uentry", but that affects
more lines.
What do you propose?
>> + mem = index+1;
> [...]
>> + for (i=0; i<hsize; i++) {
> [...]
>> + for (entry=hash[i]; entry; entry=entry->next)
>
> Minor style nit: please add spaces around "+", "=", "<", etc. for
> consistency.
Can do.
--
David Kastrup, Kriemhildstr. 15, 44793 Bochum
^ permalink raw reply
* Re: [PATCH] diff-delta.c: pack the index structure
From: David Kastrup @ 2007-09-08 8:36 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: git
In-Reply-To: <alpine.LFD.0.9999.0709072215420.21186@xanadu.home>
I answered this post already, but the answer seems to have disappeared
into a black hole. What a nuisance.
Nicolas Pitre <nico@cam.org> writes:
> On Sat, 8 Sep 2007, David Kastrup wrote:
>
>> In normal use cases, the performance wins are not overly impressive:
>> we get something like 5-10% due to the slightly better locality of
>> memory accesses using the packed structure.
>
> The gain is probably counterbalanced by the fact that you're copying
> the whole index when packing it, which is unfortunate.
The alternative would be packing in-place. That's actually a rather
contorted operation, and the gains from packing would then have to be
claimed by "realloc". I think that realloc is more likely to leave a
fragmented address space (giving the reclaimed memory to the heap
rather than back to the system), but I'd have to check glibc for
details at least on GNU/Linux.
Another option avoiding the realloc would be not to use linked lists
at all but just collect all entries sequentually in "packed" form, and
sort them into order afterwards. That's an O(n lg n) operation with a
large n. Even if one has a sorting algorithm with good memory
locality, I doubt that the locality would compensate for the lg n
factor, even when taking into account that we save ourselves the
copying. And that is not even taken the possibility of having to cull
some buckets into account, another O(n) operation (which amounts to
copying everything once, too).
> Also, could you provide actual test results backing your performance
> claim? 5-10% is still not negligible.
dak@lola:/usr/local/tmp/git$ for i in .git/objects/[0-9a-f][0-9a-f]/[0-9a-f]*;do echo $i;done|sed 's+.*ts/\(..\)/+\1+' > /tmp/objlist
dak@lola:/usr/local/tmp/git$ time ./git-pack-objects </tmp/objlist --stdout|dd of=/dev/null
4099+2 records in
4100+1 records out
2099205 bytes (2.1 MB) copied, 4.01225 seconds, 523 kB/s
real 0m4.043s
user 0m3.836s
sys 0m0.164s
dak@lola:/usr/local/tmp/git$ time git-pack-objects </tmp/objlist --stdout|dd of=/dev/null
4099+2 records in
4100+1 records out
2099205 bytes (2.1 MB) copied, 4.34936 seconds, 483 kB/s
real 0m4.381s
user 0m4.056s
sys 0m0.176s
dak@lola:/usr/local/tmp/git$
Of course, on a warm file system.
>> - struct index_entry *entry, **hash;
>> + struct unpacked_index_entry *entry, **hash;
>> + struct index_entry *aentry, **ahash;
>
> What does the "a" stand for?
array (rather than linked list). Should I rather rename entry and
hash to uentry and uhash (for unpacked)?
>> + mem = index+1;
> [...]
>> + for (i=0; i<hsize; i++) {
> [...]
>> + for (entry=hash[i]; entry; entry=entry->next)
>
> Minor style nit: please add spaces around "+", "=", "<", etc. for
> consistency.
Will do.
--
David Kastrup, Kriemhildstr. 15, 44793 Bochum
^ permalink raw reply
* Re: git-svn: Branching clarifications
From: David Kastrup @ 2007-09-08 7:58 UTC (permalink / raw)
To: git
In-Reply-To: <20070908074944.GC24166@muzzle>
Eric Wong <normalperson@yhbt.net> writes:
> David Kastrup <dak@gnu.org> wrote:
>
>> Please. git-svn is told how to find the trunk on its command line.
>> Nothing makes sense (short of an _explicit_ wish otherwise for
>> which it might make sense to create a command line option) than to
>> map master to the trunk.
>
> Keep in mind that command-line arguments for trunk, branches and tags
> are _all_ optional to git-svn.
>
> If only trunk or nothing is specified, the current behavior will
> always be correct.
Sure, since Subversion does not distinguish trunk, branches, tags, or
even projects from each other: they are just naming conventions in the
repository and nothing enforces them. So if you check out a single,
named directory, it is natural that this will be master, and tracked.
> There's also a case if only branches and/or tags are specified, with
> no trunk given. That would need to be handled, somehow...
Just barf unless a --master=thisbranch option is given. If you want
to, you can allow multiple --track=remotebranch options as well,
giving you a tracking branch for the specified remote branches under
their original name.
Something like that.
--
David Kastrup, Kriemhildstr. 15, 44793 Bochum
^ permalink raw reply
* Re: git-svn: Branching clarifications
From: Eric Wong @ 2007-09-08 7:49 UTC (permalink / raw)
To: David Kastrup; +Cc: Russ Brown, git
In-Reply-To: <85r6l9zlt4.fsf@lola.goethe.zz>
David Kastrup <dak@gnu.org> wrote:
> Eric Wong <normalperson@yhbt.net> writes:
>
> > git-svn sets "master" to the most recently committed-to branch
> > in SVN the first time it fetches. "git-log master" will tell
> > you (look at the git-svn-id: lines).
>
> Sigh. Another "surprise the user by an arbitrary looking choice that
> might possibly correspond to what he wants done because it something
> obscure in the commit history suggests so" design decision.
>
> I don't want my master set according to something that a coworker (or
> even myself) happened to commit last to.
>
> Please. git-svn is told how to find the trunk on its command line.
> Nothing makes sense (short of an _explicit_ wish otherwise for which
> it might make sense to create a command line option) than to map
> master to the trunk.
Keep in mind that command-line arguments for trunk, branches and tags
are _all_ optional to git-svn.
If only trunk or nothing is specified, the current behavior will always
be correct.
There's also a case if only branches and/or tags are specified, with no
trunk given. That would need to be handled, somehow...
I've also tracked several (both OSS and closed) projects that have a
policy of doing all work on branches with a trunk that's almost never
up-to-date.
Tracking the last-committed branch was the easiest to code, and we even
tell the user which branch they're on. I guess I could add a message
telling them all the other refs they can "git reset --hard" to if they
don't like their current one.
> As a design rule: don't second-guess the user, _ever_, and
> particularly not on decisions with large consequences. A tool should
> not have a mind of its own but do what it is told. And if it can't
> figure out what it is told, by simple, user-understandable criteria,
> barf. And of course have a way to _direct_ it when it can't figure it
> out on its own, or if the simple and obvious default would not do the
> right thing.
git-svn used to never check anything out and leave HEAD dangling. I was
happy with that, but I got a lot of user complaints from that, too.
--
Eric Wong
^ permalink raw reply
* Re: git-svn: Branching clarifications
From: David Kastrup @ 2007-09-08 6:57 UTC (permalink / raw)
To: Eric Wong; +Cc: Russ Brown, git
In-Reply-To: <20070908052126.GB28855@soma>
Eric Wong <normalperson@yhbt.net> writes:
> git-svn sets "master" to the most recently committed-to branch
> in SVN the first time it fetches. "git-log master" will tell
> you (look at the git-svn-id: lines).
Sigh. Another "surprise the user by an arbitrary looking choice that
might possibly correspond to what he wants done because it something
obscure in the commit history suggests so" design decision.
I don't want my master set according to something that a coworker (or
even myself) happened to commit last to.
Please. git-svn is told how to find the trunk on its command line.
Nothing makes sense (short of an _explicit_ wish otherwise for which
it might make sense to create a command line option) than to map
master to the trunk.
As a design rule: don't second-guess the user, _ever_, and
particularly not on decisions with large consequences. A tool should
not have a mind of its own but do what it is told. And if it can't
figure out what it is told, by simple, user-understandable criteria,
barf. And of course have a way to _direct_ it when it can't figure it
out on its own, or if the simple and obvious default would not do the
right thing.
--
David Kastrup, Kriemhildstr. 15, 44793 Bochum
^ permalink raw reply
* Re: [RFC] Convert builin-mailinfo.c to use The Better String Library.
From: David Kastrup @ 2007-09-08 6:36 UTC (permalink / raw)
To: John 'Z-Bo' Zabroski; +Cc: git
In-Reply-To: <loom.20070908T025508-792@post.gmane.org>
John 'Z-Bo' Zabroski <johnzabroski@yahoo.com> writes:
> Linus Torvalds <torvalds <at> linux-foundation.org> writes:
>
>> IOW, C++ is in that inconvenient spot where it doesn't help make
>> things simple enough to be truly usable for prototyping or simple
>> GUI programming, and yet isn't the lean system programming language
>> that C is that actively encourags you to use simple and direct
>> constructs.
>
> I want code that is Correct, Explicit, Fast, and in that order.
One beef I have with C++ is its automatic conversion rules. They were
obviously designed with two goals:
a) behave as C when not using user-defined types. That's ok.
b) behave like Fortran in mixed-type expressions involving "complex"
when using C++ (with any arbitrary user-defined type taking the
role of "complex").
And b is just madness. Not every user-defined arithmetic type is
complex. I did some work using modular arithmetic (GF(65521) and
similar) and it was some hard work to keep values going through the
wrong arithmetic conversions. Basically trial and error and reading
the generated assembly code and head scratching and standard-reading.
In short: the automatic conversions made it hard to express what one
wanted to get done, both for compiler as well as programmer.
--
David Kastrup, Kriemhildstr. 15, 44793 Bochum
^ permalink raw reply
* Re: [RFC] Convert builin-mailinfo.c to use The Better String Library.
From: David Kastrup @ 2007-09-08 6:25 UTC (permalink / raw)
To: Dmitry Kakurin; +Cc: Andreas Ericsson, Linus Torvalds, Matthieu Moy, Git
In-Reply-To: <a1bbc6950709071737h155c78b7ie6d2b77719239a6a@mail.gmail.com>
"Dmitry Kakurin" <dmitry.kakurin@gmail.com> writes:
> On 9/7/07, David Kastrup <dak@gnu.org> wrote:
>> Just compiling under C++, with no source changes, is likely to impact
>> performance and compile time rather badly
>
> This in fact is a very specific statement. Would you care to back it
> up with facts?
Read up on the Linux kernel history in the archives.
--
David Kastrup, Kriemhildstr. 15, 44793 Bochum
^ permalink raw reply
* Re: [RFC] Convert builin-mailinfo.c to use The Better String Library.
From: David Kastrup @ 2007-09-08 6:24 UTC (permalink / raw)
To: Dmitry Kakurin; +Cc: Johannes Schindelin, Linus Torvalds, Matthieu Moy, Git
In-Reply-To: <a1bbc6950709071732s1f15e5ev28bdfc5c1ab5877b@mail.gmail.com>
"Dmitry Kakurin" <dmitry.kakurin@gmail.com> writes:
> On 9/7/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>
>> No, it's not. As has been shown by some very good _arguments_.
>> Once you have facts to back up your claims, it is not any belief
>> any longer.
>
> Well I've heard *opinions* and anecdotal evidence. No facts though.
Anecdotal evidence _is_ hard facts. That's what experience is all
about.
--
David Kastrup, Kriemhildstr. 15, 44793 Bochum
^ permalink raw reply
* Re: [PATCH 2/2] git-tag -s must fail if gpg is broken and cannot sign tags
From: Carlos Rica @ 2007-09-08 5:41 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Junio C Hamano, git
In-Reply-To: <20070907045833.GZ18160@spearce.org>
2007/9/7, Shawn O. Pearce <spearce@spearce.org>:
> Junio C Hamano <gitster@pobox.com> wrote:
> > "Shawn O. Pearce" <spearce@spearce.org> writes:
> >
> > > "Shawn O. Pearce" <spearce@spearce.org> wrote:
> > >> If the user has misconfigured `user.signingkey` in their .git/config
> > >> or just doesn't have any secret keys on their keyring and they ask
> > >> for a signed tag with `git tag -s` we better make sure the resulting
> > >> tag was actually signed by gpg.
> >
> > This seems to fail the test depending on the order processes
> > happen to be scheduled. I haven't looked at it closely yet.
>
> That's not good. I noticed stepping through the code last night
> that if gpg is misconfigured (e.g. set a bad user.signingkey in
> .git/config) it will terminate and send SIGPIPE to git-tag, which
> makes it terminate.
I haven't tested it enough, but now I know that the program is terminated
in write_or_die(gpg.in, buffer, size), and it is passing the test or not
depending on the system, because I added some code before the test
and then it worked for me and if I remove that test, it is failing again.
These messages are printed:
gpg: skipped "BobTheMouse": secret key not available
gpg: signing failed: secret key not available
Just after start_command and before write_in_full.
Possibly the reason is that code in write_in_full() that makes exit(0)
without a warning when EPIPE is returned, or possibly is write()
in xwrite(), that dies directly when EPIPE is received like it was for
builtin-verify-tag.c. Catching the signal EPIPE doesn't worked for me,
so I will do some checks more to trace the code more exactly
in my system.
> All my change did was implement proper error handling. So if you
> are seeing failures now then we probably have a problem with the
> code without my patch too...
The test seems to fail also without your patch, as you say.
^ permalink raw reply
* Re: git-svn: Branching clarifications
From: Eric Wong @ 2007-09-08 5:21 UTC (permalink / raw)
To: Russ Brown; +Cc: git
In-Reply-To: <46E18095.60501@gmail.com>
Russ Brown <pickscrape@gmail.com> wrote:
> I have a few questions about how/when to use git branches when using
> git-svn (I'm a tad confused...)
>
> Say I've initialised and fetched a git repo involving trunk and one
> branch (say branch1) from an svn repository.
>
> If I do git branch -a, I see similar to the following:
>
> * master
> branch1
> trunk
>
> (branch1 and trunk are in red for me, which I figure means they're
> remotely tracked or something like that?)
Yes, that seems to be the case (I just enabled color.branch=auto in
.git/config for the first time).
> OK, so that's telling me that I currently have master checked out into
> my working copy. My question is: where did master come from? Is it a
> local branch of trunk?
git-svn sets "master" to the most recently committed-to branch
in SVN the first time it fetches. "git-log master" will tell
you (look at the git-svn-id: lines).
After you do your initial fetch/clone, it should say something like:
----------------------------------------------------------------------
Checked out HEAD:
svn://my-repository-here/branches/foo r12345
----------------------------------------------------------------------
> Moving on, say I want to work on branch1. Can I simply issue git
> checkout branch1? If I do so I get this:
>
> $ git branch -a
> * (no branch)
> master
> branch1
> trunk
>
> Which is a bit scary. It seems my working copy is orphaned...
Yes it is. Branches under the refs/remotes/ hierarchy were created
back in the day to tell the local user they should not commit to
them directly.
> OK, so let's assume I'm supposed to create a local branch of each remote
> branch I want to work on. So:
>
> $ git branch local/branch1 branch1
> $ git checkout local/branch1
>
> $ git-branch -a
> * local/branch1
> master
> branch1
> trunk
That's correct. You can also use "git checkout -b local/branch1 branch1"
instead of those two commands.
> Am I supposed to have used --track when creating this branch? What are
> the implications for specifying or not specifying that flag when using
> git-svn?
--track has no effect with git-svn. dcommit will automatically figure
out which branch it should commit to[1]. Running "git-svn dcommit -n"
with 1.5.3 will tell you which URL you'll commit to.
> So I do some editing on this branch, commit and dcommit. The changes
> appear as expected in the repo.
>
> At this point if I checkout master, the contents look like
> local/branch1, which isn't what I'd suspected (that it would be a branch
> of trunk). What does master represent?
(see above)
> So I checkout local/trunk, and create a new file, commit and dcommit.
> Umm, it's been committed to branch1 on the repo: not trunk,
>
> So I figure I'm quite obviously doing something wrong here. Could
> someone give me a hand and tell me what it is I'm getting wrong?
If you run "git-log local/trunk", does the first commit to show
a "git-svn-id: " line have the URL pointing to trunk or branch1?
Again, if you're unsure about where you're committing to,
"git-svn dcommit -n" in 1.5.3 is your friend.
[1] - as long as you don't use git-merge or git-pull. If you decide to
do those things, make sure you have Lars's latest patches
that enables --first-parent.
Otherwise stick with format-patch/am/cherry-pick/fetch/rebase
--
Eric Wong
^ permalink raw reply
* Re: git-svn 1.5.3 does not understand grafts?
From: Eric Wong @ 2007-09-08 5:01 UTC (permalink / raw)
To: Joakim Tjernlund; +Cc: git
In-Reply-To: <1189196635.14841.24.camel@gentoo-jocke.transmode.se>
Joakim Tjernlund <joakim.tjernlund@transmode.se> wrote:
> On Fri, 2007-09-07 at 18:52 +0200, Joakim Tjernlund wrote:
> > On Fri, 2007-09-07 at 18:41 +0200, Joakim Tjernlund wrote:
> > > svnadmin create /usr/local/src/TM/svn-tst/7720-svn/
> > > svn mkdir file:///usr/local/src/TM/svn-tst/7720-svn/trunk -m "Add trunk dir"
> > > svn mkdir file:///usr/local/src/TM/svn-tst/7720-svn/trunk/swp -m "Add swp dir"
> > >
> > > In my git repo I do
> > > git-svn init file:///usr/local/src/TM/svn-tst/7720-svn/trunk/swp
> > > git-svn fetch
> > > git branch svn remotes/git-svn
> > > #make remotes/git-svn parent to the initial commit in my git tree
> > > graftid=`git-show-ref -s svn`
> > > echo da783cce390ce013b19f1d308ea6813269c6a6b5 $graftid > .git/info/grafts
> > > #da783... is the initial commit in my git tree.
> > > git-svn dcommit
> > >
> > > fails with:
> > > Committing to file:///usr/local/src/TM/svn-tst/7720-svn/trunk/swp ...
> > > Commit da783cce390ce013b19f1d308ea6813269c6a6b5
> > > has no parent commit, and therefore nothing to diff against.
> > > You should be working from a repository originally created by git-svn
> >
> > Using filter-branch helps, but git-svn isn't too happy:
> >
> > git-svn init file:///usr/local/src/TM/svn-tst/7720-svn/trunk/swp
> > git-svn fetch
> > git branch svn remotes/git-svn
> > #make remotes/git-svn parent to the initial commit in my git tree
> > graftid=`git-show-ref -s svn`
> > echo da783cce390ce013b19f1d308ea6813269c6a6b5 $graftid > .git/info/grafts
> > #da783... is the initial commit in my git tree.
> > git filter-branch $graftid..HEAD
> > git-svn dcommit
> >
> > Now I get alot of complaints, but it commits to svn.
> > It takes forever though:
> > r3 = 55a489bd4f66dd1f641a4676359d7b8911dc7d83 (git-svn)
> > W: HEAD and refs/remotes/git-svn differ, using rebase:
> > :100644 100644 f85ae11af7715a224015582724cb2bab87ec914a
I haven't used filter-branch myself, but you probably need to remove all
.rev_db* files in $GIT_DIR after running it (git-svn can recreate them
unless you use the svmRevProps or noMetadata options.
> [SNIP]
>
> Just wanted to add that 1.5.2.2 works with grafts and
> that I suspect sub read_commit_parents in git-svn, but as I don't
> do perl I am stuck.
Crap, it looks like I completely forgot about the existence
of grafts while doing this function.
> Jocke
> Oh, Eric W. CC:ed as well this time
Thanks.
--
Eric Wong
^ permalink raw reply
* [PATCH] Button added to performs a GUI diff (inline)
From: Unknown, Pierre Marc Dumuid @ 2007-09-08 3:01 UTC (permalink / raw)
To: git; +Cc: pierre.dumuid
>From bd43cca7aa88282455b6bbe6e2f9d8134da1029c Mon Sep 17 00:00:00 2001
From: Pierre Marc Dumuid <pierre.dumuid@adelaide.edu.au>
Date: Sat, 8 Sep 2007 00:32:10 +0930
Subject: [PATCH] Button added to performs a GUI diff
Signed-off-by: Pierre Marc Dumuid <pierre.dumuid@adelaide.edu.au>
---
gitk | 46 +++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 45 insertions(+), 1 deletions(-)
diff --git a/gitk b/gitk
index 300fdce..b8b0b2e 100755
--- a/gitk
+++ b/gitk
@@ -737,7 +737,9 @@ proc makewindow {} {
-command changediffdisp -variable diffelide -value {1 0}
label .bleft.mid.labeldiffcontext -text " Lines of context: " \
-font $uifont
- pack .bleft.mid.diff .bleft.mid.old .bleft.mid.new -side left
+ button .bleft.top.guidiff -text "GUI diff" -command doguidiff \
+ -font $uifont
+ pack .bleft.mid.diff .bleft.mid.old .bleft.mid.new .bleft.top.guidiff -side left
spinbox .bleft.mid.diffcontext -width 5 -font $textfont \
-from 1 -increment 1 -to 10000000 \
-validate all -validatecommand "diffcontextvalidate %P" \
@@ -5332,6 +5334,48 @@ proc incrsearch {name ix op} {
}
}
+proc doguidiff {} {
+ global cflist sha1string
+
+ set taglist [$cflist tag ranges highlight]
+ set from [lindex $taglist 0]
+ set to [lindex $taglist 1]
+
+ set fname [$cflist get $from $to]
+
+ if {[catch {set merge_tool [exec git-config merge.tool]} err]} {
+ error_popup "Sorry, 'merge.tool' not defined in git-config"
+ return
+ }
+
+ if {[catch {exec git cat-file -p $sha1string^:$fname > .gitk_diffolder} err]
+ || [catch {exec git cat-file -p $sha1string:$fname > .gitk_diffnewer} err]} {
+ error_popup "Invalid file selected for comparison"
+ return
+ }
+
+ switch -regexp $merge_tool {
+ "kdiff3" {
+ exec kdiff3 --L1 "$fname (Older)" --L2 "$fname (Newer)" .gitk_diffolder .gitk_diffnewer &
+ }
+ "meld|opendiff|vimdiff|xxdiff" {
+ exec $merge_tool .gitk_diffolder .gitk_diffnewer &
+ }
+ "tkdiff" {
+ exec $merge_tool -L "$fname (Older)" -L "$fname (Newer)" .gitk_diffolder .gitk_diffnewer &
+ }
+ "vimdiff" {
+ exec $merge_tool .gitk_diffolder .gitk_diffnewer
+ }
+ "emerge" {
+ exec emacs -f emerge-files-command .gitk_diffolder .gitk_diffnewer &
+ }
+ default {
+ show_error {} "No merge.tool defined."
+ }
+ }
+}
+
proc dosearch {} {
global sstring ctext searchstring searchdirn
--
1.5.2.4
^ permalink raw reply related
* Re: [PATCH] diff-delta.c: pack the index structure
From: Nicolas Pitre @ 2007-09-08 2:34 UTC (permalink / raw)
To: David Kastrup; +Cc: git
In-Reply-To: <85fy1q11xv.fsf@lola.goethe.zz>
On Sat, 8 Sep 2007, David Kastrup wrote:
> In normal use cases, the performance wins are not overly impressive:
> we get something like 5-10% due to the slightly better locality of
> memory accesses using the packed structure.
The gain is probably counterbalanced by the fact that you're copying the
whole index when packing it, which is unfortunate.
Also, could you provide actual test results backing your performance
claim? 5-10% is still not negligible.
> - struct index_entry *entry, **hash;
> + struct unpacked_index_entry *entry, **hash;
> + struct index_entry *aentry, **ahash;
What does the "a" stand for?
> + mem = index+1;
[...]
> + for (i=0; i<hsize; i++) {
[...]
> + for (entry=hash[i]; entry; entry=entry->next)
Minor style nit: please add spaces around "+", "=", "<", etc. for
consistency.
Otherwise that looks fine to me.
Nicolas
^ permalink raw reply
* Re: how to access working tree from .git dir?
From: Josh England @ 2007-09-08 1:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <7vr6la11q9.fsf@gitster.siamese.dyndns.org>
On Fri, 2007-09-07 at 16:42 -0700, Junio C Hamano wrote:
> Josh England <jjengla@comcast.net> writes:
>
> > On Fri, 2007-09-07 at 15:12 -0700, Junio C Hamano wrote:
> >> Josh England <jjengla@comcast.net> writes:
> >>
> >> > OK. Fair enough. Maybe it would be good to note in git-sh-setup.sh that
> >> > many of the supplied functions will not work when called from within
> >> > $GIT_DIR.
> >>
> >> Sorry, "supplied functions"? Care to clarify with a patch?
> >
> > I guess really just the cd_to_topdir() function. It will silently fail
> > when run from within $GIT_DIR.
>
> Ah, I see what you meant.
>
> I think you probably are supposed to check with is_bare_repository
> or something before calling that, as asking to cd to toplevel
> implies you know there is such a thing as toplevel ;-)
That's fine. I guess using `$GIT_DIR/../` as you mentioned before will
just have to work.
-JE
^ permalink raw reply
* Re: [RFC] Convert builin-mailinfo.c to use The Better String Library.
From: John 'Z-Bo' Zabroski @ 2007-09-08 0:56 UTC (permalink / raw)
To: git
In-Reply-To: <alpine.LFD.0.999.0709070203200.5626@evo.linux-foundation.org>
Linus Torvalds <torvalds <at> linux-foundation.org> writes:
>
> And if you want a fancier language, C++ is absolutely the worst one to
> choose. If you want real high-level, pick one that has true high-level
> features like garbage collection or a good system integration, rather than
> something that lacks both the sparseness and straightforwardness of C,
> *and* doesn't even have the high-level bindings to important concepts.
>
> IOW, C++ is in that inconvenient spot where it doesn't help make things
> simple enough to be truly usable for prototyping or simple GUI
> programming, and yet isn't the lean system programming language that C is
> that actively encourags you to use simple and direct constructs.
>
> Linus
>
I want code that is Correct, Explicit, Fast, and in that order.
I'm 23 years old and learned C++ when I was 13. Back then, my compiler didn't
even support "bleeding edge" C++ language features like namespaces. I'm not a
C++ expert, and I don't have the ego to call myself a superb programmer. The
largest program I've written is 10K SLOC in C. Yet, I'd like to participate in
this discussion, if that is OKay =)
I do think I am capable of an honest critique of the downside of C++:
_Problems_ _With_ _C++_
*size*
On my bookshelf, most recent editions of the canonical C++ _books_:
Accelerated C++: Practical Programming by Example (336 pages)
The C++ Standard Template Library: A Tutorial and Reference (832 pages)
Effective C++: 50 Specific Ways to Improve Your Programs and Design (288
pages)
More Effective C++: 35 New Ways to Improve Your Programs and Designs
(336 pages)
Exceptional C++: 47 Engineering Puzzles, Programming Problems, and
Solutions (240 pages)
More Exceptional C++: 40 New Engineering Puzzles, Programming Problems,
and Solutions (304 pages)
The C++ Programming Language (1030 pages)
Modern C++ Design: Generic Programming and Design Patterns Applied (352
pages)
C++ Templates: The Complete Guide (552 pages)
Altogether, that is 3918 pages. K&R, the canonical C _book_, is 272 pages.
Becoming a C++ language lawyer is much harder than becoming a C language
lawyer. Language lawyers know "how not to hang oneself" while programming in
the language. I don't know how many of these titles are translated to other
languages, however, I am sure the *effort* required to translate all of them is
significant. Open source is more successful if there is a lingua franca for
programming, and that is C. Now, it may move away from C over time, but it will
*never* be C++ because it's encyclopedic.
*hidden complexity*
(1) it's hard to say what code will compile down to. viz., constructors can
be elided, but there is no fitness warranty; profiling your compiler to find out
whether it is elided is tedious and "searching for secrets" that should be
_explicit_
(2) people don't understand static polymorphism and compile-time dispatch;
people are used to objects sending messages dynamically (run-time dispatch)
(3) coercion
(4) networks of objects are not explicitly laid out, hiding quadratically
complex patterns of communication between objects
(5) data structure and data flow come before algorithms. Sometimes, data
structure dictates data flow (ad-hoc networks of objects); sometimes, data flow
dictates data structure (one of life's most disagreeable tasks - waiting in line
- is characterized as FIFO). This, I feel, is the most important point, because
the first rule of programming is to figure out what you want to say before you
figure out how to say it. In C++, ad-hoc networks of objects with cyclic
message paths are all too easy to create [see (4)] which means _code_ _is_ _not_
_explicit_ and as a result _code_ _is_ _not_ _fast_.
*transfer semantics on objects are not robust*
this ties into (1) in hidden complexity
the code author needs to specify a lot of boilerplate to achieve desired
transfer semantics on objects. Similarly, the code audience, be it reviewer,
maintainer or merger, needs to read a lot of boilerplate to understand how
objects get moved around in memory. Moreover, most of these concepts are
intuitively declarative in nature, such as a parent object/child object relation.
*poor re-use of effort*
"code re-use" is a misnomer; when programmers speak of code-reuse they mean
re-use of effort. There is no benefit to polymorphism if effort cannot be
consolidated easily.
*C++ Standard iffy*
Some things just disappear quickly for *frantic* reasons (strstream was
removed for aesthetics), indicating not enough foresight into what is important.
I do not want to pick a language where I have to worry about features in it's
"standard library" becoming deprecated mainly for aesthetics. As Dijkstra
preached, programming is _not_ supposed to be a frantic exercise.
*usually, better options*
See C++??: A Critique of C++ and Programing and Language Trends in the 1990s
by Ian Joyner http://web.mac.com/joynerian/iWeb/Ian%20Joyner/CPPCritique.pdf
(Somewhat outdated, but many of the points are intrinsic and will forever be
relevant). You can add to the list of better options D 1.0.
^ permalink raw reply
* .git/info/exclude w/ CFLF fails in cygwin
From: Jing Xue @ 2007-09-08 1:01 UTC (permalink / raw)
To: git
(1.5.3.1 in cygwin, Win XP)
I have cygwin configured to operate in the DOS/text mode, which means
cygwin translates LF to CRLF when writing a file, and CRLF to LF when
reading. Unfortunately cygwin's fstat() implementation doesn't take the
mode into account when reporting stat.st_size, presumably for the sake
of performance, while read() does actually do the conversion.
That causes the function add_excludes_from_file_1() in dir.c to reject a
.git/info/exclude file with CRLF ending, because the size actually read
is not the same as the size reported by fstat().
The simplest fix I have found is to explicitly open the exclude file in
binary mode, because the rest of the exclude file parsing code actually
deals with CRLF quite fine.
I would submit a patch but I am not sure if this is the appropriate fix.
By the way, parsing .git/config with CRLF in the same environment works
fine because the code reads the file by byte and doesn't do any size
validation.
Any thoughts?
--
Jing Xue
^ permalink raw reply
* Re: [RFC] Convert builin-mailinfo.c to use The Better String Library.
From: Dmitry Kakurin @ 2007-09-08 0:37 UTC (permalink / raw)
To: David Kastrup; +Cc: Andreas Ericsson, Linus Torvalds, Matthieu Moy, Git
In-Reply-To: <85sl5q1570.fsf@lola.goethe.zz>
On 9/7/07, David Kastrup <dak@gnu.org> wrote:
> Just compiling under C++, with no source changes, is likely to impact
> performance and compile time rather badly
This in fact is a very specific statement. Would you care to back it
up with facts?
--
- Dmitry
^ permalink raw reply
* Re: [RFC] Convert builin-mailinfo.c to use The Better String Library.
From: Dmitry Kakurin @ 2007-09-08 0:32 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Linus Torvalds, Matthieu Moy, Git
In-Reply-To: <Pine.LNX.4.64.0709071119510.28586@racer.site>
On 9/7/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Thu, 6 Sep 2007, Dmitry Kakurin wrote:
>
> > Anyway I don't mean to start a religious C vs. C++ war.
>
> You have a very strange way of not meaning to start a C vs. C++ war.
I honestly didn't. I didn't even think it's possible. In the
environment of mainstream commercial software development the last war
on this subj was over 8-10 years ago.
Even wars like "do we use exceptions/templates/stl" are pretty much
over. Now days it's "do we use Boost", or "do we use template
metaprogramming". But even more often it's Java/C# vs. C++.
That's why I was wondering how come C was chosen for Git.
> > It's a matter of beliefs and as such pointless.
>
> No, it's not. As has been shown by some very good _arguments_. Once you
> have facts to back up your claims, it is not any belief any longer.
Well I've heard *opinions* and anecdotal evidence. No facts though.
And it's not surprising. There could be no hard facts in such a
matter. It always boils down to "most of all, I want my software to be
X" where X is different for different people (fast,maintainable,quick
to market, scalable, beautiful, etc ... to name a few).
With different values of X any debate is pointless. And X is exactly
the matter of believes.
Anyway my curiosity is satisfied (thru the roof so to speak) and I
think it's enough on the subj. It has reminded me of good old times
though.
--
- Dmitry
^ permalink raw reply
* Re: how to access working tree from .git dir?
From: Junio C Hamano @ 2007-09-07 23:42 UTC (permalink / raw)
To: Josh England; +Cc: Git Mailing List
In-Reply-To: <1189204498.15140.4.camel@beauty>
Josh England <jjengla@comcast.net> writes:
> On Fri, 2007-09-07 at 15:12 -0700, Junio C Hamano wrote:
>> Josh England <jjengla@comcast.net> writes:
>>
>> > OK. Fair enough. Maybe it would be good to note in git-sh-setup.sh that
>> > many of the supplied functions will not work when called from within
>> > $GIT_DIR.
>>
>> Sorry, "supplied functions"? Care to clarify with a patch?
>
> I guess really just the cd_to_topdir() function. It will silently fail
> when run from within $GIT_DIR.
Ah, I see what you meant.
I think you probably are supposed to check with is_bare_repository
or something before calling that, as asking to cd to toplevel
implies you know there is such a thing as toplevel ;-)
^ permalink raw reply
* [PATCH] diff-delta.c: pack the index structure
From: David Kastrup @ 2007-09-07 23:38 UTC (permalink / raw)
To: git
In normal use cases, the performance wins are not overly impressive:
we get something like 5-10% due to the slightly better locality of
memory accesses using the packed structure.
However, since the data structure for index entries saves 33% of
memory on 32-bit platforms and 40% on 64-bit platforms, the behavior
when memory gets limited should be nicer.
This is a rather well-contained change. One obvious improvement would
be sorting the elements in one bucket according to their hash, then
using binary probing to find the elements with the right hash value.
As it stands, the output should be strictly the same as previously
unless one uses the option for limiting the amount of used memory, in
which case the created packs might be better.
Signed-off-by: David Kastrup <dak@gnu.org>
---
diff-delta.c | 62 +++++++++++++++++++++++++++++++++++++++++++---------------
1 files changed, 46 insertions(+), 16 deletions(-)
diff --git a/diff-delta.c b/diff-delta.c
index 0dde2f2..cf0136f 100644
--- a/diff-delta.c
+++ b/diff-delta.c
@@ -115,9 +115,13 @@ static const unsigned int U[256] = {
struct index_entry {
const unsigned char *ptr;
unsigned int val;
- struct index_entry *next;
};
+struct unpacked_index_entry {
+ struct index_entry entry;
+ struct unpacked_index_entry *next;
+};
+
struct delta_index {
unsigned long memsize;
const void *src_buf;
@@ -131,7 +135,8 @@ struct delta_index * create_delta_index(const void *buf, unsigned long bufsize)
unsigned int i, hsize, hmask, entries, prev_val, *hash_count;
const unsigned char *data, *buffer = buf;
struct delta_index *index;
- struct index_entry *entry, **hash;
+ struct unpacked_index_entry *entry, **hash;
+ struct index_entry *aentry, **ahash;
void *mem;
unsigned long memsize;
@@ -148,28 +153,21 @@ struct delta_index * create_delta_index(const void *buf, unsigned long bufsize)
hmask = hsize - 1;
/* allocate lookup index */
- memsize = sizeof(*index) +
- sizeof(*hash) * hsize +
+ memsize = sizeof(*hash) * hsize +
sizeof(*entry) * entries;
mem = malloc(memsize);
if (!mem)
return NULL;
- index = mem;
- mem = index + 1;
hash = mem;
mem = hash + hsize;
entry = mem;
- index->memsize = memsize;
- index->src_buf = buf;
- index->src_size = bufsize;
- index->hash_mask = hmask;
memset(hash, 0, hsize * sizeof(*hash));
/* allocate an array to count hash entries */
hash_count = calloc(hsize, sizeof(*hash_count));
if (!hash_count) {
- free(index);
+ free(hash);
return NULL;
}
@@ -183,12 +181,13 @@ struct delta_index * create_delta_index(const void *buf, unsigned long bufsize)
val = ((val << 8) | data[i]) ^ T[val >> RABIN_SHIFT];
if (val == prev_val) {
/* keep the lowest of consecutive identical blocks */
- entry[-1].ptr = data + RABIN_WINDOW;
+ entry[-1].entry.ptr = data + RABIN_WINDOW;
+ --entries;
} else {
prev_val = val;
i = val & hmask;
- entry->ptr = data + RABIN_WINDOW;
- entry->val = val;
+ entry->entry.ptr = data + RABIN_WINDOW;
+ entry->entry.val = val;
entry->next = hash[i];
hash[i] = entry++;
hash_count[i]++;
@@ -212,15 +211,46 @@ struct delta_index * create_delta_index(const void *buf, unsigned long bufsize)
continue;
entry = hash[i];
do {
- struct index_entry *keep = entry;
+ struct unpacked_index_entry *keep = entry;
int skip = hash_count[i] / HASH_LIMIT;
do {
+ --entries;
entry = entry->next;
} while(--skip && entry);
+ ++entries;
keep->next = entry;
} while(entry);
}
free(hash_count);
+ memsize = sizeof(*index)
+ +sizeof(*ahash) * (hsize+1) +
+ +sizeof(*aentry) * entries;
+ mem=malloc(memsize);
+
+ if (!mem) {
+ free(hash);
+ return NULL;
+ }
+
+ index = mem;
+ index->memsize = memsize;
+ index->src_buf = buf;
+ index->src_size = bufsize;
+ index->hash_mask = hmask;
+
+ mem = index+1;
+ ahash = mem;
+ mem = ahash + (hsize+1);
+ aentry = mem;
+
+ for (i=0; i<hsize; i++) {
+ ahash[i] = aentry;
+ for (entry=hash[i]; entry; entry=entry->next)
+ *aentry++ = entry->entry;
+ }
+ ahash[hsize] = aentry;
+ assert(aentry-(struct index_entry *)mem == entries);
+ free(hash);
return index;
}
@@ -302,7 +332,7 @@ create_delta(const struct delta_index *index,
val ^= U[data[-RABIN_WINDOW]];
val = ((val << 8) | *data) ^ T[val >> RABIN_SHIFT];
i = val & index->hash_mask;
- for (entry = index->hash[i]; entry; entry = entry->next) {
+ for (entry = index->hash[i]; entry<index->hash[i+1]; entry++) {
const unsigned char *ref = entry->ptr;
const unsigned char *src = data;
unsigned int ref_size = ref_top - ref;
--
1.5.3.GIT
^ permalink raw reply related
* Re: [RFC] Convert builin-mailinfo.c to use The Better String Library.
From: Walter Bright @ 2007-09-07 23:16 UTC (permalink / raw)
To: git
In-Reply-To: <851wda2pbo.fsf@lola.goethe.zz>
David Kastrup wrote:
> The problem is a toy problem: in real applications,
Necessarily, to make an example suitable for a n.g. post, I ruthlessly
cut down the size of it. This can have the inadvertent effect of making
it appear trivial.
> you'll need to
> access several data structures using the same index, and you'll need
> to be able to assign index values to temporary variables and so on.
The index is available:
foreach (index, value; array)
{
writefln("array[%s] = %s", index, value);
}
and it isn't necessary to worry about what the correct type for index
is, as it is inferred.
> So being able to hide the type of an index in one very specific
> application (looping through a single array completely)
foreach'ing over a subset (i.e. slice) of an array:
foreach (value; array[5 .. $])
... loop from 5 to the end ...
> at one place is not going to buy you much.
Experience with foreach in real code shows that the for loop is what
becomes a rarity. Simple as it is, foreach is one of the best liked
improvements D has. And I speak as one who has written so many for loops
that spewing out:
for (int i = 0; i < 10; i++)
is a 'finger' macro for me, i.e. my fingers blit it out without even
thinking about it.
> Anyway, D is pretty much irrelevant as a perspective for git, so you
> should take it to a language advocacy group.
I wished to answer your specific comments in this post.
^ permalink raw reply
* Re: [RFC] Convert builin-mailinfo.c to use The Better String Library.
From: Walter Bright @ 2007-09-07 22:54 UTC (permalink / raw)
To: git
In-Reply-To: <20070907205604.GC23483@artemis.corp>
Pierre Habouzit wrote:
> Sure, but it does not works on amd64 properly (and it's the
> architecture I care about) and is not ready for the current gcc (4.2,
> only 4.1 builds) and so on. It's not as stable as DMD is. It does not
> lags too much version-wise, it lags in maturity. But well, youth has a
> cure: time :)
Yes, and the more people use it, the better it will get. These are all
environmental problems, not technical limitations of the language.
^ permalink raw reply
* Re: how to access working tree from .git dir?
From: Josh England @ 2007-09-07 22:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <7vveam15w9.fsf@gitster.siamese.dyndns.org>
On Fri, 2007-09-07 at 15:12 -0700, Junio C Hamano wrote:
> Josh England <jjengla@comcast.net> writes:
>
> > OK. Fair enough. Maybe it would be good to note in git-sh-setup.sh that
> > many of the supplied functions will not work when called from within
> > $GIT_DIR.
>
> Sorry, "supplied functions"? Care to clarify with a patch?
I guess really just the cd_to_topdir() function. It will silently fail
when run from within $GIT_DIR.
-JE
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox