Re: [StGit PATCH] Check for local changes with "goto"

From: Karl Hasselström <kha@treskal.com>
Date: Jan 29, 2009, 03:45 UTC
Message-ID: <20090129034512.GD24344@diana.vm.bytemark.co.uk>
In-Reply-To: <20090128231305.16133.29214.stgit@localhost.localdomain>

On 2009-01-28 23:13:05 +0000, Catalin Marinas wrote:

> This is done by default, unless the --keep option is passed, for
> consistency with the "pop" command. The index is checked in the
> Transaction.run() function so that other commands could benefit from
> this feature (off by default).

This looks good, except for ...

> +        # Check for not clean index
> +        if check_clean and iw and not iw.index.is_clean():
> +            self.__halt('Repository not clean. Use "refresh" or '
> +                        '"status --reset"')

... this, which doesn't do what I think you think it does.

Index.is_clean() calls "git update-index --refresh", which checks for changes in the worktree relative to the index. It's bad design to have it in Index rather than IndexAndWorktree, but that's my fault, not yours. ;-) But the point that breaks your patch is that it doesn't check for changes between index and HEAD -- try it and see.

The fix I'd suggest is to move the existing is_clean() method to IndexAndWorktree, and call it maybe worktree_clean(). And create a method in Index() called is_clean(tree) that checks whether the index is clean with respect to the given Tree (I think this method should just call "git diff-index --quiet --cached <tree>".). Then call both of these methods.

Sorry if I just keep creating more work for you. :-/

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle