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

["Alexander Gavrilov" <angavrilov@gmail.com>]

On Thu, Oct 9, 2008 at 11:42 AM, Paul Mackerras <paulus@samba.org> wrote:
>> +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.

Yes, I'd better change that. Probably it works only because that name is
special only as the last parameter.

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

If I understand correctly, using a shell would require composing the
command as a string, which itself requires quoting the author name & email,
and other argument strings. I did not feel confident enough to do that, so
chose a dumb but safe solution.

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

Yes, I'll have to reword it.

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

I'm more used to languages where lists and strings are very different types...
Even in perl you have to use [] for an empty list ref, and '' for an
empty string.

Alexander