Re: Naughty, Evil git-gui patches

From: Barry Roberts <blr@robertsr.us>
Date: May 23, 2008, 05:51 UTC
Message-ID: <1211521917.832.45.camel@gdub.robertsr.us>
In-Reply-To: <20080522232417.GS29038@spearce.org>

On Thu, 2008-05-22 at 19:24 -0400, Shawn O. Pearce wrote:

> Hah!  Do you know why git-gui came along?  Because it was written as
> an emergency measure to allow a transition from no version control
> at all to git at my day-job.  :-)

Necessity, invention.

Show 18 quoted lines

> > 0002 - I have read the reasons for having merge in the gui be more 
> > strict than 'git merge' from the command line, but 'cvs update' never 
> > gave a clean way to back out, so nobody here expects that anyway.
> 
> Maybe its just the way my brain works these days with git, but I
> have never found the refusal to merge with a dirty working directory
> to be a limitation.  I usually either don't have a dirty working
> directory, or I stash into a temporary branch and switch back to
> do the merge, then rebase or cherry-pick.  Yea, that does mean I
> fall back to the command line in such cases.  Git Gui only users
> don't have that option.
> 
> Once you get used to the idea of being able to recover your old
> state after a merge has started (or even finished!) though the
> idea of a dirty merge just sends chills down my spine.  Its a
> really bad idea.  IMHO its like holding a loaded gun to your foot
> and pulling the trigger every time you do a merge.  After a while
> you run out of toes and have lost something you cared about.

I agree. I regard this one as transitional. People used to thinking in CVS have a hard time letting go. My plan for this was to maintain a local patch until the developers I work with are properly trained, and then drop it. But, it's Free Software, so if other folks have the same pain, it's in the mail archive now.

Show 7 quoted lines

> > 0003 -  Yeah, I want to list the stashes and select from available 
> > stashes to apply.  But this does the 80% of what we need (reducing 
> > command line usage), and my tcl sk1llz aren't that l33t.
> 
> 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.

When I get time, I still plan to look at it. But if my Windows using colleagues don't complain, it may not happen.

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.

Show 11 quoted lines

>  
> > 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.
> 
> I'm not sure I understand this.  Are you trying to get a diff for
> the entire working directory against the staged files in the index?
> As opposed to looking at each file individually?  What is your
> external diff program able to show that git-gui's internal diff
> viewer does not?

The intent is to edit one file at time to clean up before committing. The external diff doesn't show anything different, it just lets you edit. I use emacs/ediff, win users use winmerge or kdiff3. So it's poorly named, but that was the git feature I used. The Tortoise (or maybe WinCVS) scenario that I'm trying to enable is this:

As I'm staging files for commit, I notice I left in a debug message or test code that shouldn't be committed, and I want to get rid of in the file in the working directory. With that file selected in the "Unstaged Changes" box, I select "External Diff" from "Commit" menu (at least that's where it is now). For me that opens an ediff with my modified file and the previous version so I can easily remove things I don't want to commit. Save, exit, Rescan (necessary?), stage, commit.

Another option I considered was adding to the context menu in the diff box to revert a hunk in the file. But sometimes the changes aren't exactly hunks and you want an editor to, say, just change one line in a hunk.

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'm not objecting to supporting GIT_EXTERNAL_DIFF, I just want to
> better understand what you are trying to accomplish here so we can
> make sure its the _right_ support.

I'm positive there's room for improvement.

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

Now I get to go play with git and figure out how to disentangle the mixed stash and merge stuff in patch #2. That'll be fun.

Thanks, Barry