Denote a symbolic link in side-by-side viewer

When code reviewing a symbolic link it is hard to tell from a regular
file that just happens to have the same content.  Make the symbolic
link more obvious by showing "File Type: Symbolic Link" at the top of
a symbolic link content.

Bug: issue 622
Change-Id: I4cfbae0523e05320773938e8dc871eb8bf592977
Signed-off-by: Shawn O. Pearce <sop@google.com>
This commit is contained in:
Shawn O. Pearce 2010-08-28 15:58:38 -07:00
parent 9897558362
commit e6e878205f
7 changed files with 70 additions and 5 deletions

View File

@ -34,10 +34,16 @@ public class PatchScript {
NONE, DIFF, IMG
}
public static enum FileMode {
FILE, SYMLINK, GITLINK
}
protected Change.Key changeId;
protected ChangeType changeType;
protected String oldName;
protected String newName;
protected FileMode oldMode;
protected FileMode newMode;
protected List<String> header;
protected AccountDiffPreference diffPrefs;
protected SparseFileContent a;
@ -51,7 +57,8 @@ public class PatchScript {
protected boolean intralineDifference;
public PatchScript(final Change.Key ck, final ChangeType ct, final String on,
final String nn, final List<String> h, final AccountDiffPreference dp,
final String nn, final FileMode om, final FileMode nm,
final List<String> h, final AccountDiffPreference dp,
final SparseFileContent ca, final SparseFileContent cb,
final List<Edit> e, final DisplayMethod ma, final DisplayMethod mb,
final CommentDetail cd, final List<Patch> hist, final boolean hf,
@ -60,6 +67,8 @@ public class PatchScript {
changeType = ct;
oldName = on;
newName = nn;
oldMode = om;
newMode = nm;
header = h;
diffPrefs = dp;
a = ca;
@ -88,6 +97,14 @@ public class PatchScript {
return displayMethodB;
}
public FileMode getFileModeA() {
return oldMode;
}
public FileMode getFileModeB() {
return newMode;
}
public List<String> getPatchHeader() {
return header;
}

View File

@ -105,6 +105,7 @@ public interface GerritCss extends CssResource {
String fileLineCONTEXT();
String fileLineDELETE();
String fileLineINSERT();
String fileLineMode();
String fileLineNone();
String filePathCell();
String gerritTopMenu();

View File

@ -674,6 +674,9 @@
background: #eeeeee;
border-bottom: 1px solid #eeeeee;
}
.fileLineMode {
font-weight: bold;
}
.fileLineDELETE,
.fileLineDELETE .wdc {
background: #ffeeee;

View File

@ -62,4 +62,7 @@ public interface PatchConstants extends Constants {
String buttonReplyDone();
String cannedReplyDone();
String fileTypeSymlink();
String fileTypeGitlink();
}

View File

@ -41,3 +41,6 @@ nextFileHelp = Next file
reviewed = Reviewed
download = (Download)
fileTypeSymlink = Type: Symbolic Link
fileTypeGitlink = Type: Git Commit in Subproject

View File

@ -22,6 +22,7 @@ import static com.google.gerrit.client.patches.PatchLine.Type.REPLACE;
import com.google.gerrit.client.Gerrit;
import com.google.gerrit.common.data.CommentDetail;
import com.google.gerrit.common.data.PatchScript;
import com.google.gerrit.common.data.PatchScript.FileMode;
import com.google.gerrit.prettify.common.EditList;
import com.google.gerrit.prettify.common.SparseHtmlFile;
import com.google.gerrit.reviewdb.Patch;
@ -81,6 +82,14 @@ public class SideBySideTable extends AbstractPatchContentTable {
appendHeader(script, nc);
lines.add(null);
if(script.getFileModeA()!=FileMode.FILE||script.getFileModeB()!=FileMode.FILE){
openLine(nc);
appendModeLine(nc, script.getFileModeA());
appendModeLine(nc, script.getFileModeB());
closeLine(nc);
lines.add(null);
}
int lastB = 0;
final boolean ignoreWS = script.isIgnoreWhitespace();
for (final EditList.Hunk hunk : script.getHunks()) {
@ -153,6 +162,29 @@ public class SideBySideTable extends AbstractPatchContentTable {
}
}
private void appendModeLine(final SafeHtmlBuilder nc, final FileMode mode) {
nc.openTd();
nc.setStyleName(Gerrit.RESOURCES.css().lineNumber());
nc.nbsp();
nc.closeTd();
nc.openTd();
nc.addStyleName(Gerrit.RESOURCES.css().fileLine());
nc.addStyleName(Gerrit.RESOURCES.css().fileLineMode());
switch(mode){
case FILE:
nc.nbsp();
break;
case SYMLINK:
nc.append(PatchUtil.C.fileTypeSymlink());
break;
case GITLINK:
nc.append(PatchUtil.C.fileTypeGitlink());
break;
}
nc.closeTd();
}
@Override
public void display(final CommentDetail cd) {
if (cd.isEmpty()) {

View File

@ -24,7 +24,6 @@ import com.google.gerrit.reviewdb.Change;
import com.google.gerrit.reviewdb.Patch;
import com.google.gerrit.reviewdb.PatchLineComment;
import com.google.gerrit.reviewdb.AccountDiffPreference.Whitespace;
import com.google.gerrit.reviewdb.Patch.PatchType;
import com.google.gerrit.server.FileTypeRegistry;
import com.google.gerrit.server.patch.PatchListEntry;
import com.google.gerrit.server.patch.Text;
@ -169,9 +168,9 @@ class PatchScriptBuilder {
}
return new PatchScript(change.getKey(), content.getChangeType(), content
.getOldName(), content.getNewName(), content.getHeaderLines(),
diffPrefs, a.dst, b.dst, edits, a.displayMethod, b.displayMethod,
comments, history, hugeFile, intralineDifference);
.getOldName(), content.getNewName(), a.fileMode, b.fileMode, content
.getHeaderLines(), diffPrefs, a.dst, b.dst, edits, a.displayMethod,
b.displayMethod, comments, history, hugeFile, intralineDifference);
}
private static String oldName(final PatchListEntry entry) {
@ -357,6 +356,7 @@ class PatchScriptBuilder {
Text src;
MimeType mimeType = MimeUtil2.UNKNOWN_MIME_TYPE;
DisplayMethod displayMethod = DisplayMethod.DIFF;
PatchScript.FileMode fileMode = PatchScript.FileMode.FILE;
final SparseFileContent dst = new SparseFileContent();
int size() {
@ -438,6 +438,12 @@ class PatchScriptBuilder {
}
dst.setSize(size());
dst.setPath(path);
if (mode == FileMode.SYMLINK) {
fileMode = PatchScript.FileMode.SYMLINK;
} else if (mode == FileMode.GITLINK) {
fileMode = PatchScript.FileMode.GITLINK;
}
} catch (IOException err) {
throw new IOException("Cannot read " + within.name() + ":" + path, err);
}