[PATCH 0 of 3] RFC: Splitting patch hunks with the record extension

Steve Losh steve at stevelosh.com
Tue Jan 5 00:44:51 UTC 2010


First of all, these patches are NOT ready to be queued.  I want to get some feedback on the UI and code so I can rework them into a more acceptable state.

The record extension is great for committing small changes from a larger body of work done in the local working copy.  However, it operates at the level of patch hunks, so if your "small change" is adjacent to another change there's no way for you to untangle it without some extra work.

For example, say you've got a simple function:

    def clean_string(s):
        return s.strip()

Now you decide that you only want to strip *trailing* whitespace, and you also add a docstring:

    def clean_string(s):
        '''Return a cleaned version of the given string.'''
        return s.rstrip()

You'd like to commit the docstring addition right away because it could be useful to users, but you want to test the implications of the actual functionality change before committing (and pushing) it.

With the current record extension you can't do this because the two lines are adjacent and so are part of a single hunk.  These patches attempt to address that situation in an easy-to-use fashion.

The first patch refactors the filterpatch.prompt() function a bit to make the prompt choices easier to modify.  Currently record shows the same choices for entire files and hunks, but the next patch requires an extra option for hunks.  On its own this patch has no visible changes, but it makes the next patch a bit more elegant.

The second patch actually adds the hunk-splitting functionality.  I could really use some advice on the UI for this.

Right now it prompts the user for a series of line numbers.  Each line number the user gives will mark the start of a new patch.  I thought about only accepting a single number.  This wouldn't reduce the functionality because you could simply choose to split again for each of the new hunks, but I think it would be more convenient to be able to specify everything at once if you know all the places you want to split.

A screenshot will probably make this clearer: http://twitpic.com/wp8vs

The "line numbers?" prompt definitely needs some work and I'd appreciate any advice on what it should be.  I'm having trouble coming up with a version of "please enter the line number of the first line of each new patch" that's not too verbose but still gets the message across.

The displaying of the help text is also an ugly hack.  It works, but I'd welcome advice on how to make this better (and internationalizable).

The third patch modifies the color extension to colorize the numbered diff lines that the split option displays, because trying to read diffs without color makes me drink.

I already know of one bug in this patch: if a diff has a context line that starts with a number and a colon the color will be wrong.  I wanted to get feedback on the splitting UI in general before spending time thinking about fixing this, because if line-numbered diffs aren't going to be used it would be a waste of time to try to fix this.

The patches themselves will follow this email, but they're also at http://bitbucket.org/sjl/hg-crew-sjl-patches/qseries/ if you want to pull them directly and kick the tires.  Ignore the 'hide-closed' patch -- that's a graphlog enhancement that I've been sitting on for a while and still mean to finish up some day.


More information about the Mercurial-devel mailing list