commit 3d52397fffb2ee7f218c4e33f969258011e7c650
author David Pursehouse <dpursehouse@collab.net> Sat Jun 15 14:06:40 2019 +0900
committer David Pursehouse <dpursehouse@collab.net> Wed Jun 26 12:11:47 2019 +0900
tree 96e59a11c88bdda2fc62e2710a5c8dc644d8b7b2
parent 5122d3b3c347b935a287de8f946bdc94cbc4ed5c

Navbar: Fix handling of [home] and [logo] metalinks

Change I556285369 fixed the "[extensions]" meta link from being
literally included in the rendered html, but had the side effect
of breaking the [home] and [logo] metalinks.

Rework the fix, and add tests for [home] and [logo] that would
have caught the regression.

Bug: https://crbug.com/gerrit/11023
Change-Id: I44ecbe64588077b988aa3671ae94884b6b78d71b

java/com/google/gitiles/doc/Navbar.java

diff --git a/java/com/google/gitiles/doc/Navbar.java b/java/com/google/gitiles/doc/Navbar.java
index c866aa9..0fc2758 100644
--- a/java/com/google/gitiles/doc/Navbar.java
+++ b/java/com/google/gitiles/doc/Navbar.java
@@ -33,6 +33,8 @@
   private static final Pattern META_LINK =
       Pattern.compile(
           "^\\[(logo|home|extensions)\\]:\\s*(.+)$", Pattern.MULTILINE | Pattern.CASE_INSENSITIVE);
+  private static final Pattern EXTENSIONS_LINK =
+      Pattern.compile("^\\[extensions\\]:\\s*(.+)$", Pattern.MULTILINE | Pattern.CASE_INSENSITIVE);
 
   private MarkdownConfig cfg;
   private MarkdownToHtml fmt;
@@ -108,30 +110,27 @@
   }
 
   private String extractMetadata(String markdown) {
+    boolean extensionsFound = false;
     Matcher m = META_LINK.matcher(markdown);
-    boolean foundMetadata = m.find();
-    if (foundMetadata) {
-      StringBuffer sb = new StringBuffer();
-      do {
-        String key = m.group(1).toLowerCase();
-        String url = m.group(2).trim();
-        switch (key) {
-          case "logo":
-            logoUrl = url;
-            break;
-          case "home":
-            homeUrl = url;
-            break;
-          case "extensions":
-            Set<String> names = splitExtensionNames(url);
-            cfg = cfg.copyWithExtensions(enabled(names), disabled(names));
-            break;
-        }
-        m.appendReplacement(sb, "");
-        foundMetadata = m.find();
-      } while (foundMetadata);
-      m.appendTail(sb);
-      return sb.toString();
+    while (m.find()) {
+      String key = m.group(1).toLowerCase();
+      String url = m.group(2).trim();
+      switch (key) {
+        case "logo":
+          logoUrl = url;
+          break;
+        case "home":
+          homeUrl = url;
+          break;
+        case "extensions":
+          extensionsFound = true;
+          Set<String> names = splitExtensionNames(url);
+          cfg = cfg.copyWithExtensions(enabled(names), disabled(names));
+          break;
+      }
+    }
+    if (extensionsFound) {
+      return EXTENSIONS_LINK.matcher(markdown).replaceAll("");
     }
     return markdown;
   }

javatests/com/google/gitiles/doc/DocServletTest.java

diff --git a/javatests/com/google/gitiles/doc/DocServletTest.java b/javatests/com/google/gitiles/doc/DocServletTest.java
index 48300fc..2a9c2c5 100644
--- a/javatests/com/google/gitiles/doc/DocServletTest.java
+++ b/javatests/com/google/gitiles/doc/DocServletTest.java
@@ -43,7 +43,10 @@
     String navbar =
         "# Site Title\n"
             + "\n"
-            + "* [Home](index.md)\n"
+            + "[home]: index.md\n"
+            + "[logo]: logo.png\n"
+            + "\n"
+            + "* [Home][home]\n"
             + "* [README](README.md)\n"
             + "[extensions]: blocknote\n";
     repo.branch("master")
@@ -54,7 +57,7 @@
 
     String html = buildHtml("/repo/+doc/master/README.md");
     assertThat(html).contains("<title>Site Title - page</title>");
-
+    assertThat(html).contains("Header-anchorLogo");
     assertThat(html).contains("<span class=\"Header-anchorTitle\">Site Title</span>");
     assertThat(html).contains("<li><a href=\"/b/repo/+/master/index.md\">Home</a></li>");
     assertThat(html).contains("<li><a href=\"/b/repo/+/master/README.md\">README</a></li>");