From eb00ff3fa154f3b626fe56eb343cdc01b988b084 Mon Sep 17 00:00:00 2001 From: Bruce Zu Date: Tue, 27 Nov 2012 17:38:10 +0800 Subject: [PATCH] Allow merged change emails to include unified diff. If sendemail.includeDiff is true, the merged change emails from Gerrit will include the complete unified diff of the change. Thus users can watch the 'Watched Projects' by only following the emails of 'Submitted Changes' especially when there is not enough time to catch all emails of 'New Changes' and 'All Comments'. Change-Id: Ia7d3e2b1f46a0b45c3cff7c8d494c8ef6ea2213b --- Documentation/config-gerrit.txt | 7 ++- .../gerrit/server/mail/ChangeEmail.java | 52 ++++++++++++++++ .../gerrit/server/mail/NewChangeSender.java | 60 ------------------- .../com/google/gerrit/server/mail/Merged.vm | 4 ++ 4 files changed, 60 insertions(+), 63 deletions(-) diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index 63229e6742..eb166fe700 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -2075,9 +2075,10 @@ By default, unset, permitting delivery to any email address. [[sendemail.includeDiff]]sendemail.includeDiff:: + -If true, new change emails from Gerrit will include the complete -unified diff of the change. Variable maxmimumDiffSize places an upper -limit on how large the email can get when this option is enabled. +If true, new change emails and merged change emails from Gerrit +will include the complete unified diff of the change. +Variable maxmimumDiffSize places an upper limit on how large the +email can get when this option is enabled. + By default, false. diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java index 1bcf7321c7..0b71cb891e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java @@ -46,9 +46,15 @@ import com.google.gerrit.server.query.change.ChangeQueryBuilder; import com.google.gerrit.server.query.change.SingleGroupUser; import com.google.gwtorm.server.OrmException; +import org.eclipse.jgit.diff.DiffFormatter; +import org.eclipse.jgit.internal.JGitText; +import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.util.RawParseUtils; +import org.eclipse.jgit.util.TemporaryBuffer; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.io.IOException; import java.text.MessageFormat; import java.util.Collections; import java.util.Date; @@ -576,4 +582,50 @@ public abstract class ChangeEmail extends OutgoingEmail { velocityContext.put("patchSet", patchSet); velocityContext.put("patchSetInfo", patchSetInfo); } + + public boolean getIncludeDiff() { + return args.settings.includeDiff; + } + + /** Show patch set as unified difference. */ + public String getUnifiedDiff() { + PatchList patchList; + try { + patchList = getPatchList(); + if (patchList.getOldId() == null) { + // Octopus merges are not well supported for diff output by Gerrit. + // Currently these always have a null oldId in the PatchList. + return ""; + } + } catch (PatchListNotAvailableException e) { + log.error("Cannot format patch", e); + return ""; + } + + TemporaryBuffer.Heap buf = + new TemporaryBuffer.Heap(args.settings.maximumDiffSize); + DiffFormatter fmt = new DiffFormatter(buf); + Repository git; + try { + git = args.server.openRepository(change.getProject()); + } catch (IOException e) { + log.error("Cannot open repository to format patch", e); + return ""; + } + try { + fmt.setRepository(git); + fmt.setDetectRenames(true); + fmt.format(patchList.getOldId(), patchList.getNewId()); + return RawParseUtils.decode(buf.toByteArray()); + } catch (IOException e) { + if (JGitText.get().inMemoryBufferLimitExceeded.equals(e.getMessage())) { + return ""; + } + log.error("Cannot format patch", e); + return ""; + } finally { + fmt.release(); + git.close(); + } + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/NewChangeSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/NewChangeSender.java index 82c14059a8..2e459d40a1 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/NewChangeSender.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/NewChangeSender.java @@ -16,21 +16,10 @@ package com.google.gerrit.server.mail; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Change; -import com.google.gerrit.server.patch.PatchList; -import com.google.gerrit.server.patch.PatchListNotAvailableException; import com.google.gerrit.server.ssh.SshInfo; import com.jcraft.jsch.HostKey; -import org.eclipse.jgit.diff.DiffFormatter; -import org.eclipse.jgit.internal.JGitText; -import org.eclipse.jgit.lib.Repository; -import org.eclipse.jgit.util.RawParseUtils; -import org.eclipse.jgit.util.TemporaryBuffer; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import java.io.IOException; import java.util.ArrayList; import java.util.Collection; import java.util.HashSet; @@ -39,9 +28,6 @@ import java.util.Set; /** Sends an email alerting a user to a new change for them to review. */ public abstract class NewChangeSender extends ChangeEmail { - private static final Logger log = - LoggerFactory.getLogger(NewChangeSender.class); - private final SshInfo sshInfo; private final Set reviewers = new HashSet(); private final Set extraCC = new HashSet(); @@ -99,50 +85,4 @@ public abstract class NewChangeSender extends ChangeEmail { } return host; } - - public boolean getIncludeDiff() { - return args.settings.includeDiff; - } - - /** Show patch set as unified difference. */ - public String getUnifiedDiff() { - PatchList patchList; - try { - patchList = getPatchList(); - if (patchList.getOldId() == null) { - // Octopus merges are not well supported for diff output by Gerrit. - // Currently these always have a null oldId in the PatchList. - return ""; - } - } catch (PatchListNotAvailableException e) { - log.error("Cannot format patch", e); - return ""; - } - - TemporaryBuffer.Heap buf = - new TemporaryBuffer.Heap(args.settings.maximumDiffSize); - DiffFormatter fmt = new DiffFormatter(buf); - Repository git; - try { - git = args.server.openRepository(change.getProject()); - } catch (IOException e) { - log.error("Cannot open repository to format patch", e); - return ""; - } - try { - fmt.setRepository(git); - fmt.setDetectRenames(true); - fmt.format(patchList.getOldId(), patchList.getNewId()); - return RawParseUtils.decode(buf.toByteArray()); - } catch (IOException e) { - if (JGitText.get().inMemoryBufferLimitExceeded.equals(e.getMessage())) { - return ""; - } - log.error("Cannot format patch", e); - return ""; - } finally { - fmt.release(); - git.close(); - } - } } diff --git a/gerrit-server/src/main/resources/com/google/gerrit/server/mail/Merged.vm b/gerrit-server/src/main/resources/com/google/gerrit/server/mail/Merged.vm index bcbc7bdd4c..820806234b 100644 --- a/gerrit-server/src/main/resources/com/google/gerrit/server/mail/Merged.vm +++ b/gerrit-server/src/main/resources/com/google/gerrit/server/mail/Merged.vm @@ -42,3 +42,7 @@ Change subject: $change.subject $email.changeDetail$email.approvals + +#if($email.includeDiff) +$email.UnifiedDiff +#end