opendev/gerrit
Code Issues Proposed changes

Minor refactor of diff code to prepare for unified view

Browse Source 
Pulled some classes out of SideBySide2. There will be a lot of
common code between the upcoming Unified2 and SideBySide2.

Fixed a minor bug in skip line rendering. It was still relying
on Side.PARENT. Changed to use DisplaySide.A.

Deleted deprecated PatchSelectBox2.

Change-Id: I87d608d7e6a4330096d65b0fc1b99c1c5160d119
This commit is contained in:
Michael Zhou
2013-08-21 18:02:11 -07:00
parent d68a4b9584
commit 99c3d2b87a
13 changed files with 171 additions and 163 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentBox.java
View File

@@ -16,8 +16,6 @@ package com.google.gerrit.client.diff;
import com.google.gerrit.client.changes.CommentInfo;
import com.google.gerrit.client.diff.PaddingManager.PaddingWidgetWrapper;
import com.google.gerrit.client.diff.SideBySide2.DisplaySide;
import com.google.gerrit.client.diff.SideBySide2.DiffChunkInfo;
import com.google.gerrit.client.diff.SidePanel.GutterWrapper;
import com.google.gwt.core.client.Scheduler;
import com.google.gwt.core.client.Scheduler.ScheduledCommand;

46
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffChunkInfo.java Normal file
View File

@@ -0,0 +1,46 @@
//Copyright (C) 2013 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.client.diff;
/** Object recording the position of a diff chunk and whether it's an edit */
class DiffChunkInfo {
private DisplaySide side;
private int start;
private int end;
private boolean edit;
DiffChunkInfo(DisplaySide side, int start, int end, boolean edit) {
this.side = side;
this.start = start;
this.end = end;
this.edit = edit;
}
DisplaySide getSide() {
return side;
}
int getStart() {
return start;
}
int getEnd() {
return end;
}
boolean isEdit() {
return edit;
}
}

1
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffTable.java
View File

@@ -15,7 +15,6 @@
package com.google.gerrit.client.diff;
import com.google.gerrit.client.changes.ChangeInfo.RevisionInfo;
import com.google.gerrit.client.diff.SideBySide2.DisplaySide;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gwt.core.client.GWT;
import com.google.gwt.core.client.JsArray;

20
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DisplaySide.java Normal file
View File

@@ -0,0 +1,20 @@
// Copyright (C) 2013 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.client.diff;
/** Enum representing the side on a side-by-side view */
enum DisplaySide {
A, B;
}

1
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DraftBox.java
View File

@@ -18,7 +18,6 @@ import com.google.gerrit.client.FormatUtil;
import com.google.gerrit.client.changes.CommentApi;
import com.google.gerrit.client.changes.CommentInfo;
import com.google.gerrit.client.changes.CommentInput;
import com.google.gerrit.client.diff.SideBySide2.DisplaySide;
import com.google.gerrit.client.rpc.GerritCallback;
import com.google.gerrit.client.ui.CommentLinkProcessor;
import com.google.gerrit.reviewdb.client.PatchSet;

57
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/EditIterator.java Normal file
View File

@@ -0,0 +1,57 @@
// Copyright (C) 2013 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.client.diff;
import com.google.gwt.core.client.JsArrayString;
import net.codemirror.lib.LineCharacter;
/** An iterator for intraline edits */
class EditIterator {
private final JsArrayString lines;
private final int startLine;
private int currLineIndex;
private int currLineOffset;
EditIterator(JsArrayString lineArray, int start) {
lines = lineArray;
startLine = start;
}
LineCharacter advance(int numOfChar) {
while (currLineIndex < lines.length()) {
int lengthWithNewline =
lines.get(currLineIndex).length() - currLineOffset + 1;
if (numOfChar < lengthWithNewline) {
LineCharacter at = LineCharacter.create(
startLine + currLineIndex,
numOfChar + currLineOffset);
currLineOffset += numOfChar;
return at;
}
numOfChar -= lengthWithNewline;
advanceLine();
if (numOfChar == 0) {
return LineCharacter.create(startLine + currLineIndex, 0);
}
}
throw new IllegalStateException("EditIterator index out of bound");
}
private void advanceLine() {
currLineIndex++;
currLineOffset = 0;
}
}

1
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/FileCommentPanel.java
View File

@@ -16,7 +16,6 @@ package com.google.gerrit.client.diff;
import com.google.gerrit.client.Gerrit;
import com.google.gerrit.client.changes.CommentInfo;
import com.google.gerrit.client.diff.SideBySide2.DisplaySide;
import com.google.gwt.user.client.ui.Composite;
import com.google.gwt.user.client.ui.FlowPanel;

2
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/LineMapper.java
View File

@@ -14,8 +14,6 @@
package com.google.gerrit.client.diff;
import com.google.gerrit.client.diff.SideBySide2.DisplaySide;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

29
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/NoOpKeyCommand.java Normal file
View File

