These are chat archives for assetgraph/assetgraph

19th
Jul 2017
Peter Müller
@Munter
Jul 19 09:18
Having slept on the snap-to-svailable-props implementation I actually think it might make more sense to have it separate from getTextByFontProperties. Then getTextByFontProperties would have the sole responsiblity to represent what the code wants, and the snapping is combining that with relality. I think things might be a lot easier to test with that separation
Andreas Lind
@papandreou
Jul 19 09:19
@Munter That might be a good call, but then you also need to move the deduplication out (or repeat it elsewhere)?
Peter Müller
@Munter
Jul 19 09:20
Duplication might not really be a big problem, since the only step after this is reduction to a single character set. That has deduplication built-in
Andreas Lind
@papandreou
Jul 19 09:21
Hmm, right, I guess that's something I put in there to make the getTextByFontProperties tests pass.
Peter Müller
@Munter
Jul 19 09:22
If we want deduping we can pull that code out to a stand alone function and employ it twice. I have a feeling it wont be needed for this use case
Andreas Lind
@papandreou
Jul 19 09:38
Probably not. I guess it'll just look a bit weird in the getTextByFontProperties tests if there are duplicate entries in the returned array.
Peter Müller
@Munter
Jul 19 09:43
yeah, I would keep the implementation of that as-is
With a few extra test cases for lighter and bolder
Andreas Lind
@papandreou
Jul 19 09:48
Okay :+1: