miekg / learninggo

Learning Go Book in mmark
https://miek.nl/go
Other
542 stars 111 forks source link

Add syntax highlight #40

Closed antoniomo closed 7 years ago

antoniomo commented 7 years ago
antoniomo commented 7 years ago

Hi! I added italics to code comments, I think it looks neat and subtle but I can remove it. Another thing I'm not sure about is to bold function names as those aren't part of the language, I can remove it but I wanted to get your opinion on it too.

Other two things you might want: http://prismjs.com/plugins/line-numbers/ http://prismjs.com/plugins/line-highlight/

Let me know if any of these two plugins are wanted and I can add them.

Also I need to add the "go" language hint to the inlined code blocks, let's not merge this PR until I do that in the next commit, but I already wanted your input on the previous things :)

Fixes #2

miekg commented 7 years ago

On my phone right now, but I like italic comments. The boldness should be limited to keywords only, I think.

Thanks!

On 7 Dec 2016 9:13 am, "Antonio Macías Ojeda" notifications@github.com wrote:

Hi! I added italics to code comments, I think it looks neat and subtle but I can remove it. Another thing I'm not sure about is to bold function names as those aren't part of the language, I can remove it but I wanted to get your opinion on it too.

Other two things you might want: http://prismjs.com/plugins/line-numbers/ http://prismjs.com/plugins/line-highlight/

Let me know if any of these two plugins are wanted and I can add them.

Also I need to add the "go" language hint to the inlined code blocks, let's not merge this PR until I do that in the next commit, but I already wanted your input on the previous things :)

Fixes #2 https://github.com/miekg/learninggo/issues/2

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/miekg/learninggo/pull/40#issuecomment-265395136, or mute the thread https://github.com/notifications/unsubscribe-auth/AAVkW6lvm21d3LS0XeflAK-NWU8Nxl44ks5rFnjHgaJpZM4LGWov .

antoniomo commented 7 years ago

Ok! I'll remove the bold from the function names. I'll add that on the next commit together with the language hints on all code blocks, hopefully tonight but if not at some point this week :)

antoniomo commented 7 years ago

Added "go" hints to all code blocks (If I didn't miss any). I also modified some code snippets to make sure the tabs showed at a consistent 4 spaces, which is what "most" examples used, but others not, so now it's consistent. Ready to merge if you like it :)

miekg commented 7 years ago

Looks nice. Thanks. Can you make it the default and remove the syntaxhl.sh script?

antoniomo commented 7 years ago

Thanks! It's now included as default in the make (or make all) paths of the Makefile, but do you prefer that logic in the Makefile itself rather than a separate script?

miekg commented 7 years ago

Why is a script needed?

On 12 Dec 2016 8:53 am, "Antonio Macías Ojeda" notifications@github.com wrote:

Thanks! It's now included as default in the make (or make all) paths of the Makefile, but do you prefer that logic in the Makefile itself rather than a separate script?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/miekg/learninggo/pull/40#issuecomment-266373948, or mute the thread https://github.com/notifications/unsubscribe-auth/AAVkW8g7bMa_eRZQP8RKrkVyuhybW43dks5rHQt5gaJpZM4LGWov .

antoniomo commented 7 years ago

I can add the prism.js script to the inc/head.html, but mmark only allows one -css clause and I wanted to keep the prism-bold.css style separate from learninggo.css, so that it's easy to change the code syntax highlight by the user if he wants to. But I can add just that bit of logic to the Makefile as well :)

miekg commented 7 years ago

Yeah, I see. Just add it to inc/head.html. Keeping prism-bold.css separate is good point... Just use the @import url("orderedlist-minimal.css"); that is already being used.

With those changes, it becomes nice and clean, no script and the files are still update-able because they are separate.

antoniomo commented 7 years ago

Perfect! Will do tonight!

miekg commented 7 years ago

Ok, looks better. Sadly there are know conflicts because of the large changes to the .md file (as I merged another PR in the mean time). It would have been a better idea to not change those files in this PR and do a follow up PR with only those changes. Also why is code-style.css a symlink to prism.css?

antoniomo commented 7 years ago

Conflicts fixed :)

antoniomo commented 7 years ago

Code-style is a symlink so that people can change only that link without changing the include, in case they want to use another syntax highlighting css, seemed cleaner than having to edit.

miekg commented 7 years ago

[ Quoting notifications@github.com in "Re: [miekg/learninggo] Add syntax h..." ]

Code-style is a symlink so that people can change only that link without changing the include, in case they want to use another syntax highlighting css, seemed cleaner than having to edit.

Please don't prematurely optimize for this. It serves no need (yet) and adds an unnecessary indirection.

/Miek

-- Miek Gieben

antoniomo commented 7 years ago

Done :)

miekg commented 7 years ago

And live at https://miek.nl/go

Thanks!

antoniomo commented 7 years ago

Awesome!