-
Notifications
You must be signed in to change notification settings - Fork 689
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
Add Document#font_style and Document#font_family #1225
base: master
Are you sure you want to change the base?
Changes from 5 commits
0221daa
70a89b1
3673da5
ec8bb83
04cc07a
d81c04a
8bc9f0b
ff13eaf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,25 +1,51 @@ | ||
# frozen_string_literal: true | ||
|
||
# The <code>font_style</code> method works just like the <code>font</code> | ||
# method. | ||
# | ||
# In fact we can even use <code>font</code> with the <code>:style</code> option | ||
# to declare which size we want. | ||
# | ||
# Another way to change the font size is by supplying the <code>:style</code> | ||
# option to the text methods. | ||
# | ||
# Most font families come with some styles other than normal. Most common are | ||
# <code>bold</code>, <code>italic</code> and <code>bold_italic</code>. | ||
# | ||
# The style can be set the using the <code>:style</code> option, with either the | ||
# <code>font</code> method which will set the font and style for rest of the | ||
# document, or with the inline text methods. | ||
|
||
require_relative '../example_helper' | ||
|
||
filename = File.basename(__FILE__).gsub('.rb', '.pdf') | ||
Prawn::ManualBuilder::Example.generate(filename) do | ||
fonts = %w[Courier Helvetica Times-Roman] | ||
styles = %i[bold bold_italic italic normal] | ||
text "The default style is normal" | ||
|
||
move_down 10 | ||
font_style :bold | ||
text 'This is bold' | ||
|
||
fonts.each do |example_font| | ||
move_down 20 | ||
move_down 10 | ||
font_style :italic | ||
text 'This is italic (not bold, i.e. existing style is overwritten)' | ||
|
||
styles.each do |style| | ||
font example_font, style: style | ||
text "I'm writing in #{example_font} (#{style})" | ||
end | ||
move_down 10 | ||
font_style :bold_italic | ||
text 'This is bold italic' | ||
|
||
move_down 10 | ||
font_style :normal | ||
text 'Back to normal' | ||
|
||
move_down 10 | ||
text 'A single line of italic', style: :italic | ||
|
||
move_down 10 | ||
font_style :bold do | ||
text 'A single line of bold' | ||
end | ||
|
||
move_down 10 | ||
font 'Courier', style: :bold_italic | ||
text 'This is Courier bold italic' | ||
|
||
font 'Courier' | ||
text 'This is Courier normal (style is reset when changing font)' | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -111,11 +111,123 @@ | |
end | ||
end | ||
|
||
describe '#font' do | ||
it 'allows setting of size directly when font is created' do | ||
pdf.font 'Courier', size: 16 | ||
expect(pdf.font_size).to eq(16) | ||
end | ||
|
||
it 'allows temporary setting of a new font using a transaction' do | ||
pdf.font 'Helvetica', size: 12 | ||
|
||
pdf.font 'Courier', size: 16, style: :bold do | ||
expect(pdf.font_family).to eq('Courier') | ||
expect(pdf.font_size).to eq(16) | ||
expect(pdf.font_style).to eq(:bold) | ||
end | ||
|
||
expect(pdf.font.name).to eq('Helvetica') | ||
expect(pdf.font_size).to eq(12) | ||
expect(pdf.font_style).to eq(:normal) | ||
end | ||
|
||
it 'masks font size when using a transacation' do | ||
pdf.font 'Courier', size: 16 do | ||
expect(pdf.font_size).to eq(16) | ||
end | ||
|
||
pdf.font 'Times-Roman' | ||
pdf.font 'Courier' | ||
|
||
expect(pdf.font_size).to eq(12) | ||
end | ||
end | ||
|
||
describe '#font_size' do | ||
it 'returns default font size' do | ||
expect(pdf.font_size).to eq(12) | ||
end | ||
|
||
it 'allows setting font size in DSL style' do | ||
pdf.font_size 20 | ||
expect(pdf.font_size).to eq(20) | ||
end | ||
|
||
it 'allows setting font size in DSL style using a transaction' do | ||
pdf.font_size 20 do | ||
expect(pdf.font_size).to eq(20) | ||
end | ||
|
||
expect(pdf.font_size).to eq(12) | ||
end | ||
|
||
it 'allows setting font size as assignment' do | ||
pdf.font_size = 20 | ||
expect(pdf.font_size).to eq(20) | ||
end | ||
end | ||
|
||
describe '#font_style' do | ||
it 'returns default font style' do | ||
expect(pdf.font_style).to eq(:normal) | ||
end | ||
|
||
it 'allows setting font style in DSL style' do | ||
pdf.font_style :bold | ||
expect(pdf.font_style).to eq(:bold) | ||
expect(pdf.font.name).to eq('Helvetica-Bold') | ||
end | ||
|
||
it 'allows setting font style in DSL style using a transaction' do | ||
pdf.font_style :bold do | ||
expect(pdf.font_style).to eq(:bold) | ||
expect(pdf.font.name).to eq('Helvetica-Bold') | ||
end | ||
|
||
expect(pdf.font_style).to eq(:normal) | ||
expect(pdf.font.name).to eq('Helvetica') | ||
end | ||
|
||
it 'allows setting font style for a TTF font' do | ||
pdf.font_families.update( | ||
'DejaVu Sans' => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the test case. However, I think to really test this out we need another entry where the key is different from the font family name, e.g. using 'deja' as key. I don't think that the current implementation would work since the look-up key would be the family name from the TTF. |
||
normal: "#{Prawn::DATADIR}/fonts/DejaVuSans.ttf", | ||
bold: "#{Prawn::DATADIR}/fonts/DejaVuSans-Bold.ttf" | ||
} | ||
) | ||
pdf.font 'DejaVu Sans' | ||
expect(pdf.font_style).to eq(:normal) | ||
pdf.font_style :bold | ||
expect(pdf.font_style).to eq(:bold) | ||
expect(pdf.font_family).to eq('DejaVu Sans') | ||
expect(pdf.font.name).to eq("#{Prawn::DATADIR}/fonts/DejaVuSans-Bold.ttf") | ||
end | ||
|
||
it 'allows setting font style as assignment' do | ||
pdf.font_style = :bold | ||
expect(pdf.font_style).to eq(:bold) | ||
end | ||
end | ||
|
||
describe '#font_family' do | ||
it 'returns default font family' do | ||
expect(pdf.font_family).to eq('Helvetica') | ||
end | ||
|
||
it 'returns font family' do | ||
pdf.font 'Courier', style: :bold | ||
expect(pdf.font_family).to eq('Courier') | ||
end | ||
|
||
it 'returns font family for AFM font' do | ||
pdf.font 'ZapfDingbats' | ||
expect(pdf.font_family).to eq('ZapfDingbats') | ||
end | ||
|
||
it 'returns font family for TTF font' do | ||
pdf.font "#{Prawn::DATADIR}/fonts/DejaVuSans-Bold.ttf" | ||
expect(pdf.font_family).to eq('DejaVu Sans') | ||
end | ||
end | ||
|
||
describe 'font style support' do | ||
|
@@ -142,11 +254,14 @@ | |
pdf.font 'Helvetica' | ||
pdf.text 'In Normal Helvetica' | ||
|
||
pdf.font nil, style: :bold | ||
pdf.text 'In Bold Helvetica' | ||
|
||
text = PDF::Inspector::Text.analyze(pdf.render) | ||
expect(text.font_settings.map { |e| e[:name] }).to eq( | ||
%i[ | ||
Courier-Bold Courier-BoldOblique Courier-Oblique | ||
Courier Helvetica | ||
Courier Helvetica Helvetica-Bold | ||
] | ||
) | ||
end | ||
|
@@ -233,36 +348,6 @@ | |
end | ||
end | ||
|
||
describe 'Transactional font handling' do | ||
it 'allows setting of size directly when font is created' do | ||
pdf.font 'Courier', size: 16 | ||
expect(pdf.font_size).to eq(16) | ||
end | ||
|
||
it 'allows temporary setting of a new font using a transaction' do | ||
pdf.font 'Helvetica', size: 12 | ||
|
||
pdf.font 'Courier', size: 16 do | ||
expect(pdf.font.name).to eq('Courier') | ||
expect(pdf.font_size).to eq(16) | ||
end | ||
|
||
expect(pdf.font.name).to eq('Helvetica') | ||
expect(pdf.font_size).to eq(12) | ||
end | ||
|
||
it 'masks font size when using a transacation' do | ||
pdf.font 'Courier', size: 16 do | ||
expect(pdf.font_size).to eq(16) | ||
end | ||
|
||
pdf.font 'Times-Roman' | ||
pdf.font 'Courier' | ||
|
||
expect(pdf.font_size).to eq(12) | ||
end | ||
end | ||
|
||
describe 'Document#page_fonts' do | ||
it 'registers fonts properly by page' do | ||
pdf.font 'Courier' | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, thinking a bit more about this, why not just pass
nil
here? Then it would automatically use what's defined in#font
and we would have only one place where it would be defined.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another thing that came to mind: What happens if we pass the path to a e.g. TrueType font into
#font
and then callfont_style(:bold)
?Since nothing is defined and we cannot know which file contains the bold version, I guess the only thing to do is to error out here. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should check here for
font_families.key?(font_family)
and error out if nothing is found. This would solve both problems, the one when using a TrueType font path and the one where the used key is different from the family name. The error message should probably include the reason for the failure, i.e. that the family name was not registered.And updating the documentation for
#font_families
in this regard would also make sense.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that calling
font_style
with a key not present infont_families
for the current font should raise an error. Likewise if style is other thannormal
for a specific font file.Currently
font
does not raise in the latter situation, and changing this would be a BC break, so I suggest we avoid raising in that specific situation (font
called with style other than normal for a specific font file). I imagine that it is not uncommon that people do e.g.font 'ArialBlack.ttf', style: :bold
in some (mis-guided) attempt to signal that this is a bold font.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed on both accounts.
@pointlessone What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a little confusing because we have two very similar things:
font_families
which afford for stylesfont_registry
which does not quite do that.font_families
eventually gets mapped to entries infont_registry
. Style is only meaningful on the stage of this mapping. Whenever a font file is used directly style has no meaning. It still will be present in the font but there's no way to look up a font for any other style. We never add these fonts tofont_families
, we never look up fonts by their internal names so we're unable to correlate fonts from the same family. I don't think this should change now. Maybe in the next major release if we ever get there.I suggest, we make
font_style(:random_style) { ... }
a noop but issue awarn
ing. Possibly, when$DEBUG
is set but it rarely is so it's not very useful.