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

Theme support and additional options #35

Merged
merged 11 commits into from
Aug 1, 2023

Conversation

jessp01
Copy link
Collaborator

@jessp01 jessp01 commented Jul 27, 2023

  • Support themes (dark, light)
  • Support footer (author, title, page number)
  • Additional flags:
    • --new-page-on-hr: Interpret HR as a new page; useful for presentations
    • --page-size: [A3 | A4 | A5] (default "A4")
    • --orientation: [portrait | landscape] (default "portrait")
  • Added Colorlookup() helper which translates common colour strings ("blue", "green", "red", etc) to RGBs
  • Support processing input from URL
  • When processing from a URL, adjust relative links and image paths
  • Support Unicode encoding and custom font previously done in russian.go in convert.go

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,

- Support unicode encoding and custom font previously done in `russian.go` in `convert.go`
- Support footer (author, title, page number)
- Additional flags:
    * `-new-page-on-hr`: Interpret HR as a new page; useful for presentations
    * `-page-size`: [A3 | A4 | A5] (default "A4")
@jessp01
Copy link
Collaborator Author

jessp01 commented Jul 27, 2023

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
Copy link
Owner

@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
Copy link
Collaborator Author

jessp01 commented Jul 31, 2023

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
Copy link
Owner

@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!

- Added .pre-commit-config.yaml (see https://pre-commit.com for usage
  instructions)
@jessp01
Copy link
Collaborator Author

jessp01 commented Jul 31, 2023

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,

Copy link
Owner

@mandolyte mandolyte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mandolyte mandolyte merged commit 1bca6ab into mandolyte:master Aug 1, 2023
@mandolyte mandolyte mentioned this pull request Aug 1, 2023
@jessp01 jessp01 mentioned this pull request Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants