Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

theme: Allow custom page size setting #84

Merged
merged 1 commit into from
May 13, 2015
Merged

theme: Allow custom page size setting #84

merged 1 commit into from
May 13, 2015

Conversation

otavio
Copy link
Contributor

@otavio otavio commented Jan 10, 2015

To allow a custom page size to be used, we need to check if the
argument passed in the theme is a String, for which we ought to upcase
to use in Prawn converted, or keep the custom value to be passed over
it.

This has been tested using a custom theme with:

,----
| ...
| page:
| ...
| size: [508, 635]
| ...
| ...
`----

Signed-off-by: Otavio Salvador otavio@ossystems.com.br

@mojavelinux
Copy link
Member

Nice improvement!

I have one suggestion. Could you write the assignment as follows?

pdf_opts[:page_size] = pdf_opts[:page_size].upcase if ::String === pdf_opts[:page_size]

In Ruby, === is equivalent to is_a? in this arrangement, but slightly faster. I also avoided the use of string modification because it's best to avoid that practice.

To allow a custom page size to be used, we need to check if the
argument passed in the theme is a String, for which we ought to upcase
to use in Prawn converted, or keep the custom value to be passed over
it.

This has been tested using a custom theme with:

,----
| ...
| page:
|   ...
|   size: [508, 635]
|   ...
| ...
`----

Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
@mojavelinux mojavelinux added this to the v1.5.0 milestone May 12, 2015
@mojavelinux
Copy link
Member

This partially addresses #65. I say partially because it must be set in the theme file rather than the document, so we'll still need a change to allow it to be set in by the document.

@mojavelinux
Copy link
Member

This looks great @otavio! Thanks!

mojavelinux added a commit that referenced this pull request May 13, 2015
theme: Allow custom page size setting
@mojavelinux mojavelinux merged commit 4c66b2c into asciidoctor:master May 13, 2015
@mojavelinux mojavelinux self-assigned this May 13, 2015
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants