From 8e33d76853200f922b170aed7412761cd5c16dbb Mon Sep 17 00:00:00 2001 From: Sasa Zivkov Date: Wed, 21 Jul 2010 15:45:07 +0200 Subject: [PATCH] Refactoring AccountDiffPreference vs PatchScriptSettings+PrettySettings There was some code duplication in these classes. This refactoring removes the PatchScriptSettings and PrettySettings classes and uses AccountDiffPreference instead. Issue: bug 629 Change-Id: I57ab1522b0023503d0cbd29620236ea68b7717ed Signed-off-by: Sasa Zivkov --- .../common/data/PatchDetailService.java | 3 +- .../gerrit/common/data/PatchScript.java | 35 +++--- .../common/data/PatchScriptSettings.java | 63 ----------- .../gerrit/client/patches/PatchScreen.java | 27 +++-- .../patches/PatchScriptSettingsPanel.java | 98 ++++++---------- .../client/patches/SideBySideTable.java | 2 +- .../client/patches/UnifiedDiffTable.java | 2 +- .../rpc/patch/PatchDetailServiceImpl.java | 6 +- .../httpd/rpc/patch/PatchScriptBuilder.java | 23 ++-- .../httpd/rpc/patch/PatchScriptFactory.java | 16 +-- gerrit-prettify/pom.xml | 6 + .../prettify/client/ClientSideFormatter.java | 2 +- .../prettify/common/PrettyFormatter.java | 34 +++--- .../prettify/common/PrettySettings.java | 106 ------------------ .../reviewdb/AccountDiffPreference.java | 17 ++- 15 files changed, 130 insertions(+), 310 deletions(-) delete mode 100644 gerrit-common/src/main/java/com/google/gerrit/common/data/PatchScriptSettings.java delete mode 100644 gerrit-prettify/src/main/java/com/google/gerrit/prettify/common/PrettySettings.java diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchDetailService.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchDetailService.java index 287656690e..5aec0c766a 100644 --- a/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchDetailService.java +++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchDetailService.java @@ -16,6 +16,7 @@ package com.google.gerrit.common.data; import com.google.gerrit.common.auth.SignInRequired; import com.google.gerrit.reviewdb.Account; +import com.google.gerrit.reviewdb.AccountDiffPreference; import com.google.gerrit.reviewdb.ApprovalCategoryValue; import com.google.gerrit.reviewdb.Change; import com.google.gerrit.reviewdb.Patch; @@ -34,7 +35,7 @@ import java.util.Set; @RpcImpl(version = Version.V2_0) public interface PatchDetailService extends RemoteJsonService { void patchScript(Patch.Key key, PatchSet.Id a, PatchSet.Id b, - PatchScriptSettings settings, AsyncCallback callback); + AccountDiffPreference diffPrefs, AsyncCallback callback); @SignInRequired void saveDraft(PatchLineComment comment, diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchScript.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchScript.java index f2a2af1906..048d4408cb 100644 --- a/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchScript.java +++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchScript.java @@ -17,7 +17,6 @@ package com.google.gerrit.common.data; import com.google.gerrit.prettify.client.ClientSideFormatter; import com.google.gerrit.prettify.common.EditList; import com.google.gerrit.prettify.common.PrettyFormatter; -import com.google.gerrit.prettify.common.PrettySettings; import com.google.gerrit.prettify.common.SparseFileContent; import com.google.gerrit.prettify.common.SparseHtmlFile; import com.google.gerrit.reviewdb.AccountDiffPreference; @@ -40,7 +39,7 @@ public class PatchScript { protected String oldName; protected String newName; protected List header; - protected PatchScriptSettings settings; + protected AccountDiffPreference diffPrefs; protected SparseFileContent a; protected SparseFileContent b; protected List edits; @@ -52,7 +51,7 @@ public class PatchScript { protected boolean intralineDifference; public PatchScript(final Change.Key ck, final ChangeType ct, final String on, - final String nn, final List h, final PatchScriptSettings s, + final String nn, final List h, final AccountDiffPreference dp, final SparseFileContent ca, final SparseFileContent cb, final List e, final DisplayMethod ma, final DisplayMethod mb, final CommentDetail cd, final List hist, final boolean hf, @@ -62,7 +61,7 @@ public class PatchScript { oldName = on; newName = nn; header = h; - settings = s; + diffPrefs = dp; a = ca; b = cb; edits = e; @@ -113,12 +112,12 @@ public class PatchScript { return history; } - public PatchScriptSettings getSettings() { - return settings; + public AccountDiffPreference getDiffPrefs() { + return diffPrefs; } - public void setSettings(PatchScriptSettings s) { - settings = s; + public void setDiffPrefs(AccountDiffPreference dp) { + diffPrefs = dp; } public boolean isHugeFile() { @@ -126,7 +125,7 @@ public class PatchScript { } public boolean isIgnoreWhitespace() { - return settings.getWhitespace() != Whitespace.IGNORE_NONE; + return diffPrefs.getIgnoreWhitespace() != Whitespace.IGNORE_NONE; } public boolean hasIntralineDifference() { @@ -142,12 +141,12 @@ public class PatchScript { } public SparseHtmlFile getSparseHtmlFileA() { - PrettySettings s = new PrettySettings(settings.getPrettySettings()); - s.setFileName(a.getPath()); - s.setShowWhiteSpaceErrors(false); + AccountDiffPreference dp = new AccountDiffPreference(diffPrefs); + dp.setShowWhitespaceErrors(false); PrettyFormatter f = ClientSideFormatter.FACTORY.get(); - f.setPrettySettings(s); + f.setDiffPrefs(dp); + f.setFileName(a.getPath()); f.setEditFilter(PrettyFormatter.A); f.setEditList(edits); f.format(a); @@ -155,15 +154,15 @@ public class PatchScript { } public SparseHtmlFile getSparseHtmlFileB() { - PrettySettings s = new PrettySettings(settings.getPrettySettings()); - s.setFileName(b.getPath()); + AccountDiffPreference dp = new AccountDiffPreference(diffPrefs); PrettyFormatter f = ClientSideFormatter.FACTORY.get(); - f.setPrettySettings(s); + f.setDiffPrefs(dp); + f.setFileName(b.getPath()); f.setEditFilter(PrettyFormatter.B); f.setEditList(edits); - if (s.isSyntaxHighlighting() && a.isWholeFile() && !b.isWholeFile()) { + if (dp.isSyntaxHighlighting() && a.isWholeFile() && !b.isWholeFile()) { f.format(b.apply(a, edits)); } else { f.format(b); @@ -176,7 +175,7 @@ public class PatchScript { } public Iterable getHunks() { - int ctx = settings.getContext(); + int ctx = diffPrefs.getContext(); if (ctx == AccountDiffPreference.WHOLE_FILE_CONTEXT) { ctx = Math.max(a.size(), b.size()); } diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchScriptSettings.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchScriptSettings.java deleted file mode 100644 index 09fd241651..0000000000 --- a/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchScriptSettings.java +++ /dev/null @@ -1,63 +0,0 @@ -// Copyright (C) 2009 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.gerrit.common.data; - -import com.google.gerrit.prettify.common.PrettySettings; -import com.google.gerrit.reviewdb.AccountDiffPreference; -import com.google.gerrit.reviewdb.AccountDiffPreference.Whitespace; - -public class PatchScriptSettings { - protected int context; - protected Whitespace whitespace; - protected PrettySettings pretty; - - public PatchScriptSettings() { - context = AccountDiffPreference.DEFAULT_CONTEXT; - whitespace = Whitespace.IGNORE_NONE; - pretty = new PrettySettings(); - } - - public PatchScriptSettings(final PatchScriptSettings s) { - context = s.context; - whitespace = s.whitespace; - pretty = new PrettySettings(s.pretty); - } - - public PrettySettings getPrettySettings() { - return pretty; - } - - public void setPrettySettings(PrettySettings s) { - pretty = s; - } - - public int getContext() { - return context; - } - - public void setContext(final int ctx) { - assert 0 <= ctx || ctx == AccountDiffPreference.WHOLE_FILE_CONTEXT; - context = ctx; - } - - public Whitespace getWhitespace() { - return whitespace; - } - - public void setWhitespace(final Whitespace ws) { - assert ws != null; - whitespace = ws; - } -} diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchScreen.java index 9ecd161291..6c45138b6c 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchScreen.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchScreen.java @@ -27,7 +27,6 @@ import com.google.gerrit.client.ui.InlineHyperlink; import com.google.gerrit.client.ui.Screen; import com.google.gerrit.common.PageLinks; import com.google.gerrit.common.data.PatchScript; -import com.google.gerrit.common.data.PatchScriptSettings; import com.google.gerrit.common.data.PatchSetDetail; import com.google.gerrit.prettify.client.ClientSideFormatter; import com.google.gerrit.prettify.common.PrettyFactory; @@ -81,9 +80,9 @@ public abstract class PatchScreen extends Screen implements public Unified(final Patch.Key id, final int patchIndex, final PatchSetDetail patchSetDetail, final PatchTable patchTable) { super(id, patchIndex, patchSetDetail, patchTable); - final PatchScriptSettings s = settingsPanel.getValue(); - s.getPrettySettings().setSyntaxHighlighting(false); - settingsPanel.setValue(s); + final AccountDiffPreference dp = settingsPanel.getValue(); + dp.setSyntaxHighlighting(false); + settingsPanel.setValue(dp); } @Override @@ -181,9 +180,9 @@ public abstract class PatchScreen extends Screen implements settingsPanel = new PatchScriptSettingsPanel(); settingsPanel - .addValueChangeHandler(new ValueChangeHandler() { + .addValueChangeHandler(new ValueChangeHandler() { @Override - public void onValueChange(ValueChangeEvent event) { + public void onValueChange(ValueChangeEvent event) { update(event.getValue()); } }); @@ -206,9 +205,9 @@ public abstract class PatchScreen extends Screen implements lastScript = null; } - private void update(PatchScriptSettings s) { - if (lastScript != null && canReuse(s, lastScript)) { - lastScript.setSettings(s); + private void update(AccountDiffPreference dp) { + if (lastScript != null && canReuse(dp, lastScript)) { + lastScript.setDiffPrefs(dp); RpcStatus.INSTANCE.onRpcStart(null); settingsPanel.setEnabled(false); DeferredCommand.addCommand(new Command() { @@ -226,24 +225,24 @@ public abstract class PatchScreen extends Screen implements } } - private boolean canReuse(PatchScriptSettings s, PatchScript last) { - if (last.getSettings().getWhitespace() != s.getWhitespace()) { + private boolean canReuse(AccountDiffPreference dp, PatchScript last) { + if (last.getDiffPrefs().getIgnoreWhitespace() != dp.getIgnoreWhitespace()) { // Whitespace ignore setting requires server computation. return false; } - final int ctx = s.getContext(); + final int ctx = dp.getContext(); if (ctx == AccountDiffPreference.WHOLE_FILE_CONTEXT && !last.getA().isWholeFile()) { // We don't have the entire file here, so we can't render it. return false; } - if (last.getSettings().getContext() < ctx && !last.getA().isWholeFile()) { + if (last.getDiffPrefs().getContext() < ctx && !last.getA().isWholeFile()) { // We don't have sufficient context. return false; } - if (s.getPrettySettings().isSyntaxHighlighting() + if (dp.isSyntaxHighlighting() && !last.getA().isWholeFile()) { // We need the whole file to syntax highlight accurately. return false; diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchScriptSettingsPanel.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchScriptSettingsPanel.java index 3804580be3..758b0f0ad0 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchScriptSettingsPanel.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchScriptSettingsPanel.java @@ -18,8 +18,6 @@ import com.google.gerrit.client.Gerrit; import com.google.gerrit.client.account.Util; import com.google.gerrit.client.rpc.GerritCallback; import com.google.gerrit.client.ui.NpIntTextBox; -import com.google.gerrit.common.data.PatchScriptSettings; -import com.google.gerrit.prettify.common.PrettySettings; import com.google.gerrit.reviewdb.AccountDiffPreference; import com.google.gerrit.reviewdb.AccountDiffPreference.Whitespace; import com.google.gwt.core.client.GWT; @@ -44,13 +42,13 @@ import com.google.gwt.user.client.ui.Widget; import com.google.gwtjsonrpc.client.VoidResult; public class PatchScriptSettingsPanel extends Composite implements - HasValueChangeHandlers { + HasValueChangeHandlers { private static MyUiBinder uiBinder = GWT.create(MyUiBinder.class); interface MyUiBinder extends UiBinder { } - private PatchScriptSettings value; + private AccountDiffPreference value; private boolean enableIntralineDifference = true; private boolean enableSmallFileFeatures = true; @@ -118,15 +116,15 @@ public class PatchScriptSettingsPanel extends Composite implements colWidth.addKeyPressHandler(onEnter); if (Gerrit.isSignedIn() && Gerrit.getAccountDiffPreference() != null) { - setValue(createPatchScriptSettings(Gerrit.getAccountDiffPreference())); + setValue(Gerrit.getAccountDiffPreference()); } else { - setValue(new PatchScriptSettings()); + setValue(AccountDiffPreference.createDefault(null)); } } @Override public HandlerRegistration addValueChangeHandler( - ValueChangeHandler handler) { + ValueChangeHandler handler) { return super.addHandler(handler, ValueChangeEvent.getType()); } @@ -149,9 +147,7 @@ public class PatchScriptSettingsPanel extends Composite implements public void setEnableSmallFileFeatures(final boolean on) { enableSmallFileFeatures = on; if (enableSmallFileFeatures) { - final PrettySettings p = getValue().getPrettySettings(); - - syntaxHighlighting.setValue(p.isSyntaxHighlighting()); + syntaxHighlighting.setValue(value.isSyntaxHighlighting()); } else { syntaxHighlighting.setValue(false); } @@ -161,8 +157,7 @@ public class PatchScriptSettingsPanel extends Composite implements public void setEnableIntralineDifference(final boolean on) { enableIntralineDifference = on; if (enableIntralineDifference) { - final PrettySettings p = getValue().getPrettySettings(); - intralineDifference.setValue(p.isIntralineDifference()); + intralineDifference.setValue(value.isIntralineDifference()); } else { intralineDifference.setValue(false); } @@ -182,28 +177,26 @@ public class PatchScriptSettingsPanel extends Composite implements return reviewed; } - public PatchScriptSettings getValue() { + public AccountDiffPreference getValue() { return value; } - public void setValue(final PatchScriptSettings s) { - final PrettySettings p = s.getPrettySettings(); - - setIgnoreWhitespace(s.getWhitespace()); + public void setValue(final AccountDiffPreference dp) { + setIgnoreWhitespace(dp.getIgnoreWhitespace()); if (enableSmallFileFeatures) { - syntaxHighlighting.setValue(p.isSyntaxHighlighting()); + syntaxHighlighting.setValue(dp.isSyntaxHighlighting()); } else { syntaxHighlighting.setValue(false); } - setContext(s.getContext()); + setContext(dp.getContext()); - tabWidth.setIntValue(p.getTabSize()); - colWidth.setIntValue(p.getLineLength()); - intralineDifference.setValue(p.isIntralineDifference()); - whitespaceErrors.setValue(p.isShowWhiteSpaceErrors()); - showTabs.setValue(p.isShowTabs()); + tabWidth.setIntValue(dp.getTabSize()); + colWidth.setIntValue(dp.getLineLength()); + intralineDifference.setValue(dp.isIntralineDifference()); + whitespaceErrors.setValue(dp.isShowWhitespaceErrors()); + showTabs.setValue(dp.isShowTabs()); - value = s; + value = dp; } @UiHandler("update") @@ -212,20 +205,18 @@ public class PatchScriptSettingsPanel extends Composite implements } private void update() { - PatchScriptSettings s = new PatchScriptSettings(getValue()); - PrettySettings p = s.getPrettySettings(); + AccountDiffPreference dp = new AccountDiffPreference(value); + dp.setIgnoreWhitespace(getIgnoreWhitespace()); + dp.setContext(getContext()); + dp.setTabSize(tabWidth.getIntValue()); + dp.setLineLength(colWidth.getIntValue()); + dp.setSyntaxHighlighting(syntaxHighlighting.getValue()); + dp.setIntralineDifference(intralineDifference.getValue()); + dp.setShowWhitespaceErrors(whitespaceErrors.getValue()); + dp.setShowTabs(showTabs.getValue()); - s.setWhitespace(getIgnoreWhitespace()); - s.setContext(getContext()); - p.setTabSize(tabWidth.getIntValue()); - p.setLineLength(colWidth.getIntValue()); - p.setSyntaxHighlighting(syntaxHighlighting.getValue()); - p.setIntralineDifference(intralineDifference.getValue()); - p.setShowWhiteSpaceErrors(whitespaceErrors.getValue()); - p.setShowTabs(showTabs.getValue()); - - value = s; - fireEvent(new ValueChangeEvent(s) {}); + value = dp; + fireEvent(new ValueChangeEvent(dp) {}); if (Gerrit.isSignedIn()) { persistDiffPreferences(); @@ -234,19 +225,10 @@ public class PatchScriptSettingsPanel extends Composite implements private void persistDiffPreferences() { setEnabled(false); - final AccountDiffPreference diffPref = new AccountDiffPreference(Gerrit.getUserAccount().getId()); - diffPref.setIgnoreWhitespace(getIgnoreWhitespace()); - diffPref.setTabSize(tabWidth.getIntValue()); - diffPref.setLineLength(colWidth.getIntValue()); - diffPref.setSyntaxHighlighting(syntaxHighlighting.getValue()); - diffPref.setShowWhitespaceErrors(whitespaceErrors.getValue()); - diffPref.setIntralineDifference(intralineDifference.getValue()); - diffPref.setShowTabs(showTabs.getValue()); - diffPref.setContext(getContext()); - Util.ACCOUNT_SVC.changeDiffPreferences(diffPref, new GerritCallback() { + Util.ACCOUNT_SVC.changeDiffPreferences(value, new GerritCallback() { @Override public void onSuccess(VoidResult result) { - Gerrit.setAccountDiffPreference(diffPref); + Gerrit.setAccountDiffPreference(value); setEnabled(true); } @@ -285,7 +267,7 @@ public class PatchScriptSettingsPanel extends Composite implements if (0 <= sel) { return Whitespace.valueOf(ignoreWhitespace.getValue(sel)); } - return value.getWhitespace(); + return value.getIgnoreWhitespace(); } private void setIgnoreWhitespace(Whitespace s) { @@ -316,20 +298,4 @@ public class PatchScriptSettingsPanel extends Composite implements } context.setSelectedIndex(0); } - - private PatchScriptSettings createPatchScriptSettings(AccountDiffPreference diffPref) { - final PatchScriptSettings s = new PatchScriptSettings(); - if (diffPref != null) { - s.setWhitespace(diffPref.getIgnoreWhitespace()); - s.setContext(diffPref.getContext()); - final PrettySettings p = s.getPrettySettings(); - p.setTabSize(diffPref.getTabSize()); - p.setLineLength(diffPref.getLineLength()); - p.setSyntaxHighlighting(diffPref.isSyntaxHighlighting()); - p.setIntralineDifference(diffPref.isIntralineDifference()); - p.setShowWhiteSpaceErrors(diffPref.isShowWhitespaceErrors()); - p.setShowTabs(diffPref.isShowTabs()); - } - return s; - } } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/SideBySideTable.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/SideBySideTable.java index 9b42573cb3..1bc4dd5c14 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/SideBySideTable.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/SideBySideTable.java @@ -75,7 +75,7 @@ public class SideBySideTable extends AbstractPatchContentTable { final ArrayList lines = new ArrayList(); final SafeHtmlBuilder nc = new SafeHtmlBuilder(); final boolean intraline = - script.getSettings().getPrettySettings().isIntralineDifference() + script.getDiffPrefs().isIntralineDifference() && script.hasIntralineDifference(); appendHeader(script, nc); diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/UnifiedDiffTable.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/UnifiedDiffTable.java index b2be30a1b6..508e508282 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/UnifiedDiffTable.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/UnifiedDiffTable.java @@ -133,7 +133,7 @@ public class UnifiedDiffTable extends AbstractPatchContentTable { } final boolean syntaxHighlighting = - script.getSettings().getPrettySettings().isSyntaxHighlighting(); + script.getDiffPrefs().isSyntaxHighlighting(); final ArrayList lines = new ArrayList(); for (final EditList.Hunk hunk : script.getHunks()) { appendHunkHeader(nc, hunk); diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java index b0ee4d9593..da52596903 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java @@ -20,11 +20,11 @@ import com.google.gerrit.common.data.ApprovalSummarySet; import com.google.gerrit.common.data.ApprovalTypes; import com.google.gerrit.common.data.PatchDetailService; import com.google.gerrit.common.data.PatchScript; -import com.google.gerrit.common.data.PatchScriptSettings; import com.google.gerrit.common.errors.NoSuchEntityException; import com.google.gerrit.httpd.rpc.BaseServiceImplementation; import com.google.gerrit.httpd.rpc.Handler; import com.google.gerrit.reviewdb.Account; +import com.google.gerrit.reviewdb.AccountDiffPreference; import com.google.gerrit.reviewdb.AccountPatchReview; import com.google.gerrit.reviewdb.ApprovalCategory; import com.google.gerrit.reviewdb.ApprovalCategoryValue; @@ -92,13 +92,13 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements } public void patchScript(final Patch.Key patchKey, final PatchSet.Id psa, - final PatchSet.Id psb, final PatchScriptSettings s, + final PatchSet.Id psb, final AccountDiffPreference dp, final AsyncCallback callback) { if (psb == null) { callback.onFailure(new NoSuchEntityException()); return; } - patchScriptFactoryFactory.create(patchKey, psa, psb, s).to(callback); + patchScriptFactoryFactory.create(patchKey, psa, psb, dp).to(callback); } public void saveDraft(final PatchLineComment comment, diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchScriptBuilder.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchScriptBuilder.java index a8176bdfe2..a58c7b954f 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchScriptBuilder.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchScriptBuilder.java @@ -16,7 +16,6 @@ package com.google.gerrit.httpd.rpc.patch; import com.google.gerrit.common.data.CommentDetail; import com.google.gerrit.common.data.PatchScript; -import com.google.gerrit.common.data.PatchScriptSettings; import com.google.gerrit.common.data.PatchScript.DisplayMethod; import com.google.gerrit.prettify.common.EditList; import com.google.gerrit.prettify.common.SparseFileContent; @@ -66,7 +65,7 @@ class PatchScriptBuilder { private Repository db; private Change change; - private PatchScriptSettings settings; + private AccountDiffPreference diffPrefs; private ObjectId aId; private ObjectId bId; @@ -92,10 +91,10 @@ class PatchScriptBuilder { this.change = c; } - void setSettings(final PatchScriptSettings s) { - settings = s; + void setDiffPrefs(final AccountDiffPreference dp) { + diffPrefs = dp; - context = settings.getContext(); + context = diffPrefs.getContext(); if (context == AccountDiffPreference.WHOLE_FILE_CONTEXT) { context = MAX_CONTEXT; } else if (context > MAX_CONTEXT) { @@ -117,7 +116,7 @@ class PatchScriptBuilder { // return new PatchScript(change.getKey(), content.getChangeType(), content .getOldName(), content.getNewName(), content.getHeaderLines(), - settings, a.dst, b.dst, Collections. emptyList(), + diffPrefs, a.dst, b.dst, Collections. emptyList(), a.displayMethod, b.displayMethod, comments, history, false, false); } @@ -150,24 +149,24 @@ class PatchScriptBuilder { // IF the file is really large, we disable things to avoid choking // the browser client. // - settings.setContext(Math.min(25, context)); - settings.getPrettySettings().setSyntaxHighlighting(false); - context = settings.getContext(); + diffPrefs.setContext((short) Math.min(25, context)); + diffPrefs.setSyntaxHighlighting(false); + context = diffPrefs.getContext(); hugeFile = true; - } else if (settings.getPrettySettings().isSyntaxHighlighting()) { + } else if (diffPrefs.isSyntaxHighlighting()) { // In order to 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(settings.getWhitespace() != Whitespace.IGNORE_NONE); + packContent(diffPrefs.getIgnoreWhitespace() != Whitespace.IGNORE_NONE); } return new PatchScript(change.getKey(), content.getChangeType(), content .getOldName(), content.getNewName(), content.getHeaderLines(), - settings, a.dst, b.dst, edits, a.displayMethod, b.displayMethod, + diffPrefs, a.dst, b.dst, edits, a.displayMethod, b.displayMethod, comments, history, hugeFile, intralineDifference); } diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchScriptFactory.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchScriptFactory.java index 969dcc9e33..ee8241837b 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchScriptFactory.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchScriptFactory.java @@ -16,9 +16,9 @@ package com.google.gerrit.httpd.rpc.patch; import com.google.gerrit.common.data.CommentDetail; import com.google.gerrit.common.data.PatchScript; -import com.google.gerrit.common.data.PatchScriptSettings; import com.google.gerrit.httpd.rpc.Handler; import com.google.gerrit.reviewdb.Account; +import com.google.gerrit.reviewdb.AccountDiffPreference; import com.google.gerrit.reviewdb.Change; import com.google.gerrit.reviewdb.Patch; import com.google.gerrit.reviewdb.PatchLineComment; @@ -62,7 +62,7 @@ class PatchScriptFactory extends Handler { PatchScriptFactory create(Patch.Key patchKey, @Assisted("patchSetA") PatchSet.Id patchSetA, @Assisted("patchSetB") PatchSet.Id patchSetB, - PatchScriptSettings settings); + AccountDiffPreference diffPrefs); } private static final Logger log = @@ -79,7 +79,7 @@ class PatchScriptFactory extends Handler { @Nullable private final PatchSet.Id psa; private final PatchSet.Id psb; - private final PatchScriptSettings settings; + private final AccountDiffPreference diffPrefs; private final PatchSet.Id patchSetId; private final Change.Id changeId; @@ -102,7 +102,7 @@ class PatchScriptFactory extends Handler { @Assisted final Patch.Key patchKey, @Assisted("patchSetA") @Nullable final PatchSet.Id patchSetA, @Assisted("patchSetB") final PatchSet.Id patchSetB, - @Assisted final PatchScriptSettings settings) { + @Assisted final AccountDiffPreference diffPrefs) { this.repoManager = grm; this.builderFactory = builderFactory; this.patchListCache = patchListCache; @@ -113,7 +113,7 @@ class PatchScriptFactory extends Handler { this.patchKey = patchKey; this.psa = patchSetA; this.psb = patchSetB; - this.settings = settings; + this.diffPrefs = diffPrefs; patchSetId = patchKey.getParentKey(); changeId = patchSetId.getParentKey(); @@ -143,7 +143,7 @@ class PatchScriptFactory extends Handler { throw new NoSuchChangeException(changeId, e); } try { - final PatchList list = listFor(keyFor(settings.getWhitespace())); + final PatchList list = listFor(keyFor(diffPrefs.getIgnoreWhitespace())); final boolean intraline = list.hasIntralineDifference(); final PatchScriptBuilder b = newBuilder(list, git); final PatchListEntry content = list.get(patchKey.getFileName()); @@ -172,11 +172,11 @@ class PatchScriptFactory extends Handler { } private PatchScriptBuilder newBuilder(final PatchList list, Repository git) { - final PatchScriptSettings s = new PatchScriptSettings(settings); + final AccountDiffPreference dp = new AccountDiffPreference(diffPrefs); final PatchScriptBuilder b = builderFactory.get(); b.setRepository(git); b.setChange(change); - b.setSettings(s); + b.setDiffPrefs(dp); b.setTrees(list.getOldId(), list.getNewId()); return b; } diff --git a/gerrit-prettify/pom.xml b/gerrit-prettify/pom.xml index fb0860e0db..99eece6076 100644 --- a/gerrit-prettify/pom.xml +++ b/gerrit-prettify/pom.xml @@ -44,6 +44,12 @@ limitations under the License. ${project.version} + + com.google.gerrit + gerrit-reviewdb + ${project.version} + + com.google.gwt gwt-user diff --git a/gerrit-prettify/src/main/java/com/google/gerrit/prettify/client/ClientSideFormatter.java b/gerrit-prettify/src/main/java/com/google/gerrit/prettify/client/ClientSideFormatter.java index 1b03c625b9..4bce954e35 100644 --- a/gerrit-prettify/src/main/java/com/google/gerrit/prettify/client/ClientSideFormatter.java +++ b/gerrit-prettify/src/main/java/com/google/gerrit/prettify/client/ClientSideFormatter.java @@ -52,7 +52,7 @@ public class ClientSideFormatter extends PrettyFormatter { @Override protected String prettify(String html, String type) { - return go(prettify.getContext(), html, type, settings.getTabSize()); + return go(prettify.getContext(), html, type, diffPrefs.getTabSize()); } private static native String go(JavaScriptObject ctx, String srcText, diff --git a/gerrit-prettify/src/main/java/com/google/gerrit/prettify/common/PrettyFormatter.java b/gerrit-prettify/src/main/java/com/google/gerrit/prettify/common/PrettyFormatter.java index 4406477ca7..5d1592d1a8 100644 --- a/gerrit-prettify/src/main/java/com/google/gerrit/prettify/common/PrettyFormatter.java +++ b/gerrit-prettify/src/main/java/com/google/gerrit/prettify/common/PrettyFormatter.java @@ -14,6 +14,7 @@ package com.google.gerrit.prettify.common; +import com.google.gerrit.reviewdb.AccountDiffPreference; import com.google.gwtexpui.safehtml.client.SafeHtml; import com.google.gwtexpui.safehtml.client.SafeHtmlBuilder; @@ -71,7 +72,8 @@ public abstract class PrettyFormatter implements SparseHtmlFile { protected SparseFileContent content; protected EditFilter side; protected List edits; - protected PrettySettings settings; + protected AccountDiffPreference diffPrefs; + protected String fileName; protected Set trailingEdits; private int col; @@ -105,8 +107,12 @@ public abstract class PrettyFormatter implements SparseHtmlFile { edits = all; } - public void setPrettySettings(PrettySettings how) { - settings = how; + public void setDiffPrefs(AccountDiffPreference how) { + diffPrefs = how; + } + + public void setFileName(String fileName) { + this.fileName = fileName; } /** @@ -122,7 +128,7 @@ public abstract class PrettyFormatter implements SparseHtmlFile { String html = toHTML(src); - if (settings.isSyntaxHighlighting() && getFileType() != null + if (diffPrefs.isSyntaxHighlighting() && getFileType() != null && src.isWholeFile()) { // The prettify parsers don't like ' as an entity for the // single quote character. Replace them all out so we don't @@ -205,7 +211,7 @@ public abstract class PrettyFormatter implements SparseHtmlFile { cleanText(txt, pos, start); pos = txt.indexOf(';', start + 1) + 1; - if (settings.getLineLength() <= col) { + if (diffPrefs.getLineLength() <= col) { buf.append("
"); col = 0; } @@ -219,14 +225,14 @@ public abstract class PrettyFormatter implements SparseHtmlFile { private void cleanText(String txt, int pos, int end) { while (pos < end) { - int free = settings.getLineLength() - col; + int free = diffPrefs.getLineLength() - col; if (free <= 0) { // The current line is full. Throw an explicit line break // onto the end, and we'll continue on the next line. // buf.append("
"); col = 0; - free = settings.getLineLength(); + free = diffPrefs.getLineLength(); } int n = Math.min(end - pos, free); @@ -305,7 +311,7 @@ public abstract class PrettyFormatter implements SparseHtmlFile { private String toHTML(SparseFileContent src) { SafeHtml html; - if (settings.isIntralineDifference()) { + if (diffPrefs.isIntralineDifference()) { html = colorLineEdits(src); } else { SafeHtmlBuilder b = new SafeHtmlBuilder(); @@ -321,7 +327,7 @@ public abstract class PrettyFormatter implements SparseHtmlFile { html = html.replaceAll("\r([^\n])", r); } - if (settings.isShowWhiteSpaceErrors()) { + if (diffPrefs.isShowWhitespaceErrors()) { // We need to do whitespace errors before showing tabs, because // these patterns rely on \t as a literal, before it expands. // @@ -329,8 +335,8 @@ public abstract class PrettyFormatter implements SparseHtmlFile { html = showTrailingWhitespace(html); } - if (settings.isShowTabs()) { - String t = 1 < settings.getTabSize() ? "\t" : ""; + if (diffPrefs.isShowTabs()) { + String t = 1 < diffPrefs.getTabSize() ? "\t" : ""; html = html.replaceAll("\t", "\u00BB" + t); } @@ -496,17 +502,17 @@ public abstract class PrettyFormatter implements SparseHtmlFile { private String expandTabs(String html) { StringBuilder tmp = new StringBuilder(); int i = 0; - if (settings.isShowTabs()) { + if (diffPrefs.isShowTabs()) { i = 1; } - for (; i < settings.getTabSize(); i++) { + for (; i < diffPrefs.getTabSize(); i++) { tmp.append(" "); } return html.replaceAll("\t", tmp.toString()); } private String getFileType() { - String srcType = settings.getFilename(); + String srcType = fileName; if (srcType == null) { return null; } diff --git a/gerrit-prettify/src/main/java/com/google/gerrit/prettify/common/PrettySettings.java b/gerrit-prettify/src/main/java/com/google/gerrit/prettify/common/PrettySettings.java deleted file mode 100644 index 3ef17f540a..0000000000 --- a/gerrit-prettify/src/main/java/com/google/gerrit/prettify/common/PrettySettings.java +++ /dev/null @@ -1,106 +0,0 @@ -// Copyright (C) 2010 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.gerrit.prettify.common; - -/** Settings to configure a {@link PrettyFormatter}. */ -public class PrettySettings { - protected String fileName; - protected boolean showWhiteSpaceErrors; - protected int lineLength; - protected int tabSize; - protected boolean showTabs; - protected boolean syntaxHighlighting; - protected boolean intralineDifference; - - public PrettySettings() { - showWhiteSpaceErrors = true; - lineLength = 100; - tabSize = 8; - showTabs = true; - syntaxHighlighting = true; - intralineDifference = true; - } - - public PrettySettings(PrettySettings pretty) { - fileName = pretty.fileName; - showWhiteSpaceErrors = pretty.showWhiteSpaceErrors; - lineLength = pretty.lineLength; - tabSize = pretty.tabSize; - showTabs = pretty.showTabs; - syntaxHighlighting = pretty.syntaxHighlighting; - intralineDifference = pretty.intralineDifference; - } - - public String getFilename() { - return fileName; - } - - public PrettySettings setFileName(final String name) { - fileName = name; - return this; - } - - public boolean isShowWhiteSpaceErrors() { - return showWhiteSpaceErrors; - } - - public PrettySettings setShowWhiteSpaceErrors(final boolean show) { - showWhiteSpaceErrors = show; - return this; - } - - public int getLineLength() { - return lineLength; - } - - public PrettySettings setLineLength(final int len) { - lineLength = len; - return this; - } - - public int getTabSize() { - return tabSize; - } - - public PrettySettings setTabSize(final int len) { - tabSize = len; - return this; - } - - public boolean isShowTabs() { - return showTabs; - } - - public PrettySettings setShowTabs(final boolean show) { - showTabs = show; - return this; - } - - public boolean isSyntaxHighlighting() { - return syntaxHighlighting; - } - - public void setSyntaxHighlighting(final boolean on) { - syntaxHighlighting = on; - } - - public boolean isIntralineDifference() { - return intralineDifference; - } - - public void setIntralineDifference(final boolean on) { - intralineDifference = on; - } -} diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountDiffPreference.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountDiffPreference.java index 426eb375de..4a3dd18120 100644 --- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountDiffPreference.java +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/AccountDiffPreference.java @@ -103,6 +103,18 @@ public class AccountDiffPreference { this.accountId = accountId; } + public AccountDiffPreference(AccountDiffPreference p) { + this.accountId = p.accountId; + this.ignoreWhitespace = p.ignoreWhitespace; + this.tabSize = p.tabSize; + this.lineLength = p.lineLength; + this.syntaxHighlighting = p.syntaxHighlighting; + this.showWhitespaceErrors = p.showWhitespaceErrors; + this.intralineDifference = p.intralineDifference; + this.showTabs = p.showTabs; + this.context = p.context; + } + public Account.Id getAccountId() { return accountId; } @@ -169,7 +181,8 @@ public class AccountDiffPreference { } /** Set the number of lines of context when viewing a patch. */ - public void setContext(final short s) { - context = s; + public void setContext(final short context) { + assert 0 <= context || context == WHOLE_FILE_CONTEXT; + this.context = context; } }