Skip to content

Commit

Permalink
git-gui: avoid persisting modified author identity
Browse files Browse the repository at this point in the history
Commit 7e71adc fixes a problem with git-gui failing to pick up the
original author identity during a commit --amend operation. However, the
new author details then become persistent for the remainder of the session.
This commit fixes this by ensuring the environment variables are reset
and the author information reset once the commit is completed.
The relevant changes were reworked to reduce global variables.

Signed-off-by: Pat Thoyts <patthoyts@users.sourceforge.net>
  • Loading branch information
patthoyts committed Oct 6, 2016
1 parent 7e71adc commit cfe616b
Showing 1 changed file with 31 additions and 20 deletions.
51 changes: 31 additions & 20 deletions lib/commit.tcl
@@ -1,13 +1,8 @@
# git-gui misc. commit reading/writing support
# Copyright (C) 2006, 2007 Shawn Pearce

set author_name ""
set author_email ""
set author_date ""

proc load_last_commit {} {
global HEAD PARENT MERGE_HEAD commit_type ui_comm
global author_name author_email author_date
global HEAD PARENT MERGE_HEAD commit_type ui_comm commit_author
global repo_config

if {[llength $PARENT] == 0} {
Expand Down Expand Up @@ -40,9 +35,7 @@ You are currently in the middle of a merge that has not been fully completed. Y
} elseif {[string match {encoding *} $line]} {
set enc [string tolower [string range $line 9 end]]
} elseif {[regexp "author (.*)\\s<(.*)>\\s(\\d.*$)" $line all name email time]} {
set author_name $name
set author_email $email
set author_date $time
set commit_author [list name $name email $email date $time]
}
}
set msg [read $fd]
Expand Down Expand Up @@ -115,13 +108,10 @@ proc do_signoff {} {
}

proc create_new_commit {} {
global commit_type ui_comm
global author_name author_email author_date
global commit_type ui_comm commit_author

set commit_type normal
set author_name ""
set author_email ""
set author_date ""
unset -nocomplain commit_author
$ui_comm delete 0.0 end
$ui_comm edit reset
$ui_comm edit modified false
Expand Down Expand Up @@ -335,12 +325,12 @@ proc commit_writetree {curHEAD msg_p} {
}

proc commit_committree {fd_wt curHEAD msg_p} {
global HEAD PARENT MERGE_HEAD commit_type
global HEAD PARENT MERGE_HEAD commit_type commit_author
global current_branch
global ui_comm selected_commit_type
global file_states selected_paths rescan_active
global repo_config
global env author_name author_email author_date
global env

gets $fd_wt tree_id
if {[catch {close $fd_wt} err]} {
Expand Down Expand Up @@ -380,10 +370,8 @@ A rescan will be automatically started now.
}
}

if {$author_name ne ""} {
set env(GIT_AUTHOR_NAME) $author_name
set env(GIT_AUTHOR_EMAIL) $author_email
set env(GIT_AUTHOR_DATE) $author_date
if {[info exists commit_author]} {
set old_author [commit_author_ident $commit_author]
}
# -- Create the commit.
#
Expand All @@ -397,8 +385,14 @@ A rescan will be automatically started now.
error_popup [strcat [mc "commit-tree failed:"] "\n\n$err"]
ui_status [mc "Commit failed."]
unlock_index
unset -nocomplain commit_author
commit_author_reset $old_author
return
}
if {[info exists commit_author]} {
unset -nocomplain commit_author

This comment has been minimized.

Copy link
@orgads

orgads Oct 7, 2016

Contributor

Do you need -nocomplain here?

This comment has been minimized.

Copy link
@patthoyts

patthoyts Oct 8, 2016

Author Owner

good point

commit_author_reset $old_author
}

# -- Update the HEAD ref.
#
Expand Down Expand Up @@ -525,3 +519,20 @@ proc commit_postcommit_wait {fd_ph cmt_id} {
}
fconfigure $fd_ph -blocking 0
}

proc commit_author_ident {details} {
global env
array set author $details
set old [array get env GIT_AUTHOR_*]
set env(GIT_AUTHOR_NAME) $author(name)
set env(GIT_AUTHOR_EMAIL) $author(email)
set env(GIT_AUTHOR_DATE) $author(date)
return $old
}
proc commit_author_reset {details} {
global env
unset env(GIT_AUTHOR_NAME) env(GIT_AUTHOR_EMAIL) env(GIT_AUTHOR_DATE)
if {$details ne {}} {
array set env $details
}
}

4 comments on commit cfe616b

@orgads
Copy link
Contributor

@orgads orgads commented on cfe616b Oct 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks!

The encoding bug is unresolved though.

@patthoyts
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One bug per feature branch! The encoding one is on another assuming you mean the patches from karsten blees that are on the git-for-windows release. The pu branch contains those + this one. As per the it repository, pu is subject to random rebasing but handy for testing what might end up in master.

@orgads
Copy link
Contributor

@orgads orgads commented on cfe616b Oct 9, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, sure. I didn't mean to include a fix on this branch.

I don't know what patch you mean. Couldn't find it. I referred to git-for-windows/git#761.

@patthoyts
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok - no. I suspect that is a problem with handling unicode environment variables in Tcl. I've not had any chance to dig into that.

Please sign in to comment.