google / gitiles

A simple browser for Git repositories.
https://gerrit.googlesource.com/gitiles/
Other
582 stars 174 forks source link

Gitiles does not match GitHub markdown when creating header anchors #97

Closed electrofelix closed 6 years ago

electrofelix commented 7 years ago

GitHub creates anchors for headers using lower case names, consequently if users construct a manual TOC at the top of a markdown document, writing the links for GitHub to link correctly takes the form of:

# Contents
- [My Heading][#my-heading]

## My Heading

However current gitiles will preserve the case of the headings and you need to use:

# Contents
- [My Heading][#My-Heading]

## My Heading

Consequently you cannot write internal link references that will work for both gitiles and github.

Since it appears the intent is to match the GitHub flavour of markdown, this appears to be a bug.

electrofelix commented 7 years ago

Following patch will fix this issue, I'm currently in the process of working with legal about signing the Google CLA in order to contribute this myself:

From 8f355ae9051c1d18f391cefb6a63f6ca87b49c5a Mon Sep 17 00:00:00 2001
From: Darragh Bailey <daragh.bailey@gmail.com>
Date: Mon, 15 May 2017 18:56:31 +0100
Subject: [PATCH] Match GitHub TOC rendering for links

GitHub uses lowercase for anchor links, and correspondingly and TOC
references are written to use the same. This results in TOC's that work
when viewed in GitHub, but are broken in Gitiles, and vice-versa.

Switches to use lowercase for the name and id to ensure that it matches
the GitHub flavour of markdown.

Change-Id: I7a07430b183315e0d1433c4ff227a46e5c51bff3
---
 .../src/main/java/com/google/gitiles/doc/MarkdownToHtml.java          | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gitiles-servlet/src/main/java/com/google/gitiles/doc/MarkdownToHtml.java b/gitiles-servlet/src/main/java/com/google/gitiles/doc/MarkdownToHtml.java
index 99f1c53..c3ae140 100644
--- a/gitiles-servlet/src/main/java/com/google/gitiles/doc/MarkdownToHtml.java
+++ b/gitiles-servlet/src/main/java/com/google/gitiles/doc/MarkdownToHtml.java
@@ -210,8 +210,8 @@ public class MarkdownToHtml implements Visitor {
     if (id != null) {
       html.open("a")
           .attribute("class", "h")
-          .attribute("name", id)
-          .attribute("href", "#" + id)
+          .attribute("name", id.toLowerCase())
+          .attribute("href", "#" + id.toLowerCase())
           .open("span")
           .close("span")
           .close("a");
-- 
2.7.4

However in the mean time it would be good to get some feedback on whether there is a concern about backwards compatibility? Or should it be assumed that anyone relying on the old behaviour was using buggy behaviour?

electrofelix commented 6 years ago

This was fixed by the landing of https://gerrit-review.googlesource.com/107280