mandolyte / mdtopdf

Markdown to PDF
MIT License
133 stars 32 forks source link

Theme support and additional options #35

Closed jessp01 closed 1 year ago

jessp01 commented 1 year ago

Please see full usage example here: https://github.com/jessp01/mdtopdf/tree/theme-support#additional-options

PS Eliminating the need for russian.go also means we'll now be able to install convert.go thusly:

$ go install github.com/mandolyte/mdtopdf/cmd@latest

At the moment, an attempt to do so yields:

../../go/pkg/mod/github.com/mandolyte/mdtopdf@v1.4.1/cmd/russian.go:13:5: input redeclared in this block
    ../../go/pkg/mod/github.com/mandolyte/mdtopdf@v1.4.1/cmd/convert.go:13:5: other declaration of input
../../go/pkg/mod/github.com/mandolyte/mdtopdf@v1.4.1/cmd/russian.go:14:5: output redeclared in this block
    ../../go/pkg/mod/github.com/mandolyte/mdtopdf@v1.4.1/cmd/convert.go:14:5: other declaration of output
../../go/pkg/mod/github.com/mandolyte/mdtopdf@v1.4.1/cmd/russian.go:15:5: help redeclared in this block
    ../../go/pkg/mod/github.com/mandolyte/mdtopdf@v1.4.1/cmd/convert.go:16:5: other declaration of help
../../go/pkg/mod/github.com/mandolyte/mdtopdf@v1.4.1/cmd/russian.go:17:6: main redeclared in this block
    ../../go/pkg/mod/github.com/mandolyte/mdtopdf@v1.4.1/cmd/convert.go:18:6: other declaration of main
../../go/pkg/mod/github.com/mandolyte/mdtopdf@v1.4.1/cmd/russian.go:59:6: usage redeclared in this block
    ../../go/pkg/mod/github.com/mandolyte/mdtopdf@v1.4.1/cmd/convert.go:73:6: other declaration of usage

I also think it would be good to change the name from convert.go to mdtopdf.go as it would mean one could conveniently run $GOBIN/mdtopdf after installing with go install github.com/mandolyte/mdtopdf/cmd@latest

Cheers,

jessp01 commented 1 year ago

For a quick review of the end result, here's the repo's MD, converted with:

$ go run convert.go  -i ../README.md -o mdtopdf.pdf \
  --new-page-on-hr --with-footer  --title "MDtoPDF" \
  --author "Cecil New" --theme dark --page-size A5

mdtopdf.pdf

mandolyte commented 1 year ago

@jessp01 I tried to duplicate the functionality of "russian.go", but it didn't work. Instead of the two Russian lines, it looks like garbled glyphs. I did a quick compare of your enhancement and the code in russian.go, but nothing popped out. Could you take a look at that?

I also noted that if I use the help flag, the double-hyphens are not shown for the new options that contain hyphens. I seem to recall that the flag package treats both single and double hyphens the same. So I think it is OK, but wanted to confirm that with you.

Thanks for this work!!

jessp01 commented 1 year ago

Hi @mandolyte ,

I just ran the below command (as added to the README) and it worked correctly (see attached PDF: russian.pdf):

go run convert.go -i russian.md -o russian.pdf \
    --unicode-encoding cp1251 --font-file helvetica_1251.json --font-name Helvetica_1251

Can you please post the command you tried to use?

As to the flag package, indeed, it honours both notations (- and --) for any flag you specify. So that:

go run convert.go -i russian.md -o russian.pdf \
    -unicode-encoding cp1251 -font-file helvetica_1251.json -font-name Helvetica_1251

Is parsed exactly like the first command. I don't see it as a problem but it is unconventional, which I'm not crazy about either:) For my own projects, I use github.com/urfave/cli which has a more comprehensive API and does allow you to specify both a short and long option for each flag. For example, see what I've done in my zaje project: https://github.com/jessp01/zaje/blob/master/common_functions.go#L87

We can change the code to use this package instead but I wouldn't say it's very important.

What do you think about changing the name from convert.go to md2pdf.go (or mdtopdf if you prefer)? I think that is rather important as convert is a bit generic (it's also the name of the main ImageMagick binary, which can be confusing).

Cheers,

mandolyte commented 1 year ago

@jessp01 Thanks for checking on that. I had not included the unicode-encoding option. Once I did that, it worked fine.

Also, I'm OK with renaming the convert command -- good idea. So please go ahead and include that rename in this PR and update the README to explain how to install the command directly.

Thanks!

jessp01 commented 1 year ago

Hi @mandolyte ,

Change made. In order for go install github.com/mandolyte/mdtopdf/cmd@latest to work, you'll need to create a new release and set it as the latest.

I also added .pre-commit-config.yaml to the repo. To use it:

This will create .git/hooks/pre-commit and following that, these tests will run every time you invoke git commit:

go fmt...................................................................Passed
go imports...............................................................Passed
go vet...................................................................Passed
go lint..................................................................Passed
go-critic................................................................Passed

This will make sure we adhere to certain standards and also catch some hard to spot errors.

Cheers,