Merge "Sort email comment groups"
diff --git a/gerrit-common/BUCK b/gerrit-common/BUCK
index 452b2fe..b2ae8b0 100644
--- a/gerrit-common/BUCK
+++ b/gerrit-common/BUCK
@@ -61,6 +61,7 @@
':client',
'//lib:guava',
'//lib:junit',
+ '//lib:truth',
],
)
diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/FilenameComparator.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/FilenameComparator.java
new file mode 100644
index 0000000..08a09b7
--- /dev/null
+++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/FilenameComparator.java
@@ -0,0 +1,64 @@
+// Copyright (C) 2016 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.reviewdb.client.Patch;
+
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.HashSet;
+import java.util.Set;
+
+public class FilenameComparator implements Comparator<String> {
+ public static final FilenameComparator INSTANCE = new FilenameComparator();
+
+ private static final Set<String> cppHeaderSuffixes = new HashSet<String>(
+ Arrays.asList(".h", ".hxx", ".hpp"));
+
+ private FilenameComparator() {}
+
+ @Override
+ public int compare(final String path1, final String path2) {
+ if (Patch.COMMIT_MSG.equals(path1) && Patch.COMMIT_MSG.equals(path2)) {
+ return 0;
+ } else if (Patch.COMMIT_MSG.equals(path1)) {
+ return -1;
+ } else if (Patch.COMMIT_MSG.equals(path2)) {
+ return 1;
+ }
+ if (Patch.MERGE_LIST.equals(path1) && Patch.MERGE_LIST.equals(path2)) {
+ return 0;
+ } else if (Patch.MERGE_LIST.equals(path1)) {
+ return -1;
+ } else if (Patch.MERGE_LIST.equals(path2)) {
+ return 1;
+ }
+
+ int s1 = path1.lastIndexOf('.');
+ int s2 = path2.lastIndexOf('.');
+ if (s1 > 0 && s2 > 0 &&
+ path1.substring(0, s1).equals(path2.substring(0, s2))) {
+ String suffixA = path1.substring(s1);
+ String suffixB = path2.substring(s2);
+ // C++ and C: give priority to header files (.h/.hpp/...)
+ if (cppHeaderSuffixes.contains(suffixA)) {
+ return -1;
+ } else if (cppHeaderSuffixes.contains(suffixB)) {
+ return 1;
+ }
+ }
+ return path1.compareTo(path2);
+ }
+}
diff --git a/gerrit-common/src/test/java/com/google/gerrit/common/data/FilenameComparatorTest.java b/gerrit-common/src/test/java/com/google/gerrit/common/data/FilenameComparatorTest.java
new file mode 100644
index 0000000..ef8f0a9
--- /dev/null
+++ b/gerrit-common/src/test/java/com/google/gerrit/common/data/FilenameComparatorTest.java
@@ -0,0 +1,65 @@
+// Copyright (C) 2016 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 static com.google.common.truth.Truth.assertThat;
+
+import org.junit.Test;
+
+public class FilenameComparatorTest {
+ private FilenameComparator comparator = FilenameComparator.INSTANCE;
+
+ @Test
+ public void basicPaths() {
+ assertThat(comparator.compare(
+ "abc/xyz/FileOne.java", "xyz/abc/FileTwo.java")).isLessThan(0);
+ assertThat(comparator.compare(
+ "abc/xyz/FileOne.java", "abc/xyz/FileOne.java")).isEqualTo(0);
+ assertThat(comparator.compare(
+ "zzz/yyy/FileOne.java", "abc/xyz/FileOne.java")).isGreaterThan(0);
+ }
+
+ @Test
+ public void specialPaths() {
+ assertThat(comparator.compare(
+ "ABC/xyz/FileOne.java", "/COMMIT_MSG")).isGreaterThan(0);
+ assertThat(comparator.compare(
+ "/COMMIT_MSG", "ABC/xyz/FileOne.java")).isLessThan(0);
+
+ assertThat(comparator.compare(
+ "ABC/xyz/FileOne.java", "/MERGE_LIST")).isGreaterThan(0);
+ assertThat(comparator.compare(
+ "/MERGE_LIST", "ABC/xyz/FileOne.java")).isLessThan(0);
+
+ assertThat(comparator.compare(
+ "/COMMIT_MSG", "/MERGE_LIST")).isLessThan(0);
+ assertThat(comparator.compare(
+ "/MERGE_LIST", "/COMMIT_MSG")).isGreaterThan(0);
+
+ assertThat(comparator.compare(
+ "/COMMIT_MSG", "/COMMIT_MSG")).isEqualTo(0);
+ assertThat(comparator.compare(
+ "/MERGE_LIST", "/MERGE_LIST")).isEqualTo(0);
+ }
+
+ @Test
+ public void cppExtensions() {
+ assertThat(comparator.compare("abc/file.h", "abc/file.cc")).isLessThan(0);
+ assertThat(comparator.compare("abc/file.c", "abc/file.hpp"))
+ .isGreaterThan(0);
+ assertThat(comparator.compare("abc..xyz.file.h", "abc.xyz.file.cc"))
+ .isLessThan(0);
+ }
+}
\ No newline at end of file
diff --git a/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/FileInfo.java b/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/FileInfo.java
index b21126d..e557470 100644
--- a/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/FileInfo.java
+++ b/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/FileInfo.java
@@ -15,6 +15,7 @@
package com.google.gerrit.client.info;
import com.google.gerrit.client.rpc.Natives;
+import com.google.gerrit.common.data.FilenameComparator;
import com.google.gerrit.reviewdb.client.Patch;
import com.google.gwt.core.client.JavaScriptObject;
import com.google.gwt.core.client.JsArray;
@@ -30,7 +31,6 @@
public final native boolean binary() /*-{ return this.binary || false; }-*/;
public final native String status() /*-{ return this.status; }-*/;
-
// JSNI methods cannot have 'long' as a parameter type or a return type and
// it's suggested to use double in this case:
// http://www.gwtproject.org/doc/latest/DevGuideCodingBasicsJSNI.html#important
@@ -48,37 +48,8 @@
public final native void _row(int r) /*-{ this._row = r }-*/;
public static void sortFileInfoByPath(JsArray<FileInfo> list) {
- Collections.sort(Natives.asList(list), new Comparator<FileInfo>() {
- @Override
- public int compare(FileInfo a, FileInfo b) {
- if (Patch.COMMIT_MSG.equals(a.path())) {
- return -1;
- } else if (Patch.COMMIT_MSG.equals(b.path())) {
- return 1;
- }
- if (Patch.MERGE_LIST.equals(a.path())) {
- return -1;
- } else if (Patch.MERGE_LIST.equals(b.path())) {
- return 1;
- }
-
- // Look at file suffixes to check if it makes sense to use a different order
- int s1 = a.path().lastIndexOf('.');
- int s2 = b.path().lastIndexOf('.');
- if (s1 > 0 && s2 > 0 &&
- a.path().substring(0, s1).equals(b.path().substring(0, s2))) {
- String suffixA = a.path().substring(s1);
- String suffixB = b.path().substring(s2);
- // C++ and C: give priority to header files (.h/.hpp/...)
- if (suffixA.indexOf(".h") == 0) {
- return -1;
- } else if (suffixB.indexOf(".h") == 0) {
- return 1;
- }
- }
- return a.path().compareTo(b.path());
- }
- });
+ Collections.sort(Natives.asList(list),
+ Comparator.comparing(FileInfo::path, FilenameComparator.INSTANCE));
}
public static String getFileName(String path) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/CommentSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/CommentSender.java
index 35eea03..77086f7 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/CommentSender.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/CommentSender.java
@@ -1,4 +1,4 @@
-// Copyright (C) 2009 The Android Open Source Project
+// Copyright (C) 2016 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.
@@ -16,6 +16,7 @@
import com.google.common.base.Strings;
import com.google.common.collect.Ordering;
+import com.google.gerrit.common.data.FilenameComparator;
import com.google.gerrit.common.errors.EmailException;
import com.google.gerrit.common.errors.NoSuchEntityException;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
@@ -41,6 +42,7 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
+import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
@@ -238,6 +240,8 @@
}
}
+ Collections.sort(groups,
+ Comparator.comparing(g -> g.filename, FilenameComparator.INSTANCE));
return groups;
}
