Merge pull request #13 from ThiefMaster/use-markdown-parser

Use markdown parser to handle code blocks properly
mattermost-community · Jul 16, 2018 · acc78b7 · acc78b7
2 parents 3ca0173 + fb3d73f
commit acc78b7
Show file tree

Hide file tree

Showing 2 changed files with 92 additions and 36 deletions.
diff --git a/server/plugin.go b/server/plugin.go
@@ -1,13 +1,14 @@
 package main
 
 import (
-	"strings"
+	"fmt"
 	"sync/atomic"
 
 	"github.com/mattermost/mattermost-server/mlog"
 
 	"github.com/mattermost/mattermost-server/model"
 	"github.com/mattermost/mattermost-server/plugin"
+	"github.com/mattermost/mattermost-server/utils/markdown"
 )
 
 // Plugin the main struct for everything
@@ -44,44 +45,42 @@ func (p *Plugin) OnConfigurationChange() error {
 // to the database.
 func (p *Plugin) MessageWillBePosted(c *plugin.Context, post *model.Post) (*model.Post, string) {
 	links := p.links.Load().([]*AutoLinker)
+	postText := post.Message
+	offset := 0
+	markdown.Inspect(post.Message, func(node interface{}) bool {
+		switch node.(type) {
+		// never descend into the text content of a link/image
+		case *markdown.InlineLink:
+			return false
+		case *markdown.InlineImage:
+			return false
+		case *markdown.ReferenceLink:
+			return false
+		case *markdown.ReferenceImage:
+			return false
+		}
 
-	cbMessages := make([]string, 0)
-	codeBlocks := strings.Split(post.Message, "```")
-	codeBlockSkip := false
+		if textNode, ok := node.(*markdown.Text); ok {
+			startPos, endPos := textNode.Range.Position+offset, textNode.Range.End+offset
+			origText := postText[startPos:endPos]
+			if textNode.Text != origText {
+				mlog.Error(fmt.Sprintf("Markdown text did not match range text, '%s' != '%s'", textNode.Text, origText))
+				return true
+			}
 
-	// split and turn linker on/off if we think we're in a code block like ```hello```
-	for _, cb := range codeBlocks {
-		if codeBlockSkip {
-			cbMessages = append(cbMessages, cb)
-			codeBlockSkip = false
-		} else {
-			// split and turn linker on/off if we think we're in a inline code block like `hello`
-			icbMessages := make([]string, 0)
-			icodeBlocks := strings.Split(cb, "`")
-			icodeBlockSkip := false
-			for _, icb := range icodeBlocks {
-				if icodeBlockSkip {
-					icbMessages = append(icbMessages, icb)
-					icodeBlockSkip = false
-				} else {
-					for _, l := range links {
-						icb = l.Replace(icb)
-					}
-					icbMessages = append(icbMessages, icb)
-					icodeBlockSkip = true
-				}
+			newText := origText
+			for _, l := range links {
+				newText = l.Replace(newText)
 			}
 
-			cbMessages = append(cbMessages, strings.Join(icbMessages, "`"))
-			codeBlockSkip = true
+			if origText != newText {
+				postText = postText[:startPos] + newText + postText[endPos:]
+				offset += len(newText) - len(origText)
+			}
 		}
-	}
-
-	post.Message = strings.Join(cbMessages, "```")
-
-	// for _, l := range links {
-	// 	post.Message = l.Replace(post.Message)
-	// }
+		return true
+	})
+	post.Message = postText
 
 	return post, ""
 }
diff --git a/server/plugin_test.go b/server/plugin_test.go
@@ -36,12 +36,17 @@ func TestPlugin(t *testing.T) {
 	}
 }
 
-func TestCodeBlock(t *testing.T) {
-
+func TestSpecialCases(t *testing.T) {
 	links := make([]*Link, 0)
 	links = append(links, &Link{
 		Pattern:  "(Mattermost)",
 		Template: "[Mattermost](https://mattermost.com)",
+	}, &Link{
+		Pattern:  "(Example)",
+		Template: "[Example](https://example.com)",
+	}, &Link{
+		Pattern:  "(foobar)",
+		Template: "fb",
 	})
 	validConfiguration := Configuration{links}
 
@@ -129,6 +134,58 @@ func TestCodeBlock(t *testing.T) {
 			"```\n` Mattermost `\n```\nMattermost",
 			"```\n` Mattermost `\n```\n[Mattermost](https://mattermost.com)",
 		},
+		{
+			"  Mattermost",
+			"  [Mattermost](https://mattermost.com)",
+		},
+		{
+			"    Mattermost",
+			"    Mattermost",
+		},
+		{
+			"    ```\nMattermost\n    ```",
+			"    ```\n[Mattermost](https://mattermost.com)\n    ```",
+		},
+		{
+			"` ``` `\nMattermost\n` ``` `",
+			"` ``` `\n[Mattermost](https://mattermost.com)\n` ``` `",
+		},
+		{
+			"Mattermost \n Mattermost",
+			"[Mattermost](https://mattermost.com) \n [Mattermost](https://mattermost.com)",
+		},
+		{
+			"[Mattermost](https://mattermost.com)",
+			"[Mattermost](https://mattermost.com)",
+		},
+		{
+			"[  Mattermost  ](https://mattermost.com)",
+			"[  Mattermost  ](https://mattermost.com)",
+		},
+		{
+			"[  Mattermost  ][1]\n\n[1]: https://mattermost.com",
+			"[  Mattermost  ][1]\n\n[1]: https://mattermost.com",
+		},
+		{
+			"![  Mattermost  ](https://mattermost.com/example.png)",
+			"![  Mattermost  ](https://mattermost.com/example.png)",
+		},
+		{
+			"![  Mattermost  ][1]\n\n[1]: https://mattermost.com/example.png",
+			"![  Mattermost  ][1]\n\n[1]: https://mattermost.com/example.png",
+		},
+		{
+			"foobar\nExample\nfoobar Mattermost",
+			"fb\n[Example](https://example.com)\nfb [Mattermost](https://mattermost.com)",
+		},
+		{
+			"foobar",
+			"fb",
+		},
+		{
+			"foobarfoobar",
+			"foobarfoobar",
+		},
 	}
 
 	for _, tt := range tests {