Re: [PATCH 5/7] gitk: Make cherry-pick call git-citool on conflicts.

[Paul Mackerras <paulus@samba.org>]

Alexander Gavrilov writes:

> Now that git-gui has facilities to help users resolve
> conflicts, it makes sense to launch it from other gui
> tools when they happen.

Nice idea...

> +proc exec_citool {args {baseid {}}} {

I'm a little nervous about you having a parameter called "args", since
that specific name has a special meaning in Tcl; it's how Tcl handles
variable-length argument lists.

> +    global commitinfo env
> +
> +    if {[info exists env(GIT_AUTHOR_NAME)]} {
> +	set old_name $env(GIT_AUTHOR_NAME)
> +    }
> +    if {[info exists env(GIT_AUTHOR_EMAIL)]} {
> +	set old_email $env(GIT_AUTHOR_EMAIL)
> +    }
> +    if {[info exists env(GIT_AUTHOR_DATE)]} {
> +	set old_date $env(GIT_AUTHOR_DATE)
> +    }
> +
> +    if {$baseid ne {}} {
> +	if {![info exists commitinfo($baseid)]} {
> +	    getcommit $baseid
> +	}
> +	set author [lindex $commitinfo($baseid) 1]
> +	set date [lindex $commitinfo($baseid) 2]
> +	if {[regexp {^\s*(\S.*\S|\S)\s*<(.*)>\s*$} \
> +	            $author author name email]
> +	    && $date ne {}} {
> +	    set env(GIT_AUTHOR_NAME) $name
> +	    set env(GIT_AUTHOR_EMAIL) $email
> +	    set env(GIT_AUTHOR_DATE) $date
> +	}
> +    }
> +
> +    eval exec git citool $args &

If we can assume the existence of a shell (which we do elsewhere), we
can perhaps do this more simply by putting the environment variable
settings in the command before the command name.  It's a pity that git
citool won't take the author name/email/date as command-line arguments
or from a file, since it ends up being pretty verbose doing it the way
you have.

> @@ -7861,7 +7908,17 @@ proc cherrypick {} {
>      # no error occurs, and exec takes that as an indication of error...
>      if {[catch {exec sh -c "git cherry-pick -r $rowmenuid 2>&1"} err]} {
>  	notbusy cherrypick
> -	error_popup $err
> +	if {[regexp -line \
> +	    {Entry '(.*)' would be overwritten by merge} $err msg fname]} {
> +	    error_popup [mc "Cherry-pick failed: file '%s' had local modifications.
> +Your working directory is in an inconsistent state." $fname]

That message seems a bit too scary.  It's not inconsistent, it's just
got local modifications.  If I remember correctly, in this situation
git cherry-pick will back out all the changes it did and leave the
working directory as it was before.

> +	} elseif {[regexp -line {^CONFLICT \(.*\):} $err msg]} {
> +	    # Force citool to read MERGE_MSG
> +	    file delete [file join [gitdir] "GITGUI_MSG"]
> +	    exec_citool [list] $rowmenuid

[list] as an idiom for the empty list is a little unusual (here and
elsewhere in your patches); {} would be more usual.

Paul.