Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bad include #6322

Closed
ghost opened this issue Sep 11, 2019 · 13 comments
Closed

Bad include #6322

ghost opened this issue Sep 11, 2019 · 13 comments
Labels

Comments

@ghost
Copy link

ghost commented Sep 11, 2019

Using this file:

#+INCLUDE: "app.php" src php

I get this result:

WARN 2019/09/09 23:04:11 Bad include
org.Keyword{Key:"INCLUDE", Value:"\"app.php\" src php"}:
open posts\app.php: The system cannot find the path specified.

Using this file:

#+INCLUDE: "posts\app.php" src php

I get this result:

WARN 2019/09/09 23:05:23 Bad include
org.Keyword{Key:"INCLUDE", Value:"\"posts\\app.php\" src php"}:
open posts\posts\app.php: The system cannot find the path specified.
@kaushalmodi
Copy link
Contributor

@cup You're an Org mode user too (I have seen you in the Nim community too)! :)

Copying @niklasfasching on this issue as he's the maintainer of go-org.

@ghost
Copy link
Author

ghost commented Sep 12, 2019

@kaushalmodi, yeah. The only problem with Org mode, is syntax highlighting is
bad with Jekyll, as GitHub refuses to open source their highlighter:

https://github.com/cup/rosso/blob/master/publish/markup.md#org-mode

However I think Hugo has its own highlighter, so if I can resolve this issue
with includes I could switch away from Jekyll.

@ghost ghost mentioned this issue Sep 12, 2019
@kaushalmodi
Copy link
Contributor

I'm a bit confused.. the issue here is with just highlighting? I think that the real issue here is parsing of Org mode #+include and then parsing what gets included.. it could be an another Org file or just a code snippet as in your example.

Hugo does do highlighting of Org syntax, but that's a different thing than parsing, and that gets done by Chroma.

@niklasfasching
Copy link
Contributor

As asked in the other ticket, could you give some more information on what exactly the issue is?

@ghost
Copy link
Author

ghost commented Sep 12, 2019

@niklasfasching while it might work as expected when invoking go-org directly, it doesnt work as expected when invoking via Hugo.

So likely Hugo is breaking the path before being passed to go-org.

@niklasfasching
Copy link
Contributor

niklasfasching commented Sep 12, 2019

go-org uses the DocumentName to get a base path for resolving includes. It looks like hugo passes a relative path as DocumentName for files in nested directories (e.g. content/foo/bar.org -> foo/bar.org) but not for files immediately in the content directory (e.g. content/foo.org -> /foo.org). Not sure why this happens, will look into it next week.
Or that's just an artifact of the test i did - in any case, it looks like the culprit is a DocumentName that starts with / which makes go-org resolve the include in the wrong place. Includes using absolute paths and includes in files not immediately inside the content directory should work until then.

@jerith
Copy link

jerith commented Sep 13, 2019

The problem here is that the path in DocumentName is relative to the content dir rather than the repo root. This means that when go-org expands the relative path, it gets with /path/to/repo/foo.org instead of /path/to/repo/content/foo.org (and similarly for nested dirs within content).

I'm currently working around this by symlinking posts to content/posts, but I think the solution is for hugo to either pass an absolute path to go-org's Parse method or pass DocumentName with content/ prefixed to it so that it's relative to the repo root again.

@jerith
Copy link

jerith commented Sep 13, 2019

I seem to get the correct behaviour if I compile my own hugo with this very hacky change:

diff --git a/helpers/content.go b/helpers/content.go
index f6576c04..9dce57fc 100644
--- a/helpers/content.go
+++ b/helpers/content.go
@@ -22,6 +22,7 @@ import (
        "fmt"
        "html/template"
        "os/exec"
+       "path/filepath"
        "runtime"
        "unicode"
        "unicode/utf8"
@@ -762,7 +763,12 @@ func orgRender(ctx *RenderingContext, c ContentSpec) []byte {
                return highlightedSource
        }

-       html, err := config.Parse(bytes.NewReader(ctx.Content), ctx.DocumentName).Write(writer)
+       contentDir := ctx.Cfg.GetString("contentDir")
+       if contentDir == "" {
+               contentDir = "content"
+       }
+       docpath := filepath.Join(contentDir, ctx.DocumentName)
+       html, err := config.Parse(bytes.NewReader(ctx.Content), docpath).Write(writer)
        if err != nil {
                jww.ERROR.Printf("Could not render org: %s. Using unrendered content.", err)
                return ctx.Content

There's almost certainly a much cleaner way to do it, but I'm not sufficiently familiar with hugo's code to find it, unfortunately.

@jerith
Copy link

jerith commented Sep 13, 2019

Here is where the RenderingContext is constructed when rendering a full page:

func (cp *pageContentOutput) renderContent(p page.Page, content []byte) []byte {
return cp.p.s.ContentSpec.RenderBytes(&helpers.RenderingContext{
Content: content, RenderTOC: true, PageFmt: cp.p.m.markup,
Cfg: p.Language(),
DocumentID: p.File().UniqueID(), DocumentName: p.File().Path(),
Config: cp.p.getRenderingConfig()})
}

As seen in the above code, DocumentName is set to p.File().Path(). If we added a new field to RenderingContext (maybe DocumentFilename?) we could set it to p.File().Filename() (which returns the absolute path) and then use it in orgRender() rather than trying to glue the content path back onto DocumentName.

@niklasfasching
Copy link
Contributor

Includes of content directory files are fixed in master / the next release - @cup your example should work now..

We could look into supporting includes from other directories as well, this just fixes the most common use-case.

@ghost
Copy link
Author

ghost commented Oct 7, 2019

@niklasfasching I ended up just writing a shortcode:

{{ $path := .Get 0 -}}
{{ $input := path.Join "content" .Page.File.Dir $path | readFile -}}
{{ $lang := index (split $path ".") 1 -}}
{{ if .Get 1 -}}
   {{ $lang = .Get 1 -}}
{{ end -}}
{{ highlight $input $lang "" -}}

https://github.com/cup/autumn/blob/master/layouts/shortcodes/read.html

so I am using that with MarkDown. At any rate, Thanks for your work

@niklasfasching
Copy link
Contributor

Thx! Cool, so this can be closed?

@ghost ghost closed this as completed Dec 22, 2019
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 10, 2022
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants