Re: Naughty, Evil git-gui patches

From: Shawn O. Pearce <spearce@spearce.org>
Date: May 26, 2008, 02:14 UTC
Message-ID: <20080526021413.GB29038@spearce.org>
In-Reply-To: <1211521917.832.45.camel@gdub.robertsr.us>

Barry Roberts <blr@robertsr.us> wrote:

Show 13 quoted lines

> On Thu, 2008-05-22 at 19:24 -0400, Shawn O. Pearce wrote:
> > 
> > Building a good looking list of stashes would probably require using
> > several columns of text widgets with a single scrollbar.  This is
> > how the blame viewer and gitk are put together.  Its ugly as s**t.
> 
> A simpler option I considered  is just a simple numeric text field in
> the stash commands that are allowed to specify a stash (show, apply,
> drop, pop).  Choosing any of these would just open a dialog with a list
> of stashes and a text field that defaults to 0.  That text field value
> gets put into the stash@{$number}.  Is that too lame to bother
> implementing?  Clearly a list to choose from is more standard UI design,
> I just don't know if it justifies the cost for me.

Good point.  A text field would at least let you select a stash,
but I'm not sure there is much of a point without also having a
way to view the stash's diff.  But that could also just be a text
field to enter a stash.  ;-)

> > > 0004 - This is just a concession to (I think) Tortoise.  Just before you 
> > > commit, you notice that you left in a debug message.  This gives us an 
> > > easy way to fix by diff before commit'ing.  This requires setting 
> > > GIT_EXTERNAL_DIFF, or it's not very interesting.

OK.  Your (clipped) explanation makes sense.  I'm not sure
GIT_EXTERNAL_DIFF is the best way to select that program; perhaps
it should be a git-gui specific setting in ~/.gitconfig or something.

> What I don't like about my patch now is that it is synchronous.  git-gui
> is locked up waiting for the ediff/winmerge/kdiff3 window to close.
> Plus, then I have to click OK to close the console window.  Hmm. Maybe I
> should look at how you launch gitk.  That seems more appropriate here.

I think the procedure is called do_gitk, but it invokes another Tcl/Tk process with the path of gitk. The trick to making it run asynchronous is to append "&" to the end of the argument list when you call exec:

	exec [list $env(GIT_EXTERNAL_DIFF) $file &]

Show 7 quoted lines

> > I definately see some value in your bastard patches and would like to
> > work with you to get them into a shape that we can include them.  :-)
> 
> For now, I think the external diff and pull functions would be the
> easiest to clean up.  My stash menu is ugly, and if git-gui's merge is
> forever safer than cmd line git-merge, that's fine with me.  I would be
> interested in your opinion on that prioritization.

Both are useful. Whatever order you want to work on them in. I think you are right that the diff and pull work is smaller and easier to clean up.

-- 
Shawn.