From d62a6a94a4bc3ecd341178701a7c6d80989803d8 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Thu, 5 Dec 2013 12:45:32 -0800 Subject: [PATCH] SideBySide2: Defer syntax highlighting on large files If a file contains more than 500 lines on either side defer syntax highlighting by 250 milliseconds. This gives the browser a chance to get the file opened and visible on screen before the active mode starts chugging through the text building up the DOM. Users see the file appear, and then CM3 highlights the visible region and colors fill in slightly later. This gives the UI a slightly more responsive feeling while still offering users the syntax highlighting they want visible. Change-Id: I86307406b93690d864e9fa652b19157667a75a82 --- Documentation/rest-api-changes.txt | 19 +++-- .../google/gerrit/client/diff/DiffInfo.java | 1 + .../gerrit/client/diff/SideBySide2.java | 70 +++++++++++++++---- .../google/gerrit/server/change/GetDiff.java | 4 +- 4 files changed, 74 insertions(+), 20 deletions(-) diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt index d9ceed7622..7a7a1731d9 100644 --- a/Documentation/rest-api-changes.txt +++ b/Documentation/rest-api-changes.txt @@ -2311,11 +2311,13 @@ As response a link:#diff-info[DiffInfo] entity is returned that describes the di { "meta_a": { "name": "gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java", - "content_type": "text/x-java-source" + "content_type": "text/x-java-source", + "lines": 372 }, "meta_b": { "name": "gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java", - "content_type": "text/x-java-source" + "content_type": "text/x-java-source", + "lines": 578 }, "change_type": "MODIFIED", "diff_header": [ @@ -2379,11 +2381,13 @@ If the `intraline` parameter is specified, intraline differences are included in { "meta_a": { "name": "gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java", - "content_type": "text/x-java-source" + "content_type": "text/x-java-source", + "lines": 372 }, "meta_b": { "name": "gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java", - "content_type": "text/x-java-source" + "content_type": "text/x-java-source", + "lines": 578 }, "change_type": "MODIFIED", "diff_header": [ @@ -2431,11 +2435,13 @@ be generated. { "meta_a": { "name": "gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java", - "content_type": "text/x-java-source" + "content_type": "text/x-java-source", + "lines": 578 }, "meta_b": { "name": "gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java", - "content_type": "text/x-java-source" + "content_type": "text/x-java-source", + "lines": 578 }, "change_type": "MODIFIED", "content": [ @@ -3008,6 +3014,7 @@ The `DiffFileMetaInfo` entity contains meta information about a file diff. |Field Name |Description |`name` |The name of the file. |`content_type`|The content type of the file. +|`lines` |The total number of lines in the file. |========================== [[diff-info]] diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffInfo.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffInfo.java index d5f781913f..be222ef08b 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffInfo.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffInfo.java @@ -99,6 +99,7 @@ public class DiffInfo extends JavaScriptObject { public static class FileMeta extends JavaScriptObject { public final native String name() /*-{ return this.name; }-*/; public final native String content_type() /*-{ return this.content_type; }-*/; + public final native int lines() /*-{ return this.lines || 0 }-*/; protected FileMeta() { } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide2.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide2.java index e00dc20ee8..e25e95e606 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide2.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide2.java @@ -49,6 +49,7 @@ import com.google.gwt.core.client.JavaScriptObject; import com.google.gwt.core.client.JsArray; import com.google.gwt.core.client.JsArrayString; import com.google.gwt.core.client.Scheduler; +import com.google.gwt.core.client.Scheduler.RepeatingCommand; import com.google.gwt.core.client.Scheduler.ScheduledCommand; import com.google.gwt.dom.client.Element; import com.google.gwt.dom.client.NativeEvent; @@ -122,6 +123,7 @@ public class SideBySide2 extends Screen { private JsArray draftsBase; private JsArray draftsRevision; private DiffInfo diff; + private boolean largeFile; private LineMapper mapper; private List markers; private List undoLineClass; @@ -188,10 +190,16 @@ public class SideBySide2 extends Screen { @Override public void onSuccess(DiffInfo diffInfo) { diff = diffInfo; - new ModeInjector() - .add(getContentType(diff.meta_a())) - .add(getContentType(diff.meta_b())) - .inject(modeInjectorCb); + if (prefs.syntaxHighlighting()) { + largeFile = isLargeFile(diffInfo); + if (largeFile) { + modeInjectorCb.onSuccess(null); + } else { + injectMode(diffInfo, modeInjectorCb); + } + } else { + modeInjectorCb.onSuccess(null); + } } })); @@ -514,6 +522,18 @@ public class SideBySide2 extends Screen { resizeCodeMirror(); } }); + + if (largeFile && prefs.syntaxHighlighting()) { + Scheduler.get().scheduleFixedDelay(new RepeatingCommand() { + @Override + public boolean execute() { + if (prefs.syntaxHighlighting() && isAttached()) { + setSyntaxHighlighting(prefs.syntaxHighlighting()); + } + return false; + } + }, 250); + } } private CodeMirror createCodeMirror( @@ -526,7 +546,7 @@ public class SideBySide2 extends Screen { .set("cursorHeight", 0.85) .set("lineNumbers", true) .set("tabSize", prefs.tabSize()) - .set("mode", getContentType(meta)) + .set("mode", largeFile ? null : getContentType(meta)) .set("lineWrapping", false) .set("styleSelectedText", true) .set("showTrailingSpace", prefs.showWhitespaceErrors()) @@ -563,14 +583,26 @@ public class SideBySide2 extends Screen { prefsAction.update(); } - void setSyntaxHighlighting(final boolean b) { - operation(new Runnable() { - @Override - public void run() { - cmA.setOption("mode", b ? getContentType(diff.meta_a()) : null); - cmB.setOption("mode", b ? getContentType(diff.meta_b()) : null); - } - }); + void setSyntaxHighlighting(boolean b) { + if (b) { + injectMode(diff, new AsyncCallback() { + @Override + public void onSuccess(Void result) { + if (prefs.syntaxHighlighting()) { + cmA.setOption("mode", getContentType(diff.meta_a())); + cmB.setOption("mode", getContentType(diff.meta_b())); + } + } + + @Override + public void onFailure(Throwable caught) { + prefs.syntaxHighlighting(false); + } + }); + } else { + cmA.setOption("mode", (String) null); + cmB.setOption("mode", (String) null); + } } void setContext(final int context) { @@ -1537,6 +1569,13 @@ public class SideBySide2 extends Screen { : null; } + private void injectMode(DiffInfo diffInfo, AsyncCallback cb) { + new ModeInjector() + .add(getContentType(diffInfo.meta_a())) + .add(getContentType(diffInfo.meta_b())) + .inject(cb); + } + DiffPreferences getPrefs() { return prefs; } @@ -1614,4 +1653,9 @@ public class SideBySide2 extends Screen { } }); } + + private static boolean isLargeFile(DiffInfo diffInfo) { + return (diffInfo.meta_a() != null && diffInfo.meta_a().lines() > 500) + || (diffInfo.meta_b() != null && diffInfo.meta_b().lines() > 500); + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetDiff.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetDiff.java index 04136bd3e4..f0365b7b8e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetDiff.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetDiff.java @@ -132,12 +132,14 @@ public class GetDiff implements RestReadView { result.metaA = new FileMeta(); result.metaA.name = Objects.firstNonNull(ps.getOldName(), ps.getNewName()); result.metaA.setContentType(ps.getFileModeA(), ps.getMimeTypeA()); + result.metaA.lines = ps.getA().size(); } if (ps.getDisplayMethodB() != DisplayMethod.NONE) { result.metaB = new FileMeta(); result.metaB.name = ps.getNewName(); result.metaB.setContentType(ps.getFileModeB(), ps.getMimeTypeB()); + result.metaB.lines = ps.getB().size(); } if (intraline) { @@ -179,7 +181,7 @@ public class GetDiff implements RestReadView { static class FileMeta { String name; String contentType; - String url; + Integer lines; void setContentType(FileMode fileMode, String mimeType) { switch (fileMode) {