@@ -0,0 +1,29 @@
// Copyright (C) 2013 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.client.diff;
import com.google.gwt.event.dom.client.KeyPressEvent;
import com.google.gwtexpui.globalkey.client.KeyCommand;
/** A KeyCommand that does nothing, used to display a help message */
class NoOpKeyCommand extends KeyCommand {
NoOpKeyCommand(int mask, int key, String help) {
super(mask, key, help);
}
@Override
public void onKeyPress(KeyPressEvent event) {
}
}

55
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PatchSelectBox2.java
View File

@@ -1,55 +0,0 @@
//Copyright (C) 2013 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.client.diff;
import com.google.gerrit.client.Gerrit;
import com.google.gerrit.client.diff.SideBySide2.DisplaySide;
import com.google.gerrit.client.patches.PatchUtil;
import com.google.gwt.core.client.GWT;
import com.google.gwt.event.dom.client.ClickEvent;
import com.google.gwt.uibinder.client.UiBinder;
import com.google.gwt.uibinder.client.UiField;
import com.google.gwt.uibinder.client.UiHandler;
import com.google.gwt.user.client.ui.Composite;
import com.google.gwt.user.client.ui.HTMLPanel;
import com.google.gwt.user.client.ui.Image;
/**
* HTMLPanel to select among patches
* TODO: Implement this.
*/
class PatchSelectBox2 extends Composite {
interface Binder extends UiBinder<HTMLPanel, PatchSelectBox2> {}
private static Binder uiBinder = GWT.create(Binder.class);
@UiField
Image icon;
private DiffTable table;
private DisplaySide side;
PatchSelectBox2(DiffTable table, DisplaySide side) {
initWidget(uiBinder.createAndBindUi(this));
icon.setTitle(PatchUtil.C.addFileCommentToolTip());
icon.addStyleName(Gerrit.RESOURCES.css().link());
this.table = table;
this.side = side;
}
@UiHandler("icon")
void onIconClick(ClickEvent e) {
table.createOrEditFileComment(side);
}
}

1
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PatchSetSelectBox2.java
View File

@@ -17,7 +17,6 @@ package com.google.gerrit.client.diff;
import com.google.gerrit.client.Dispatcher;
import com.google.gerrit.client.Gerrit;
import com.google.gerrit.client.changes.ChangeInfo.RevisionInfo;
import com.google.gerrit.client.diff.SideBySide2.DisplaySide;
import com.google.gerrit.client.patches.PatchUtil;
import com.google.gerrit.client.ui.InlineHyperlink;
import com.google.gerrit.reviewdb.client.Change;

1
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PublishedBox.java
View File

@@ -21,7 +21,6 @@ import com.google.gerrit.client.changes.CommentApi;
import com.google.gerrit.client.changes.CommentInfo;
import com.google.gerrit.client.changes.CommentInput;
import com.google.gerrit.client.changes.Util;
import com.google.gerrit.client.diff.SideBySide2.DisplaySide;
import com.google.gerrit.client.patches.PatchUtil;
import com.google.gerrit.client.rpc.GerritCallback;
import com.google.gerrit.client.ui.CommentLinkProcessor;

118
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide2.java
View File

