From 1b42dc572a0475596ffc64f8a9e0dbf8ec823094 Mon Sep 17 00:00:00 2001 From: bep Date: Fri, 12 Dec 2014 20:28:28 +0100 Subject: [PATCH] Fix RelPermalink() and Urls in menus vs canonifyUrls canonifyUrls=true, RelPermalink and baseUrl with sub-path did not work. This fixes that by adding a check for canonifyUrl=trues=true in RelPermalink(). So given - baseUrl "http://somehost.com/sub/" - the path "some-path/file.html" For canonifyUrls=false RelPermalink() returns "/sub/some-path/file.html" For canonifyUrls=true RelPermalink() returns "/some-path/file.html" In the last case, the Url will be made absolute and clickable in a later step. This commit also makes the menu urls defined in site config releative. To make them work with canonifying of urls, the context root is prepended if canonifying is turned off. Fixes #519 Fixes #711 --- docs/content/extras/menus.md | 7 +++++ helpers/url.go | 19 ++++++++++++++ helpers/url_test.go | 23 +++++++++++++++++ hugolib/menu_test.go | 18 ++++++++++--- hugolib/node.go | 5 ++-- hugolib/page.go | 12 ++++++++- hugolib/page_permalink_test.go | 47 +++++++++++++++++++--------------- hugolib/site.go | 12 +++++++-- 8 files changed, 112 insertions(+), 31 deletions(-) diff --git a/docs/content/extras/menus.md b/docs/content/extras/menus.md index 957432567..fcf90096f 100644 --- a/docs/content/extras/menus.md +++ b/docs/content/extras/menus.md @@ -96,10 +96,12 @@ Here’s an example `config.toml`: pre = "" weight = -110 identifier = "about" + url = "/about/" [[menu.main]] name = "getting started" pre = "" weight = -100 + url = "/getting-started/" And the equivalent example `config.yaml`: @@ -110,11 +112,16 @@ And the equivalent example `config.yaml`: Pre: "" Weight: -110 Identifier: "about" + Url: "/about/" - Name: "getting started" Pre: "" Weight: -100 + Url: "/getting-started/" --- + +**NOTE:** The urls must be relative to the context root. If the `BaseUrl` is `http://example.com/mysite/`, then the urls in the menu must not include the context root `mysite`. + ## Nesting All nesting of content is done via the `parent` field. diff --git a/helpers/url.go b/helpers/url.go index 46bc951f4..1b7608178 100644 --- a/helpers/url.go +++ b/helpers/url.go @@ -78,6 +78,25 @@ func MakePermalink(host, plink string) *url.URL { return base } +// AddContextRoot adds the context root to an URL if it's not already set. +// For relative URL entries on sites with a base url with a context root set (i.e. http://example.com/mysite), +// relative URLs must not include the context root if canonifyUrls is enabled. But if it's disabled, it must be set. +func AddContextRoot(baseUrl, relativePath string) string { + + url, err := url.Parse(baseUrl) + if err != nil { + panic(err) + } + + newPath := path.Join(url.Path, relativePath) + + // path strips traling slash + if strings.HasSuffix(relativePath, "/") { + newPath += "/" + } + return newPath +} + func UrlPrep(ugly bool, in string) string { if ugly { x := Uglify(SanitizeUrl(in)) diff --git a/helpers/url_test.go b/helpers/url_test.go index 1d5c770ea..3df1a05c2 100644 --- a/helpers/url_test.go +++ b/helpers/url_test.go @@ -68,6 +68,29 @@ func TestUrlPrep(t *testing.T) { } +func TestAddContextRoot(t *testing.T) { + tests := []struct { + baseUrl string + url string + expected string + }{ + {"http://example.com/sub/", "/foo", "/sub/foo"}, + {"http://example.com/sub/", "/foo/index.html", "/sub/foo/index.html"}, + {"http://example.com/sub1/sub2", "/foo", "/sub1/sub2/foo"}, + {"http://example.com", "/foo", "/foo"}, + // cannot guess that the context root is already added int the example below + {"http://example.com/sub/", "/sub/foo", "/sub/sub/foo"}, + {"http://example.com/тря", "/трям/", "/тря/трям/"}, + } + + for _, test := range tests { + output := AddContextRoot(test.baseUrl, test.url) + if output != test.expected { + t.Errorf("Expected %#v, got %#v\n", test.expected, output) + } + } +} + func TestPretty(t *testing.T) { assert.Equal(t, PrettifyUrlPath("/section/name.html"), "/section/name/index.html") assert.Equal(t, PrettifyUrlPath("/section/sub/name.html"), "/section/sub/name/index.html") diff --git a/hugolib/menu_test.go b/hugolib/menu_test.go index afe881a94..667105f50 100644 --- a/hugolib/menu_test.go +++ b/hugolib/menu_test.go @@ -265,18 +265,27 @@ func TestPageMenu(t *testing.T) { // issue #719 func TestMenuWithUnicodeUrls(t *testing.T) { for _, uglyUrls := range []bool{true, false} { - doTestMenuWithUnicodeUrls(t, uglyUrls) + for _, canonifyUrls := range []bool{true, false} { + doTestMenuWithUnicodeUrls(t, canonifyUrls, uglyUrls) + } } } -func doTestMenuWithUnicodeUrls(t *testing.T, uglyUrls bool) { +func doTestMenuWithUnicodeUrls(t *testing.T, canonifyUrls, uglyUrls bool) { + viper.Set("CanonifyUrls", canonifyUrls) viper.Set("UglyUrls", uglyUrls) + ts := setupMenuTests(t, MENU_PAGE_SOURCES) defer resetMenuTestState(ts) unicodeRussian := ts.findTestMenuEntryById("unicode", "unicode-russian") - expectedBase := "http://foo.local/zoo/%D0%BD%D0%BE%D0%B2%D0%BE%D1%81%D1%82%D0%B8-%D0%BF%D1%80%D0%BE%D0%B5%D0%BA%D1%82%D0%B0" + expectedBase := "/%D0%BD%D0%BE%D0%B2%D0%BE%D1%81%D1%82%D0%B8-%D0%BF%D1%80%D0%BE%D0%B5%D0%BA%D1%82%D0%B0" + + if !canonifyUrls { + expectedBase = "/zoo" + expectedBase + } + var expected string if uglyUrls { expected = expectedBase + ".html" @@ -288,6 +297,7 @@ func doTestMenuWithUnicodeUrls(t *testing.T, uglyUrls bool) { } func TestTaxonomyNodeMenu(t *testing.T) { + viper.Set("CanonifyUrls", true) ts := setupMenuTests(t, MENU_PAGE_SOURCES) defer resetMenuTestState(ts) @@ -333,7 +343,7 @@ func TestHomeNodeMenu(t *testing.T) { defer resetMenuTestState(ts) home := ts.site.newHomeNode() - homeMenuEntry := &MenuEntry{Name: home.Title, Url: string(home.Permalink)} + homeMenuEntry := &MenuEntry{Name: home.Title, Url: home.Url} for i, this := range []struct { menu string diff --git a/hugolib/node.go b/hugolib/node.go index 0c3ed9ce6..ccf3e8822 100644 --- a/hugolib/node.go +++ b/hugolib/node.go @@ -38,7 +38,7 @@ func (n *Node) Now() time.Time { func (n *Node) HasMenuCurrent(menuId string, inme *MenuEntry) bool { if inme.HasChildren() { - me := MenuEntry{Name: n.Title, Url: string(n.Permalink)} + me := MenuEntry{Name: n.Title, Url: n.Url} for _, child := range inme.Children { if me.IsSameResource(child) { @@ -52,8 +52,7 @@ func (n *Node) HasMenuCurrent(menuId string, inme *MenuEntry) bool { func (n *Node) IsMenuCurrent(menuId string, inme *MenuEntry) bool { - me := MenuEntry{Name: n.Title, Url: string(n.Permalink)} - + me := MenuEntry{Name: n.Title, Url: n.Url} if !me.IsSameResource(inme) { return false } diff --git a/hugolib/page.go b/hugolib/page.go index c30728c03..395462d84 100644 --- a/hugolib/page.go +++ b/hugolib/page.go @@ -397,6 +397,16 @@ func (p *Page) RelPermalink() (string, error) { return "", err } + if viper.GetBool("CanonifyUrls") { + // replacements for relpermalink with baseUrl on the form http://myhost.com/sub/ will fail later on + // have to return the Url relative from baseUrl + relpath, err := helpers.GetRelativePath(link.String(), string(p.Site.BaseUrl)) + if err != nil { + return "", err + } + return "/" + filepath.ToSlash(relpath), nil + } + link.Scheme = "" link.Host = "" link.User = nil @@ -549,7 +559,7 @@ func (page *Page) Menus() PageMenus { ret := PageMenus{} if ms, ok := page.Params["menu"]; ok { - link, _ := page.Permalink() + link, _ := page.RelPermalink() me := MenuEntry{Name: page.LinkTitle(), Weight: page.Weight, Url: link} diff --git a/hugolib/page_permalink_test.go b/hugolib/page_permalink_test.go index 97b3d24c6..b73e08721 100644 --- a/hugolib/page_permalink_test.go +++ b/hugolib/page_permalink_test.go @@ -11,34 +11,39 @@ import ( func TestPermalink(t *testing.T) { tests := []struct { - file string - dir string - base template.URL - slug string - url string - uglyurls bool - expectedAbs string - expectedRel string + file string + dir string + base template.URL + slug string + url string + uglyUrls bool + canonifyUrls bool + expectedAbs string + expectedRel string }{ - {"x/y/z/boofar.md", "x/y/z", "", "", "", false, "/x/y/z/boofar/", "/x/y/z/boofar/"}, - {"x/y/z/boofar.md", "x/y/z/", "", "", "", false, "/x/y/z/boofar/", "/x/y/z/boofar/"}, - {"x/y/z/boofar.md", "x/y/z/", "", "boofar", "", false, "/x/y/z/boofar/", "/x/y/z/boofar/"}, - {"x/y/z/boofar.md", "x/y/z", "http://barnew/", "", "", false, "http://barnew/x/y/z/boofar/", "/x/y/z/boofar/"}, - {"x/y/z/boofar.md", "x/y/z/", "http://barnew/", "boofar", "", false, "http://barnew/x/y/z/boofar/", "/x/y/z/boofar/"}, - {"x/y/z/boofar.md", "x/y/z", "", "", "", true, "/x/y/z/boofar.html", "/x/y/z/boofar.html"}, - {"x/y/z/boofar.md", "x/y/z/", "", "", "", true, "/x/y/z/boofar.html", "/x/y/z/boofar.html"}, - {"x/y/z/boofar.md", "x/y/z/", "", "boofar", "", true, "/x/y/z/boofar.html", "/x/y/z/boofar.html"}, - {"x/y/z/boofar.md", "x/y/z", "http://barnew/", "", "", true, "http://barnew/x/y/z/boofar.html", "/x/y/z/boofar.html"}, - {"x/y/z/boofar.md", "x/y/z/", "http://barnew/", "boofar", "", true, "http://barnew/x/y/z/boofar.html", "/x/y/z/boofar.html"}, + {"x/y/z/boofar.md", "x/y/z", "", "", "", false, false, "/x/y/z/boofar/", "/x/y/z/boofar/"}, + {"x/y/z/boofar.md", "x/y/z/", "", "", "", false, false, "/x/y/z/boofar/", "/x/y/z/boofar/"}, + {"x/y/z/boofar.md", "x/y/z/", "", "boofar", "", false, false, "/x/y/z/boofar/", "/x/y/z/boofar/"}, + {"x/y/z/boofar.md", "x/y/z", "http://barnew/", "", "", false, false, "http://barnew/x/y/z/boofar/", "/x/y/z/boofar/"}, + {"x/y/z/boofar.md", "x/y/z/", "http://barnew/", "boofar", "", false, false, "http://barnew/x/y/z/boofar/", "/x/y/z/boofar/"}, + {"x/y/z/boofar.md", "x/y/z", "", "", "", true, false, "/x/y/z/boofar.html", "/x/y/z/boofar.html"}, + {"x/y/z/boofar.md", "x/y/z/", "", "", "", true, false, "/x/y/z/boofar.html", "/x/y/z/boofar.html"}, + {"x/y/z/boofar.md", "x/y/z/", "", "boofar", "", true, false, "/x/y/z/boofar.html", "/x/y/z/boofar.html"}, + {"x/y/z/boofar.md", "x/y/z", "http://barnew/", "", "", true, false, "http://barnew/x/y/z/boofar.html", "/x/y/z/boofar.html"}, + {"x/y/z/boofar.md", "x/y/z/", "http://barnew/", "boofar", "", true, false, "http://barnew/x/y/z/boofar.html", "/x/y/z/boofar.html"}, + {"x/y/z/boofar.md", "x/y/z/", "http://barnew/boo/", "boofar", "", true, false, "http://barnew/boo/x/y/z/boofar.html", "/boo/x/y/z/boofar.html"}, + {"x/y/z/boofar.md", "x/y/z/", "http://barnew/boo/", "boofar", "", true, true, "http://barnew/boo/x/y/z/boofar.html", "/x/y/z/boofar.html"}, + {"x/y/z/boofar.md", "x/y/z/", "http://barnew/boo", "boofar", "", true, true, "http://barnew/boo/x/y/z/boofar.html", "/x/y/z/boofar.html"}, // test url overrides - {"x/y/z/boofar.md", "x/y/z", "", "", "/z/y/q/", false, "/z/y/q/", "/z/y/q/"}, + {"x/y/z/boofar.md", "x/y/z", "", "", "/z/y/q/", false, false, "/z/y/q/", "/z/y/q/"}, } viper.Set("DefaultExtension", "html") for i, test := range tests { - viper.Set("uglyurls", test.uglyurls) + viper.Set("uglyurls", test.uglyUrls) + viper.Set("canonifyurls", test.canonifyUrls) p := &Page{ Node: Node{ UrlPath: UrlPath{ @@ -75,7 +80,7 @@ func TestPermalink(t *testing.T) { expected = test.expectedRel if u != expected { - t.Errorf("Test %d: Expected abs url: %s, got: %s", i, expected, u) + t.Errorf("Test %d: Expected rel url: %s, got: %s", i, expected, u) } } } diff --git a/hugolib/site.go b/hugolib/site.go index 413e9f4e2..95a978f93 100644 --- a/hugolib/site.go +++ b/hugolib/site.go @@ -106,6 +106,7 @@ type SiteInfo struct { Permalinks PermalinkOverrides Params map[string]interface{} BuildDrafts bool + canonifyUrls bool } // SiteSocial is a place to put social details on a site level. These are the @@ -362,6 +363,7 @@ func (s *Site) initializeSiteInfo() { Copyright: viper.GetString("copyright"), DisqusShortname: viper.GetString("DisqusShortname"), BuildDrafts: viper.GetBool("BuildDrafts"), + canonifyUrls: viper.GetBool("CanonifyUrls"), Pages: &s.Pages, Recent: &s.Pages, Menus: &s.Menus, @@ -608,10 +610,16 @@ func (s *Site) getMenusFromConfig() Menus { } menuEntry.MarshallMap(ime) + if strings.HasPrefix(menuEntry.Url, "/") { - // make it absolute so it matches the nodes - menuEntry.Url = s.permalinkStr(menuEntry.Url) + // make it match the nodes + menuEntryUrl := menuEntry.Url + if !s.Info.canonifyUrls { + menuEntryUrl = helpers.AddContextRoot(string(s.Info.BaseUrl), menuEntryUrl) + } + menuEntry.Url = s.prepUrl(menuEntryUrl) } + if ret[name] == nil { ret[name] = &Menu{} }