Compute the author/commiter name and email from the git configuration
PATCH 1/1 Compute the author/commiter name and email from the git configuration
The author/commiter name and email are retrieved from the system configuration and the local and global git configurations.
When the name is not available, propose one default by using the system user name.
When the email is not available, propose one default by concatenate the user name and the host name.
The author name and email are used as default value in the author field while committing with the GUI.
Fix issue 47 (field Author don't fill)
---
This is the second version of the patch.
The differences are the following ones:
- use System.getenv for environment variables
- use canonical host name.
- static cache the host name
.../egit/ui/internal/actions/CommitAction.java | 17 +++ .../org/spearce/jgit/lib/RepositoryConfigTest.java | 70 ++++++++++++ .../src/org/spearce/jgit/lib/Constants.java | 16 +++ .../src/org/spearce/jgit/lib/RepositoryConfig.java | 116 ++++++++++++++++++++ .../src/org/spearce/jgit/util/SystemReader.java | 24 ++++ 5 files changed, 243 insertions(+), 0 deletions(-) create mode 100644 org.spearce.jgit/src/org/spearce/jgit/util/SystemReader.java
org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/CommitAction.java +17 -0
@@ -41,6 +41,7 @@ import org.spearce.jgit.lib.PersonIdent; import org.spearce.jgit.lib.RefUpdate; import org.spearce.jgit.lib.Repository; +import org.spearce.jgit.lib.RepositoryConfig; import org.spearce.jgit.lib.Tree; import org.spearce.jgit.lib.TreeEntry; import org.spearce.jgit.lib.GitIndex.Entry;
@@ -70,8 +71,10 @@ public void run(IAction act) { } Repository[] repos = getRepositoriesFor(getProjectsForSelectedResources()); + Repository repository = null; amendAllowed = repos.length == 1; for (Repository repo : repos) { + repository = repo; if (!repo.getRepositoryState().canCommit()) { MessageDialog.openError(getTargetPart().getSite().getShell(), "Cannot commit now", "Repository state:"
@@ -95,12 +98,26 @@ public void run(IAction act) { } } + assert repository != null; + RepositoryConfig repositoryConfig = repository.getConfig(); + String author = null; + String username = repositoryConfig.getAuthorName(); + if (username != null && username.length() != 0) { + author = username; + String email = repositoryConfig.getAuthorEmail(); + if (email != null && email.length() != 0) { + author = author + " <" + email + ">"; + } + } + loadPreviousCommit(); CommitDialog commitDialog = new CommitDialog(getTargetPart().getSite().getShell()); commitDialog.setAmending(amending); commitDialog.setAmendAllowed(amendAllowed); commitDialog.setFileList(files); + commitDialog.setAuthor(author); + if (previousCommit != null) commitDialog.setPreviousCommitMessage(previousCommit.getMessage());
org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java +70 -0
81 lines not on this page · raw message
org.spearce.jgit/src/org/spearce/jgit/lib/Constants.java +16 -0
@@ -228,6 +228,22 @@ /** Packed refs file */ public static final String PACKED_REFS = "packed-refs"; + /** The environment variable that contains the system user name */ + public static final String OS_USER_NAME_KEY = "user.name"; + + /** The environment variable that contains the author's name */ + public static final String GIT_AUTHOR_NAME_KEY = "GIT_AUTHOR_NAME"; + + /** The environment variable that contains the author's email */ + public static final String GIT_AUTHOR_EMAIL_KEY = "GIT_AUTHOR_EMAIL"; + + /** The environment variable that contains the commiter's name */ + public static final String GIT_COMMITER_NAME_KEY = "GIT_COMMITER_NAME"; + + /** The environment variable that contains the commiter's email */ + public static final String GIT_COMMITER_EMAIL_KEY = "GIT_COMMITER_EMAIL"; + + /** * Create a new digest function for objects. *
org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java +116 -0
146 lines not on this page · raw message
org.spearce.jgit/src/org/spearce/jgit/util/SystemReader.java new file +24 -0
@@ -0,0 +1,24 @@ +package org.spearce.jgit.util; + +/** + * Interface to read values from the system. + * <p> + * When writing unit tests, extending this interface with a custom class + * permits to simulate an access to a system variable or property. + * </p> + */ +public interface SystemReader { + + /** + * @param variable system variable to read + * @return value of the system variable + */ + String getEnvironmentVariable(String variable); + + /** + * @param key of the system property to read + * @return value of the system property + */ + String getProperty(String key); + +}
Signature · 1 line
1.6.0.4
Quoted 8 lines · Yann Simon
org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java +0
-0@@ -116,4 +120,70 @@ public void test006_readCaseInsensitive() throws IOException { assertEquals(true, repositoryConfig.getBoolean("foo", null, "bar", false)); assertEquals("", repositoryConfig.getString("foo", null, "bar")); }
org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java public void test006_readCaseInsensitive() throws IOException { · line 132
Quoted 10 lines
org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java +10
-0+ + private class FakeSystemReader implements SystemReader { + Map<String, String> values = new HashMap<String, String>(); + public String getEnvironmentVariable(String variable) { + return values.get(variable); + } + public String getProperty(String key) { + return values.get(key); + } + }
I like this approach. Perhaps this should simply be done in RepositoryTestCase so all of our unit tests have a stable default identity, no matter what. They could change it on a test-by-test basis if needed, especially if the reader is created and populated with default values during the setUp() method.
org.spearce.jgit/src/org/spearce/jgit/lib/Constants.java line 241
Quoted 7 lines
org.spearce.jgit/src/org/spearce/jgit/lib/Constants.java +3
-0+ + /** The environment variable that contains the commiter's name */ + public static final String GIT_COMMITER_NAME_KEY = "GIT_COMMITER_NAME";
There are 2 't's in GIT_COMMITTER_NAME. Both the variable name and the value are wrong.
org.spearce.jgit/src/org/spearce/jgit/lib/Constants.java line 244
Quoted 2 lines
org.spearce.jgit/src/org/spearce/jgit/lib/Constants.java +2
-0+ /** The environment variable that contains the commiter's email */ + public static final String GIT_COMMITER_EMAIL_KEY = "GIT_COMMITER_EMAIL";
Again, 2 't's.
Quoted 5 lines
org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java +0
-0@@ -308,6 +323,83 @@ public String getString(final String section, String subsection, final String na
...
org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java public String getString(final String section, String subsection, final String na · line 340
Quoted 1 line
org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java +1
-0+ public String getCommiterName() {
2 't's in comitter.
org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java public String getString(final String section, String subsection, final String na · line 376
Quoted 1 line
org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java +1
-0+ public String getCommiterEmail() {
There are 2 't's in Committer.
Quoted 1 line
org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java +1
-0+ private String getUserEmailInternal(String gitVariableKey, boolean author) {
...
org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java public String getString(final String section, String subsection, final String na · line 396
Quoted 7 lines
org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java +7
-0+ // try to construct an email + String userName = author ? getAuthorName() : getCommiterName(); + if (userName != null && userName.length() != 0) { + String hostnameTmp = getHostname(); + if (hostnameTmp != null && hostnameTmp.length() != 0) { + email = userName + "@" + hostnameTmp; + }
If you do what I suggest below, getHostname() will never return null, so this logic will simplify out and we'll always be able to set email if we have a userName.
Also, I think this should be defaulting to the Java "user.name" property and not to the GIT_AUTHOR_NAME environment variable. So if the user.email isn't set, and the *_EMAIL env var isn't set, use the "user.name" property and the hostname to form the email address.
I don't think this should ever produce null. If we don't have a username from user.name, use some generic string like "unknown-user". Then we at least still have some identity data. Its very unlikely the JVM won't have a "user.name" property for us, so its very likely we won't get good tests in application level code for null return values.
org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java private static boolean eq(final String a, final String b) { · line 1053
Quoted 6 lines
org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java +5
-0@@ -957,4 +1049,28 @@ private static boolean eq(final String a, final String b) { + + /** + * @return the canonical hostname + */ + public static String getHostname() {
I don't see a reason for this to be public. Please make it private.
org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java private static boolean eq(final String a, final String b) { · line 1062
Quoted 6 lines
org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java +6
-0+ if (hostname == null) { + try { + InetAddress localMachine = InetAddress.getLocalHost(); + hostname = localMachine.getCanonicalHostName(); + } catch (UnknownHostException e) { + // we do nothing
Set hostname = "localhost" so we don't incur an UHE on every attempt to get the hostname. If it failed once, its very likely to fail again, and again, and again.
org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java private static boolean eq(final String a, final String b) { · line 1072
Quoted 5 lines
org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java +5
-0+ /** + * Overrides the default system reader by a custom one. + * @param systemReader new system reader + */ + public void setSystemReader(SystemReader systemReader) {
Method should be static.
org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java private static boolean eq(final String a, final String b) { · line 1073
Quoted 1 line
org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java +1
-0+ RepositoryConfig.systemReader = systemReader;
There is no need for RepositoryConfig here. Rename the parameter so it doesn't hide the field.
org.spearce.jgit/src/org/spearce/jgit/util/SystemReader.java line 1
Quoted 7 lines
org.spearce.jgit/src/org/spearce/jgit/util/SystemReader.java new file +1
-0@@ -0,0 +1,24 @@ +package org.spearce.jgit.util;
Missing copyright header.
org.spearce.jgit/src/org/spearce/jgit/util/SystemReader.java line 16
Quoted 5 lines
org.spearce.jgit/src/org/spearce/jgit/util/SystemReader.java new file +5
-0+ /** + * @param variable system variable to read + * @return value of the system variable + */ + String getEnvironmentVariable(String variable);
Maybe just emulate the System class and call this getenv() ?
Signature · 1 line
Shawn.