opendev/gerrit
Code Issues Proposed changes

Merge branch 'stable-2.8'

Browse Source 
* stable-2.8:
  Allow large patches to function reasonably in diffs
This commit is contained in:
Shawn Pearce 2013-11-16 18:48:08 -08:00
commit fc798093b8
4 changed files with 52 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchScreen.java
View File

@ -50,6 +50,7 @@ import com.google.gwtexpui.globalkey.client.KeyCommandSet;
public abstract class PatchScreen extends Screen implements
CommentEditorContainer {
static final PrettyFactory PRETTY = ClientSideFormatter.FACTORY;
static final short LARGE_FILE_CONTEXT = 100;
public static class SideBySide extends PatchScreen {
public SideBySide(final Patch.Key id, final int patchIndex,
@ -460,6 +461,19 @@ public abstract class PatchScreen extends Screen implements
setToken(Dispatcher.toPatchUnified(idSideA, patchKey));
}
if (script.isHugeFile()) {
AccountDiffPreference dp = script.getDiffPrefs();
int context = dp.getContext();
if (context == AccountDiffPreference.WHOLE_FILE_CONTEXT) {
context = Short.MAX_VALUE;
} else if (context > Short.MAX_VALUE) {
context = Short.MAX_VALUE;
}
dp.setContext((short) Math.min(context, LARGE_FILE_CONTEXT));
dp.setSyntaxHighlighting(false);
script.setDiffPrefs(dp);
}
contentTable.display(patchKey, idSideA, idSideB, script, patchSetDetail);
contentTable.display(script.getCommentDetail(), script.isExpandAllComments());
contentTable.finishDisplay();

18
gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchScriptSettingsPanel.java
View File

@ -23,6 +23,9 @@ import com.google.gerrit.client.ui.NpIntTextBox;
import com.google.gerrit.reviewdb.client.AccountDiffPreference;
import com.google.gerrit.reviewdb.client.AccountDiffPreference.Whitespace;
import com.google.gwt.core.client.GWT;
import com.google.gwt.dom.client.NodeList;
import com.google.gwt.dom.client.OptionElement;
import com.google.gwt.dom.client.SelectElement;
import com.google.gwt.event.dom.client.ClickEvent;
import com.google.gwt.event.dom.client.KeyCodes;
import com.google.gwt.event.dom.client.KeyPressEvent;
@ -157,6 +160,21 @@ public class PatchScriptSettingsPanel extends Composite {
} else {
syntaxHighlighting.setValue(false);
}
NodeList<OptionElement> options =
context.getElement().<SelectElement>cast().getOptions();
// WHOLE_FILE_CONTEXT is the last option in the list.
int lastIndex = options.getLength() - 1;
OptionElement currOption = options.getItem(lastIndex);
if (enableSmallFileFeatures) {
currOption.setDisabled(false);
} else {
currOption.setDisabled(true);
if (context.getSelectedIndex() == lastIndex) {
// Select the next longest context from WHOLE_FILE_CONTEXT
context.setSelectedIndex(lastIndex - 1);
}
}
toggleEnabledStatus(save.isEnabled());
}

26
gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/SideBySideTable.java
View File

@ -52,6 +52,7 @@ public class SideBySideTable extends AbstractPatchContentTable {
private SparseHtmlFile a;
private SparseHtmlFile b;
private boolean isHugeFile;
protected boolean isFileCommentBorderRowExist;
protected void createFileCommentEditorOnSideA() {
@ -94,6 +95,7 @@ public class SideBySideTable extends AbstractPatchContentTable {
protected void render(final PatchScript script, final PatchSetDetail detail) {
final ArrayList<Object> lines = new ArrayList<Object>();
final SafeHtmlBuilder nc = new SafeHtmlBuilder();
isHugeFile = script.isHugeFile();
allocateTableHeader(script, nc);
lines.add(null);
if (!isDisplayBinary) {
@ -209,7 +211,7 @@ public class SideBySideTable extends AbstractPatchContentTable {
for (int row = 0; row < lines.size(); row++) {
setRowItem(row, lines.get(row));
if (lines.get(row) instanceof SkippedLine) {
createSkipLine(row, (SkippedLine) lines.get(row), script.getA().isWholeFile());
createSkipLine(row, (SkippedLine) lines.get(row), isHugeFile);
}
}
}
@ -540,18 +542,16 @@ public class SideBySideTable extends AbstractPatchContentTable {
if (numRows > 0) {
line.incrementStart(numRows);
// If we got here, we must have the whole file anyway.
createSkipLine(row + loopTo, line, true);
createSkipLine(row + loopTo, line, isHugeFile);
} else if (numRows < 0) {
line.reduceSize(-numRows);
// If we got here, we must have the whole file anyway.
createSkipLine(row, line, true);
createSkipLine(row, line, isHugeFile);
} else {
table.removeRow(row + loopTo);
}
}
private void createSkipLine(int row, SkippedLine line, boolean isWholeFile) {
private void createSkipLine(int row, SkippedLine line, boolean isHugeFile) {
FlowPanel p = new FlowPanel();
InlineLabel l1 = new InlineLabel(" " + PatchUtil.C.patchSkipRegionStart() + " ");
InlineLabel l2 = new InlineLabel(" " + PatchUtil.C.patchSkipRegionEnd() + " ");
@ -560,7 +560,7 @@ public class SideBySideTable extends AbstractPatchContentTable {
all.addClickHandler(expandAllListener);
all.setStyleName(Gerrit.RESOURCES.css().skipLine());
if (line.getSize() > 30 && isWholeFile) {
if (line.getSize() > 30) {
// Only show the expand before/after if skipped more than 30 lines.
Anchor b = new Anchor(PatchUtil.M.expandBefore(NUM_ROWS_TO_EXPAND), true);
Anchor a = new Anchor(PatchUtil.M.expandAfter(NUM_ROWS_TO_EXPAND), true);
@ -573,16 +573,16 @@ public class SideBySideTable extends AbstractPatchContentTable {
p.add(b);
p.add(l1);
p.add(all);
if (isHugeFile) {
p.add(new InlineLabel(" " + line.getSize() + " "));
} else {
p.add(all);
}
p.add(l2);
p.add(a);
} else if (isWholeFile) {
p.add(l1);
p.add(all);
p.add(l2);
} else {
p.add(l1);
p.add(new InlineLabel(" " + line.getSize() + " "));
p.add(all);
p.add(l2);
}
table.setWidget(row, 1, p);

16
gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptBuilder.java
View File

@ -193,18 +193,16 @@ class PatchScriptBuilder {
// IF the file is really large, we disable things to avoid choking
// the browser client.
//
diffPrefs.setContext((short) Math.min(25, context));
diffPrefs.setSyntaxHighlighting(false);
context = diffPrefs.getContext();
hugeFile = true;
} else {
// In order to expand the skipped common lines or syntax highlight the
// file properly we need to give the client the complete file contents.
// So force our context temporarily to the complete file size.
//
context = MAX_CONTEXT;
}
// In order to expand the skipped common lines or syntax highlight the
// file properly we need to give the client the complete file contents.
// So force our context temporarily to the complete file size.
//
context = MAX_CONTEXT;
packContent(diffPrefs.getIgnoreWhitespace() != Whitespace.IGNORE_NONE);
}