@@ -100,10 +100,6 @@ public class SideBySide2 extends Screen {
interface Binder extends UiBinder<FlowPanel, SideBySide2> {}
private static Binder uiBinder = GWT.create(Binder.class);
enum DisplaySide {
A, B;
}
private static final JsArrayString EMPTY =
JavaScriptObject.createArray().cast();
@@ -162,7 +158,7 @@ public class SideBySide2 extends Screen {
}
context = pref.getContext();
this.handlers = new ArrayList<HandlerRegistration>(6);
handlers = new ArrayList<HandlerRegistration>(6);
// TODO: Re-implement necessary GlobalKey bindings.
addDomHandler(GlobalKey.STOP_PROPAGATION, KeyPressEvent.getType());
keysNavigation = new KeyCommandSet(Gerrit.C.sectionNavigation());
@@ -434,7 +430,6 @@ public class SideBySide2 extends Screen {
linePaddingOnOtherSideMap = new HashMap<LineHandle, LinePaddingWidgetWrapper>();
diffChunks = new ArrayList<DiffChunkInfo>();
render(diffInfo);
Collections.sort(diffChunks, getDiffChunkComparator());
lineActiveBoxMap = new HashMap<LineHandle, CommentBox>();
linePublishedBoxesMap = new HashMap<LineHandle, List<PublishedBox>>();
linePaddingManagerMap = new HashMap<LineHandle, PaddingManager>();
@@ -537,12 +532,12 @@ public class SideBySide2 extends Screen {
}
int chunkEndA = mapper.getLineA() - 1;
int chunkEndB = mapper.getLineB() - 1;
if (bLength > 0) {
addDiffChunkAndPadding(cmA, chunkEndA, chunkEndB, bLength, aLength > 0);
}
if (aLength > 0) {
addDiffChunkAndPadding(cmB, chunkEndB, chunkEndA, aLength, bLength > 0);
}
if (bLength > 0) {
addDiffChunkAndPadding(cmA, chunkEndA, chunkEndB, bLength, aLength > 0);
}
markEdit(cmA, currentA, current.edit_a(), origLineA);
markEdit(cmB, currentB, current.edit_b(), origLineB);
if (aLength == 0) {
@@ -757,7 +752,7 @@ public class SideBySide2 extends Screen {
List<SkippedLine> temp = new ArrayList<SkippedLine>();
for (SkippedLine skip : skips) {
CommentInfo info = box.getCommentInfo();
int startLine = info.side() == Side.PARENT
int startLine = box.getSide() == DisplaySide.A
? skip.getStartA()
: skip.getStartB();
int boxLine = info.line();
@@ -792,8 +787,7 @@ public class SideBySide2 extends Screen {
}
}
private void checkAndAddSkip(List<SkippedLine> list,
SkippedLine toAdd) {
private void checkAndAddSkip(List<SkippedLine> list, SkippedLine toAdd) {
if (toAdd.getSize() > 1) {
list.add(toAdd);
}
@@ -1182,16 +1176,16 @@ public class SideBySide2 extends Screen {
res = res + (prev ? -1 : 1);
DiffChunkInfo lookUp = diffChunks.get(getWrapAroundDiffChunkIndex(res));
// If edit, skip the deletion chunk and set focus on the insertion one.
if (lookUp.edit && lookUp.side == DisplaySide.A) {
if (lookUp.isEdit() && lookUp.getSide() == DisplaySide.A) {
res = res + (prev ? -1 : 1);
}
DiffChunkInfo target = diffChunks.get(getWrapAroundDiffChunkIndex(res));
CodeMirror targetCm = getCmFromSide(target.side);
targetCm.setCursor(LineCharacter.create(target.start));
CodeMirror targetCm = getCmFromSide(target.getSide());
targetCm.setCursor(LineCharacter.create(target.getStart()));
targetCm.focus();
targetCm.scrollToY(Math.max(
0,
targetCm.heightAtLine(target.start, "local") -
targetCm.heightAtLine(target.getStart(), "local") -
0.5 * cmB.getScrollbarV().getClientHeight()));
}
};
@@ -1207,13 +1201,15 @@ public class SideBySide2 extends Screen {
return new Comparator<DiffChunkInfo>() {
@Override
public int compare(DiffChunkInfo o1, DiffChunkInfo o2) {
if (o1.side == o2.side) {
return o1.start - o2.start;
} else if (o1.side == DisplaySide.A) {
int comp = mapper.lineOnOther(o1.side, o1.start).getLine() - o2.start;
if (o1.getSide() == o2.getSide()) {
return o1.getStart() - o2.getStart();
} else if (o1.getSide() == DisplaySide.A) {
int comp = mapper.lineOnOther(o1.getSide(), o1.getStart())
.getLine() - o2.getStart();
return comp == 0 ? -1 : comp;
} else {
int comp = o1.start - mapper.lineOnOther(o2.side, o2.start).getLine();
int comp = o1.getStart() -
mapper.lineOnOther(o2.getSide(), o2.getStart()).getLine();
return comp == 0 ? 1 : comp;
}
}
@@ -1231,7 +1227,8 @@ public class SideBySide2 extends Screen {
res = -res - 1;
if (res > 0) {
DiffChunkInfo info = diffChunks.get(res - 1);
if (info.side == side && info.start <= line && line <= info.end) {
if (info.getSide() == side && info.getStart() <= line &&
line <= info.getEnd()) {
return info;
}
}
@@ -1343,81 +1340,4 @@ public class SideBySide2 extends Screen {
CodeMirror getCmB() {
return cmB;
}
static class EditIterator {
private final JsArrayString lines;
private final int startLine;
private int currLineIndex;
private int currLineOffset;
EditIterator(JsArrayString lineArray, int start) {
lines = lineArray;
startLine = start;
}
LineCharacter advance(int numOfChar) {
while (currLineIndex < lines.length()) {
int lengthWithNewline =
lines.get(currLineIndex).length() - currLineOffset + 1;
if (numOfChar < lengthWithNewline) {
LineCharacter at = LineCharacter.create(
startLine + currLineIndex,
numOfChar + currLineOffset);
currLineOffset += numOfChar;
return at;
}
numOfChar -= lengthWithNewline;
advanceLine();
if (numOfChar == 0) {
return LineCharacter.create(startLine + currLineIndex, 0);
}
}
throw new IllegalStateException("EditIterator index out of bound");
}
private void advanceLine() {
currLineIndex++;
currLineOffset = 0;
}
}
static class DiffChunkInfo {
private DisplaySide side;
private int start;
private int end;
private boolean edit;
DiffChunkInfo(DisplaySide side, int start, int end, boolean edit) {
this.side = side;
this.start = start;
this.end = end;
this.edit = edit;
}
DisplaySide getSide() {
return side;
}
int getStart() {
return start;
}
int getEnd() {
return end;
}
boolean isEdit() {
return edit;
}
}
private static class NoOpKeyCommand extends KeyCommand {
private NoOpKeyCommand(int mask, int key, String help) {
super(mask, key, help);
}
@Override
public void onKeyPress(KeyPressEvent event) {
}
}
}