asciidoctor / asciidoctor-latex

:triangular_ruler: Add LaTeX features to AsciiDoc & convert AsciiDoc to LaTeX
http://asciidoctor.org
Other
114 stars 26 forks source link

Bug fixes & code improvements #46

Closed yacinehmito closed 7 years ago

yacinehmito commented 8 years ago

I am sharing the couple of bug fixes I needed to make for writing my memoir. See the commit messages for the details.

jxxcarlson commented 8 years ago

Am in Morocco on vacation — will look at this at the end of the week when I return.

Thanks!

On Jul 8, 2016, at 11:05 PM, Yacine Hmito notifications@github.com wrote:

I am sharing the couple of bug fixes I needed to make for writing my memoir. See the commit messages for the details.

You can view, comment on, or merge this pull request online at:

https://github.com/asciidoctor/asciidoctor-latex/pull/46 https://github.com/asciidoctor/asciidoctor-latex/pull/46 Commit Summary

Quick fix for manual TOC placement Fix problematic empty line in split Improved TeX utilites Image macro now takes :imagesdir: into account Env blocks are truly compound File Changes

M lib/asciidoctor/latex/environment_block.rb https://github.com/asciidoctor/asciidoctor-latex/pull/46/files#diff-0 (7) M lib/asciidoctor/latex/node_processors.rb https://github.com/asciidoctor/asciidoctor-latex/pull/46/files#diff-1 (72) Patch Links:

https://github.com/asciidoctor/asciidoctor-latex/pull/46.patch https://github.com/asciidoctor/asciidoctor-latex/pull/46.patch https://github.com/asciidoctor/asciidoctor-latex/pull/46.diff https://github.com/asciidoctor/asciidoctor-latex/pull/46.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/asciidoctor/asciidoctor-latex/pull/46, or mute the thread https://github.com/notifications/unsubscribe/AApgN-knWqI6uHK0u7khTbCE22Bz7_Iaks5qTteqgaJpZM4JId2E.

mojavelinux commented 8 years ago

I'm very glad to see the addition of the braces method to handle an arbitrary number of arguments. It's always good to avoid handling only a fixed number of positional arguments.

yacinehmito commented 8 years ago

Thanks for the review. It reminded me of the PR. I amended the commits to add your snippet about imagesdir and clean up the code in the Fix problematic empty line in split commit.

yacinehmito commented 8 years ago

What is the status on this?

yacinehmito commented 7 years ago

Bumping.

prudhomm commented 7 years ago

has this branch been merged into master?

yacinehmito commented 7 years ago

It hasn't been.

yacinehmito commented 7 years ago

In light of the recent activity on the repo, I'm bumping this PR.

tribut commented 7 years ago

Not sure if it means anything to you guys, but I reviewed at all the commits in this PR and recommend merging.

It fixes bugs, does some necessary cleanup and adds support for the toc macro.

mojavelinux commented 7 years ago

@yacinehmito Can you add a new commit (or force push a previous one) so that the CI job runs?

mojavelinux commented 7 years ago

Not sure if it means anything to you guys, but I reviewed at all the commits in this PR and recommend merging.

Of course that means something! It means a lot, in fact. Thanks!

jxxcarlson commented 7 years ago

@yacinehmito @mojavelinux @tribut @prudhomm Thank you all!

jxxcarlson commented 7 years ago

Oops, there are some failing tests -- likely the tests are at fault. They are very sensitive to minor changes. I will try to get this resolved in the next few hours.

jirutka commented 7 years ago

IIRC you hadn’t finished the tests and many of them were failing since ever, but it’s quite a long time, so I’m not sure.

Doctest examples in TeX are indeed sensitive to any kind of change in output, ’cause there’s no normalization. I still don’t know if it’s even possible to write generic normalization and/or pretty-formatter for TeX syntax.

HTML examples are not sensible for changes that do not have any semantic meaning (like order of attributes, formatting etc.), because Doctest compares them in a normalized form.

yacinehmito commented 7 years ago

Thanks!