Repository: gohugoio/hugo (language: go)

Issue:
Indented `highlight` shortcode brings in the indentation as part of the code block
- When using the highlight shortcode for code blocks in lists, the indentation needed to have the code blocks indented gets added to the code block itself. 
- This does not happen when using code fences. 

So the bug seems to be localized to Hugo and not Blackfriday. 

Here is a test page demonstrating the bug:

- [HTML][1] | [Markdown][2]

In the same test page above, the [first section][3] has the same code blocks, but done using the code fences, where that extra indentation issue is not seen.

Another effect of not removing that indentation is that the indentation before the closing `{{< /highlight >}}` creates an extra blank line in the final HTML.


[1]: https://ox-hugo.scripter.co/test/posts/source-block-indented/#code-blocks-in-list-using-highlight-shortcode
[2]: https://raw.githubusercontent.com/kaushalmodi/ox-hugo/master/test/site/content/posts/source-block-indented.md
[3]: https://ox-hugo.scripter.co/test/posts/source-block-indented/#code-blocks-in-list-using-code-fences


Discussion / hints:
Updated the tests showing the bug with smaller/targeted test files.
@bep can you please tag this as a bug (and hopefully see if an easy fix is possible for this?
Can this be tagged as a Bug please? This issue prevents me from using `highlight` shortcode (and thus the line number prefixing using Chroma) in lists.
@bep can you please add this issue to a near term milestone. This particular issue prevents me from using highlight shortcode in lists (which I seem to end up using a lot more than general, I think). 

Also, I don't have any outside-hugo workaround for this. 

Thanks. 
Your shortcode variant shows the hightlighting as a table with line numbers, you can adjust the indentation with CSS, I guess, but there will be indentation ...

```
 {{< highlight emacs-lisp "linenos=table, linenostart=1" >}}
````

If you remove the "linenos=table", that should fix it.
> you can adjust the indentation with CSS

The indentation is due to extra spaces in the pre block. And also the closing tag of the highlight shortcode introduces a blank line.

The indentation also increases with each nested list item. So it gets very tricky to fix with CSS (also not sure if it would be possible because then I need to make the CSS rules apply only on the badly indented blocks created by highlight shortcode and not by the fenced blocks).

> If you remove the "linenos=table", that should fix it.

But I need to use the line numbers and so I am using the highlight shortcode. I use the fenced code blocks when I don't need line numbers.

---

Can you please reopen this issue? The root cause is that the highlight shortcode absorbs all the indentation whitespace which it should not. It should be behaving like indented fenced code blocks.
>The indentation is due to extra spaces in the pre block. And also the closing tag of the highlight shortcode introduces a blank line.


OK, but in any case you need to take that with Chroma. Note that the code fence example and shortcode example are two totally different cases.
@bep 

## When using fenced code blocks

From https://ox-hugo.scripter.co/test/posts/source-block-indented/#code-blocks-in-list-using-code-fences:

![image](https://user-images.githubusercontent.com/3578197/50305881-28d71080-0462-11e9-8bcb-913882dc9e61.png)

## When using `highlight` shortcode

From https://ox-hugo.scripter.co/test/posts/source-block-indented/#code-blocks-in-list-using-highlight-shortcode:

![image](https://user-images.githubusercontent.com/3578197/50306014-72bff680-0462-11e9-8ad6-2d02e532156a.png)

So the solution would be to figure out the indentation level of the starting `{{< highlight >}}` shortcode tag, and remove that indentation from the whole text block within it, including removal of that indentation from the closing `{{< /highlight >}}` tag.


> OK, but in any case you need to take that with Chroma.

From the above screenshots, see that Chroma is used in both cases, and Chroma knows nothing about the highlight shortcode. 

I think that Chroma simply wraps the code blocks that Hugo passes to it in pre tags. Isn't that right?

Chroma also cannot blindly remove all the leading spaces, because what if one actually wants indentation in the code blocks (Python comes to mind).
@bep 

One more take at explaining the issue:

![image](https://user-images.githubusercontent.com/3578197/50306916-cc292500-0464-11e9-8a0b-0b87cf4d1953.png)

I have reopened this to track this **upstream issue**. I'm not spending more time investigating this. Others are welcome, but I suggest you start by looking at what you get from Chroma.

Also, for the future: When doing a "it works fine for code fences, but fails for shortcodes", it would be good if you do the same thing in the two cases.
@bep 

I don't know Go, but trying some poorman's debug here using printing of debug info to stdout ..

I am still not convinced that this is a Chroma issue.. see below.

I added those two `fmt.Printf` to do a real dumb analysis of what's going to Chroma and what's coming back.

```go
func (h highlighters) chromaHighlight(code, lang, optsStr string) (string, error) {
	opts, err := h.cs.parsePygmentsOpts(optsStr)
	if err != nil {
		jww.ERROR.Print(err.Error())
		return code, err
	}

	style, found := opts["style"]
	if !found || style == "" {
		style = "friendly"
	}

	f, err := h.cs.chromaFormatterFromOptions(opts)
	if err != nil {
		jww.ERROR.Print(err.Error())
		return code, err
	}

	b := bp.GetBuffer()
	defer bp.PutBuffer(b)

	// Added debug
	fmt.Printf("[%s] input to Chroma = `%s'\n", lang, code)
	
	err = chromaHighlight(b, code, lang, style, f)
	if err != nil {
		jww.ERROR.Print(err.Error())
		return code, err
	}

	// Added debug
	fmt.Printf("output from Chroma = `%s'\n\n", b.String())

	return h.injectCodeTag(`<div class="highlight">`+b.String()+"</div>", lang), nil
}
```

### Fenced code block

For the code block in:

````
    -   List item 1.1

        ```emacs-lisp
        (message "I am in list at level-2 indentation")
        ```
````

I get:

```text
[emacs-lisp] input to Chroma = `(message "I am in list at level-2 indentation")'
output from Chroma = `<pre class="chroma"><span class="p">(</span><span class="nf">message</span> <span class="s">&#34;I am in list at level-2 indentation&#34;</span><span class="p">)</span></pre>'
```

*Notice that the `code` string input has the whitespace indentation already stripped off in Hugo land.*

### highlight shortcode with table

For the code block in:

````
    -   List item 1.1

        {{< highlight emacs-lisp "linenos=table, linenostart=1" >}}
        (message "I am in list at level-2 indentation")
        {{< /highlight >}}
````

I get:

```text
[emacs-lisp] input to Chroma = `        (message "I am in list at level-2 indentation")
        '
output from Chroma = `<div class="chroma">
<table class="lntable"><tr><td class="lntd">
<pre class="chroma"><span class="lnt">1
</span><span class="lnt">2
</span></pre></td>
<td class="lntd">
<pre class="chroma">        <span class="p">(</span><span class="nf">message</span> <span class="s">&#34;I am in list at level-2 indentation&#34;</span><span class="p">)</span>
        </pre></td></tr></table>
</div>
'
```

*Notice that the `code` string input here has the unstripped extra whitespace indentation.*

---

Now I need your help in understanding where that `code` variable is created, that gets passed to `chromaHighlight` here:

https://github.com/gohugoio/hugo/blob/7c30e2cbb08fdf0e61f80c7f1aa29909aeca4211/helpers/pygments.go#L83
@bep OK, I have another proof that this has little to do with Chroma.

This time, I removed the linenos and linenostart args.

With this in the content file:

````
    -   List item 1.1

        {{< highlight emacs-lisp >}}
        (message "I am in list at level-2 indentation")
        {{< /highlight >}}
````

I get this in debug:

```text
[emacs-lisp] input to Chroma = `        (message "I am in list at level-2 indentation")
        '
output from Chroma = `<pre class="chroma">        <span class="p">(</span><span class="nf">message</span> <span class="s">&#34;I am in list at level-2 indentation&#34;</span><span class="p">)</span>
        </pre>'
```

![image](https://user-images.githubusercontent.com/3578197/50309800-28447700-046e-11e9-9214-77777cecc670.png)

@bep What do you think of the above analysis? Can you remove the "upstream" tag?

I believe the fix is simple but I don't know how to code that in Go:

1. First analyze if that `code` variable (that's passed to `chromaHighlight` in https://github.com/gohugoio/hugo/blob/7c30e2cbb08fdf0e61f80c7f1aa29909aeca4211/helpers/pygments.go#L83) is ending in `".*\n\s+$" ` (crude regex, but trying to check here that the `code` is ending in a newline char followed by one or more spaces). 
2. Do the following if above condition matches. 
3. Split the `code` into an array split by the `\n` char. 
4. Count the number of spaces in the last element (these will be the exact indentation count, the number of spaces before the closing `{{ /highlight }}` tag). Let's say the this count is N.
5. Loop through the array. 
6. If any element of this array (including that last element) **begins** with *N* spaces, delete those spaces. 
7. Join this array back into a string joined by the same `\n` character. 
8. Pass that to Chroma. 

I claim that 

1) Chroma adds this superflous whitespace inside the `pre`
2) Chroma can fix this
3) Chroma should be the place for this fix

That Hugo _can work around this by implementing a 8 step algo_ isn't very convincing ...

Note that the Christmas train has left for this particular issue. I started to look at it assuming we (Hugo) did something stupid (adding some space), but that isn't the case. 
> I claim that Chroma adds this superflous whitespace inside the pre

Did you read through the debug messages I pasted? The whitespace exists in the `code` string that Hugo is passing to Chroma! Chroma is **not adding** any space. It's just wrapping your passed whitespace with HTML pre blocks. 

Pinging @alecthomas for some help here .. 
Will come back to this ... after Christmas.
As I see it, this is local to Hugo, not upstream.

The shortcode parser doesn't care where the shortcode is in the document and doesn't understand the context of the surrounding markdown.  It doesn't know that all of the leading whitespace in your code block is for the surrounding markdown list.  It just sees whitespace and assumes you put it there for a reason.

Making the shortcode parser "markdown-aware" would take a lot of work, I think, but I'm wondering if this scenario could be solved easier if the shortcode parser was a BFv2 renderer.  Either way, I don't think this is a trivial matter.
Thanks for your input @moorereason.

> Making the shortcode parser "markdown-aware" would take a lot of work, I think, but I'm wondering if this scenario could be solved easier if the shortcode parser was a BFv2 renderer. Either way, I don't think this is a trivial matter.

I agree. The scope of this issue is just the highlight shortcode and the introduced extra offset. Making the shortcode parser "markdown-aware" certainly is a huge undertaking.

As I was thinking more on my earlier comment ( https://github.com/gohugoio/hugo/issues/4717#issuecomment-449375012 ), on the steps needed to fix the `code` variable, I thought that the scope of this issue is very much limited to the `highlight` shortcode and that triggered the light bulb!

@bep I present the fix by updating the inbuilt `highlight` shortcode, and I can send a PR very soon for this. Here's the proposal:

### Old `highlight` shortcode

```
{{ if len .Params | eq 2 }}{{ highlight (trim .Inner "\n\r") (.Get 0) (.Get 1) }}{{ else }}{{ highlight (trim .Inner "\n\r") (.Get 0) "" }}{{ end }}
```

### New `highlight` shortcode

```
{{- $code := (trim .Inner "\n\r") -}}
{{- $lang := .Get 0 -}}
{{- $options := .Get 1 | default "" -}}
{{- $code_ending_in_line_with_spaces := (findRE "[\\r\\n][[:blank:]]+$" $code) -}}
{{/* Ref: https://github.com/gohugoio/hugo/issues/4717#issuecomment-449375012 */}}
{{- with $code_ending_in_line_with_spaces -}}
    {{- $code = (replace $code "\r" "") -}} {{/* Windows -> Unix style line endings for ease in further parsing */}}
    {{- $offset := (trim (index . 0) "\n") -}}
    {{- $lines := (split $code "\n") -}}
    {{- $num_lines := (len $lines) -}}
    {{- $scratch := newScratch -}}
    {{- $scratch.Add "lines_minus_offset" (slice) -}}
    {{- range $i, $line := $lines -}}
        {{- $line_minus_offset := (strings.TrimPrefix $offset $line) -}}
        {{- if (lt $i (sub $num_lines 1)) -}} {{/* Do not add the last blank line */}}
            {{- $scratch.Add "lines_minus_offset" (slice $line_minus_offset) -}}
        {{- end -}}
    {{- end -}}
    {{- $code = (delimit ($scratch.Get "lines_minus_offset") "\n") -}}
    {{- $scratch.Delete "lines_minus_offset" -}}
{{- end -}}
{{- highlight $code $lang $options -}}
```

Comments?
@bep Can you please revisit this? It's proven that this needs to be fixed on Hugo side, either in the highlight shortcode or probably in pygments.go. 
Is this already released on version 0.70.x ?
@bep needs to just merge this PR ( https://github.com/gohugoio/hugo/pull/5553 ). I have been using this in my local Hugo build since _\<gasp\>_ 2 yrs! 
Sorry about the "duplicate" commit reference -- I decided to remove the "HACK:" prefix from my commit message after I pushed my branch to GitHub.
How do we upvote this one... I'm guessing it's a weird use case in a blog, but in technical documentation, it's common to have indented code blocks within numbered lists, for example. We're trying to use `readFile` within those blocks, and this sure breaks that in a hurry.
@kaushalmodi

Please take this for a spin when you have a moment. It's a _potential_ replacement for the existing highlight shortcode.

Notes:
- You must use the `{{< highlight >}}` notation. It will throw a polite error if you try to use the `{{% highlight %}}` notation.
- On a site with 300 pages, and 5 calls to the highlight shortcode per page, this increased build time from 513 ms to 558 ms (average of 5 runs). It uses the shortcode `.Position` method to determine position on page, which the source code notes may be expensive.
- On any given line between the shortcode boundaries, it expects consistent indentation characters. Or to put it another way, to align a line of code to a given column, use spaces or tabs, but not both. You can use spaces to align line 1, and tabs to align line 2 (I'm not sure why you would do this), but don't mix tabs and spaces on the same line when aligning the block under a list item.
- I made no effort to deal with nesting. If this shortcode is nested with another, it ain't gonna work.

<details>
<summary>shortcodes/highlight.html</summary>

```go-html-template
{{- if (strings.FindRE `{{%\s+highlight.+%}}` .Page.RawContent) -}}
  {{- $msg := "The %q shortcode must be called using the \"{{< highlight >}}\" notation. See %s" -}}
  {{- errorf "highlight-shortcode-notation" $msg .Name .Position -}}
{{- end -}}

{{- /* Trim trailing whitespace. */ -}}
{{- $inner := strings.TrimRight " \t" .Inner -}}

{{- /* Trim leading and trailing end of line sequences. */ -}}
{{- $inner = strings.Trim $inner "\n\r" -}}

{{- /* Determine number of characters used to indent shortcode. */ -}}
{{- $indentationOffset := sub (strings.ReplaceRE `.+:(\d*).+` "$1" .Position | int) 1 -}}

{{- if $indentationOffset -}}

  {{- /* Normalize end of line sequence. */ -}}
  {{- $inner = strings.Replace $inner "\r\n" "\n" -}}

  {{- /* Split inner into individual lines. */ -}}
  {{- $lines := strings.Split $inner "\n" -}}

  {{- /* Remove indentation caused by position of shortcode on page. */ -}}
  {{- $trimmedLines := slice -}}
  {{- range $lines -}}
    {{- /* Determine indentation character by looking at first character. */ -}}
    {{- $indentationCharacter := substr . 0 1 -}}
    {{- $trimmedLines = $trimmedLines | append (strings.TrimPrefix (strings.Repeat $indentationOffset $indentationCharacter) . ) -}}
  {{- end -}}

  {{- /* Put the lines back together. */ -}}
  {{- $inner = delimit $trimmedLines "\n" -}}

{{- end -}}

{{- if len .Params | eq 2 -}}
  {{- highlight $inner (.Get 0) (.Get 1) -}}
{{- else -}}
  {{- highlight $inner (.Get 0) -}}
{{- end -}}
```
</details>


Here's a test site if you want to see how it works:

```bash
git clone --single-branch -b hugo-github-issue-4717 https://github.com/jmooring/hugo-testing hugo-github-issue-4717
cd hugo-github-issue-4717
hugo server
```

Rendered with existing shortcode:

<img src="https://user-images.githubusercontent.com/335669/149846821-a2569919-d92e-44b2-bd4a-6cfa762a6f08.png" width="66%" alt="">

Rendered with revised shortcode:

<img src="https://user-images.githubusercontent.com/335669/149846879-aa054d47-94ab-4520-bc40-8f543a15e93e.png" width="66%" alt="">

@jmooring I had been this this shortcode:

<details>
<summary>shortcodes/highlight.html</summary>

```html
{{- $code := (trim .Inner "\n\r") -}}
{{- $lang := .Get 0 -}}
{{- $options := .Get 1 | default "" -}}
{{- $code_ending_in_line_with_spaces := (findRE "[\\r\\n][[:blank:]]+$" $code) -}}
{{/* Ref: https://github.com/gohugoio/hugo/issues/4717#issuecomment-449375012 */}}
{{- with $code_ending_in_line_with_spaces -}}
    {{- $code = (replace $code "\r" "") -}} {{/* Windows -> Unix style line endings for ease in further parsing */}}
    {{- $offset := (trim (index . 0) "\n") -}}
    {{- $lines := (split $code "\n") -}}
    {{- $num_lines := (len $lines) -}}
    {{- $scratch := newScratch -}}
    {{- $scratch.Add "lines_minus_offset" (slice) -}}
    {{- range $i, $line := $lines -}}
        {{- $line_minus_offset := (strings.TrimPrefix $offset $line) -}}
        {{- if (lt $i (sub $num_lines 1)) -}} {{/* Do not add the last blank line */}}
            {{- $scratch.Add "lines_minus_offset" (slice $line_minus_offset) -}}
        {{- end -}}
    {{- end -}}
    {{- $code = (delimit ($scratch.Get "lines_minus_offset") "\n") -}}
    {{- $scratch.Delete "lines_minus_offset" -}}
{{- end -}}
{{- highlight $code $lang $options -}}
```

</details>

That shortcode has been working great for me (it was also submitted in a PR with a test in https://github.com/gohugoio/hugo/pull/5553).

I tested out your shortcode and it's working well as well 👍 .
I was asked if my recent "indentation fix" fixes this issue, but no, this is a different setup. Hugo currently includes every bytes (include whitespace) between open/close shortcode as the `.Inner` (including indentation), which is then passed on to the highlight func. To me this is about the `highlight` shortcode (which we can certainly adapt) and not a general thing.


Fix patch:
diff --git a/docs/content/en/variables/shortcodes.md b/docs/content/en/variables/shortcodes.md
index 14eb2aca234..37ae30629e4 100644
--- a/docs/content/en/variables/shortcodes.md
+++ b/docs/content/en/variables/shortcodes.md
@@ -45,4 +45,8 @@ toc: false
 [markdownshortcode]: /content-management/shortcodes/#shortcodes-with-markdown
 [shortcodes]: /templates/shortcode-templates/
 
+.InnerDeindent {{< new-in "0.100.0" >}}
+: Gets the `.Inner` with any indentation removed. This is what's used in the built-in `{{</* highlight */>}}` shortcode.
+
+
 
diff --git a/hugolib/shortcode.go b/hugolib/shortcode.go
index 366875b88a5..33767fc6811 100644
--- a/hugolib/shortcode.go
+++ b/hugolib/shortcode.go
@@ -62,6 +62,12 @@ type ShortcodeWithPage struct {
 	// this ordinal will represent the position of this shortcode in the page content.
 	Ordinal int
 
+	// Indentation before the opening shortcode in the source.
+	indentation string
+
+	innerDeindentInit sync.Once
+	innerDeindent     template.HTML
+
 	// pos is the position in bytes in the source file. Used for error logging.
 	posInit   sync.Once
 	posOffset int
@@ -70,6 +76,27 @@ type ShortcodeWithPage struct {
 	scratch *maps.Scratch
 }
 
+// InnerDeindent returns the (potentially de-indented) inner content of the shortcode.
+func (scp *ShortcodeWithPage) InnerDeindent() template.HTML {
+	if scp.indentation == "" {
+		return scp.Inner
+	}
+	scp.innerDeindentInit.Do(func() {
+		b := bp.GetBuffer()
+		text.VisitLinesAfter(string(scp.Inner), func(s string) {
+			if strings.HasPrefix(s, scp.indentation) {
+				b.WriteString(strings.TrimPrefix(s, scp.indentation))
+			} else {
+				b.WriteString(s)
+			}
+		})
+		scp.innerDeindent = template.HTML(b.String())
+		bp.PutBuffer(b)
+	})
+
+	return scp.innerDeindent
+}
+
 // Position returns this shortcode's detailed position. Note that this information
 // may be expensive to calculate, so only use this in error situations.
 func (scp *ShortcodeWithPage) Position() text.Position {
@@ -326,7 +353,7 @@ func renderShortcode(
 		hasVariants = hasVariants || more
 	}
 
-	data := &ShortcodeWithPage{Ordinal: sc.ordinal, posOffset: sc.pos, Params: sc.params, Page: newPageForShortcode(p), Parent: parent, Name: sc.name}
+	data := &ShortcodeWithPage{Ordinal: sc.ordinal, posOffset: sc.pos, indentation: sc.indentation, Params: sc.params, Page: newPageForShortcode(p), Parent: parent, Name: sc.name}
 	if sc.params != nil {
 		data.IsNamedParams = reflect.TypeOf(sc.params).Kind() == reflect.Map
 	}
diff --git a/tpl/tplimpl/embedded/templates/shortcodes/highlight.html b/tpl/tplimpl/embedded/templates/shortcodes/highlight.html
index b063f92ad04..54e92bb090f 100644
--- a/tpl/tplimpl/embedded/templates/shortcodes/highlight.html
+++ b/tpl/tplimpl/embedded/templates/shortcodes/highlight.html
@@ -1,1 +1,1 @@
-{{ if len .Params | eq 2 }}{{ highlight (trim .Inner "\n\r") (.Get 0) (.Get 1) }}{{ else }}{{ highlight (trim .Inner "\n\r") (.Get 0) "" }}{{ end }}
\ No newline at end of file
+{{ if len .Params | eq 2 }}{{ highlight (trim .InnerDeindent "\n\r") (.Get 0) (.Get 1) }}{{ else }}{{ highlight (trim .InnerDeindent "\n\r") (.Get 0) "" }}{{ end }}
\ No newline at end of file
diff --git a/tpl/tplimpl/template_ast_transformers.go b/tpl/tplimpl/template_ast_transformers.go
index dee1fd6c2ec..bc02d93ac35 100644
--- a/tpl/tplimpl/template_ast_transformers.go
+++ b/tpl/tplimpl/template_ast_transformers.go
@@ -272,7 +272,7 @@ func (c *templateContext) collectInner(n *parse.CommandNode) {
 			idents = nt.Ident
 		}
 
-		if c.hasIdent(idents, "Inner") {
+		if c.hasIdent(idents, "Inner") || c.hasIdent(idents, "InnerDeindent") {
 			c.t.parseInfo.IsInner = true
 			break
 		}


Test patch:
diff --git a/hugolib/shortcode_test.go b/hugolib/shortcode_test.go
index 15c27a42e01..5b8a5c2959a 100644
--- a/hugolib/shortcode_test.go
+++ b/hugolib/shortcode_test.go
@@ -1009,3 +1009,47 @@ echo "foo";
 	b.AssertFileContent("public/p1/index.html", "<pre><code>echo &quot;foo&quot;;\n</code></pre>")
 
 }
+
+func TestShortcodeHighlightDeindent(t *testing.T) {
+	t.Parallel()
+
+	files := `
+-- config.toml --
+[markup]
+[markup.highlight]
+codeFences = true
+noClasses = false
+-- content/p1.md --
+---
+title: "p1"
+---
+
+## Indent 5 Spaces
+
+     {{< highlight bash >}}
+     line 1;
+     line 2;
+     line 3;
+     {{< /highlight >}}
+
+-- layouts/_default/single.html --
+{{ .Content }}
+`
+
+	b := NewIntegrationTestBuilder(
+		IntegrationTestConfig{
+			T:           t,
+			TxtarString: files,
+			Running:     true,
+		},
+	).Build()
+
+	b.AssertFileContent("public/p1/index.html", `
+<pre><code> <div class="highlight"><pre tabindex="0" class="chroma"><code class="language-bash" data-lang="bash"><span class="line"><span class="cl">line 1<span class="p">;</span>
+</span></span><span class="line"><span class="cl">line 2<span class="p">;</span>
+</span></span><span class="line"><span class="cl">line 3<span class="p">;</span></span></span></code></pre></div>
+</code></pre>
+
+	`)
+
+}