Yann Simon <yann.simon.fr@xxxxxxxxx> wrote:
> Add a field 'committer'.
> The fields 'author' and 'committer' are populated with the values
> found in the configuration.
>
> Validate the author and the committer.
>
> Add the signed-off line in the comment text box when the user clicks
> on the signed-off checkbox.
>
> Use Text.DELIMITER as line break for plateform independance.
Far too many things in one change. Please break these apart into
multiple commits so they are easier to review and test in isolation.
> I remove the modifications of PersonIdent and send them in another patch.
Thanks, that one is now applied.
> diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/CommitAction.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/CommitAction.java
> index ae26770..9a9d494 100644
> --- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/CommitAction.java
> +++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/actions/CommitAction.java
> @@ -99,15 +99,16 @@ public void run(IAction act) {
> }
>
> String author = null;
> + String committer = null;
> if (repository != null) {
> final RepositoryConfig config = repository.getConfig();
> author = config.getAuthorName();
> - if (author != null && author.length() != 0) {
> - final String email = config.getAuthorEmail();
> - if (email != null && email.length() != 0) {
> - author = author + " <" + email + ">";
> - }
> - }
> + final String authorEmail = config.getAuthorEmail();
> + author = author + " <" + authorEmail + ">";
> +
> + committer = config.getCommitterName();
> + final String committerEmail = config.getAuthorEmail();
> + committer = committer + " <" + committerEmail + ">";
Don't you mean getCommitterEmail() here?
> @@ -117,9 +118,13 @@ public void run(IAction act) {
> commitDialog.setAmendAllowed(amendAllowed);
> commitDialog.setFileList(files);
> commitDialog.setAuthor(author);
> + commitDialog.setCommitter(committer);
>
> - if (previousCommit != null)
> + if (previousCommit != null) {
> commitDialog.setPreviousCommitMessage(previousCommit.getMessage());
> + PersonIdent previousAuthor = previousCommit.getAuthor();
> + commitDialog.setPreviousAuthor(previousAuthor.getName() + " <" + previousAuthor.getEmailAddress() + ">");
Isn't this a bug fix for "amend" so that the original author is
reused when amending the prior commit? Please mention it in the
commit message; and ideally this should be its own change.
> @@ -312,6 +352,7 @@ private static String getFileStatus(IFile file) {
> }
>
> } catch (Exception e) {
> + e.printStackTrace();
Would it be better to log this through an Activator so it shows
up in the Error Log view?
> @@ -321,6 +362,7 @@ private static String getFileStatus(IFile file) {
> * @return The message the user entered
> */
> public String getCommitMessage() {
> + commitMessage.replaceAll(Text.DELIMITER, "\n"); //$NON-NLS-1$
> return commitMessage;
String is immutable. This replaceAll is a noop. You probably
meant to return the return value of the replaceAll method.
There may be other comments for these changes; it would be easier
to review if they were more broken up into independent commits.
--
Shawn.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
