mandolyte / mdtopdf

Markdown to PDF
MIT License
133 stars 32 forks source link

Non-local and non-png images cause conversion error #6

Closed sirnewton01 closed 6 years ago

sirnewton01 commented 6 years ago

Use a markdown document that references an image that is either not local or isn't a png. An example is the README.md for this project, which has a godoc image link. Run mktopdf -i -o test.pdf.

Expected behaviour is that the output doesn't has the image, fine since not all markdown/HTML syntax is supported. A warning might be useful.

Instead, export completely fails with an error indicating that it cannot open the file path and dumps the http URL for the image link.

sirnewton01 commented 6 years ago

I patched this locally by using this diff:

diff --git a/nodeProcessing.go b/nodeProcessing.go
index 3a91607..8ef5d26 100644
--- a/nodeProcessing.go
+++ b/nodeProcessing.go
@@ -21,6 +21,7 @@ package mdtopdf

 import (
    "fmt"
+   "os"
    "strings"

    "github.com/jung-kurt/gofpdf"
@@ -203,9 +204,18 @@ func (r *PdfRenderer) processImage(node *bf.Node, entering bool) {
            fmt.Sprintf("Destination[%v] Title[%v]",
                string(node.LinkData.Destination),
                string(node.LinkData.Title)))
-       r.Pdf.ImageOptions(string(node.LinkData.Destination),
-           -1, 0, 0, 0, true,
-           gofpdf.ImageOptions{ImageType: "PNG", ReadDpi: true}, 0, "")
+
+       // Do a basic sanity check that this is a PNG and it is local
+
+       var imgPath = string(node.LinkData.Destination)
+
+       if _, err := os.Stat(imgPath); err == nil {
+           if strings.HasSuffix(string(node.LinkData.Destination), "png") {
+               r.Pdf.ImageOptions(string(node.LinkData.Destination),
+                   -1, 0, 0, 0, true,
+                   gofpdf.ImageOptions{ImageType: "PNG", ReadDpi: true}, 0, "")
+           }
+       }
    } else {
        r.tracer("Image (leaving)", "")
    }
sirnewton01 commented 6 years ago

Great project, btw. It's nice to see non-HTML output options for markdown.

sirnewton01 commented 6 years ago

Actually, the ImageOptions struct allows an unspecified image type and it will auto-detect based on the file name according to the godocs. Here is a revised diff that should handle jpeg, gif and png.

diff --git a/nodeProcessing.go b/nodeProcessing.go
index 3a91607..11f293c 100644
--- a/nodeProcessing.go
+++ b/nodeProcessing.go
@@ -21,6 +21,7 @@ package mdtopdf

 import (
    "fmt"
+   "os"
    "strings"

    "github.com/jung-kurt/gofpdf"
@@ -203,9 +204,16 @@ func (r *PdfRenderer) processImage(node *bf.Node, entering bool) {
            fmt.Sprintf("Destination[%v] Title[%v]",
                string(node.LinkData.Destination),
                string(node.LinkData.Title)))
-       r.Pdf.ImageOptions(string(node.LinkData.Destination),
-           -1, 0, 0, 0, true,
-           gofpdf.ImageOptions{ImageType: "PNG", ReadDpi: true}, 0, "")
+
+       // Do a basic sanity check that this is a PNG and it is local
+
+       var imgPath = string(node.LinkData.Destination)
+
+       if _, err := os.Stat(imgPath); err == nil {
+           r.Pdf.ImageOptions(string(node.LinkData.Destination),
+               -1, 0, 0, 0, true,
+               gofpdf.ImageOptions{ImageType: "", ReadDpi: true}, 0, "")
+       }
    } else {
        r.tracer("Image (leaving)", "")
    }
mandolyte commented 6 years ago

Thanks! I may not have time to incorporate this in the next few days. This week is a bit crazy.

sirnewton01 commented 6 years ago

There's no rush. I have my patch and its working great for my purposes. Yeah, crazy week all around I think.

mandolyte commented 6 years ago

Fix is incorporated and auto-detection working. Added a JPEG image to improve test case. Also added an error message to the trace/log file when file does not actually exist locally.