Skip to content
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

resolves #1022 provide more information about missing font #1045

Merged
merged 1 commit into from
Nov 14, 2017

Conversation

mojavelinux
Copy link
Contributor

  • when an afm font is missing, provide more information to the user
  • look for font name in family option in case of missing TTF

@pointlessone
Copy link
Member

@mojavelinux Could you please fix the build?

@mojavelinux
Copy link
Contributor Author

I forgot to run rubocop. I've addressed the violations.

For the record, I have no idea how Travis is running rubocop. When I run it locally, I get 13,000 violations. It would be nice to know what the right command is so I can actually test it locally.

@pointlessone
Copy link
Member

Default Rake task is running both Rubocop checks and specs. Use bundler to be sure you're running all the right versions.

bundle exec rake

@mojavelinux
Copy link
Contributor Author

Aha, I had the gem extracted under pkg, which was getting caught up in the glob. I think the glob needs to be more specific because it's including stuff it shouldn't.

I took that away and I still get violations. Here's a partial output using Ruby 2.4.1.

lib/prawn/graphics/patterns.rb:1:1: C: Missing frozen string literal comment.
require 'digest/sha1'
^
lib/prawn/image_handler.rb:1:1: C: Missing frozen string literal comment.
# ImageHandler provides a way to register image processors with Prawn
^
lib/prawn/security/arcfour.rb:1:1: C: Missing frozen string literal comment.
# Implementation of the "ARCFOUR" algorithm ("alleged RC4 (tm)"). Implemented
^
lib/prawn/encoding.rb:1:1: C: Missing frozen string literal comment.
# Copyright September 2008, Gregory Brown, James Healy  All Rights Reserved.
^
lib/prawn/font_metric_cache.rb:1:1: C: Missing frozen string literal comment.
# font_metric_cache.rb : The Prawn font class

@mojavelinux
Copy link
Contributor Author

Strange. I cloned the project again in a fresh space and then the rubocop passed. I have no idea why I'm getting different results.

@mojavelinux
Copy link
Contributor Author

I've tried everything I can think of. I don't know why this old checkout is giving me errors. There are no additional files reported by git. Whatever, I'll just clone it again.

@pointlessone
Copy link
Member

You should never use a packaged gem for development. Packaged gem used to be a proper distribution mechanism at the very beginning of Ruby gems but since then it has changed (about 14 years ago when RubyForge came about). Nowadays it's actively discouraged to package supporting infrastructure. Packaged gem should only contain files that are required for proper execution and documentation generation.

As for your checkout you may have different version of Rubocop installed. Our config inherits from the default one so somewhat version-dependent.

@mojavelinux
Copy link
Contributor Author

If I run the tests from /tmp/prawn, I get no violations. If I move the prawn directory into my home directory, then I get violations. So it's clearly looking at parent directories, so it's not really a portable set up.

@pointlessone
Copy link
Member

That's interesting. Rubocop does indeed traverse tree to find config but it should not go all the way tho the /. I should check the Rubocop code…

@pointlessone
Copy link
Member

@mojavelinux Do you have a merge bit on this repo?

@mojavelinux
Copy link
Contributor Author

The problem seems to be that my folder structure is "projects/prawn/prawn". If I rename it to "projects/prawn/prawn-core" then I get no violations. I don't know what the heck is going on there.

@mojavelinux
Copy link
Contributor Author

But on a different computer, I don't get that problem. It seems like information has been cached somewhere. I just don't know where that is. I'll keep poking at it to see if I can figure out what is causing the delta.

@mojavelinux
Copy link
Contributor Author

Do you have a merge bit on this repo?

It appears so, but merging is blocked unless there is one approved review.

pointlessone
pointlessone previously approved these changes Nov 8, 2017
@mojavelinux
Copy link
Contributor Author

You should never use a packaged gem for development.

I 100% agree with you. What I meant to say was that I had some junk inside the project left behind when I was looking for something. I should have just said, "I left behind a temporary directory".

But that's a separate issue from why I get violations when the folder structure is "projects/prawn/prawn". But I'm willing to say that it's a dirty cache somewhere.

@pointlessone
Copy link
Member

merging is blocked unless there is one approved review

Indeed. My bad. 😅

@mojavelinux
Copy link
Contributor Author

👍

@mojavelinux
Copy link
Contributor Author

Regarding rubocop, each computer I try I get different results. If I run bundle exec rake I get different results than bundle exec rubocop.

- when an afm font is missing, provide more information to the user
- look for font name in family option in case of missing TTF
- add test to confirm error is raised and message is correct
@mojavelinux
Copy link
Contributor Author

I added an entry to the changelog. Could you approve again?

@mojavelinux
Copy link
Contributor Author

@pointlessone Did you want me to merge?

@pointlessone
Copy link
Member

@mojavelinux Yes, please.

@mojavelinux mojavelinux merged commit 50e1175 into prawnpdf:master Nov 14, 2017
@mojavelinux mojavelinux deleted the issue-1022 branch November 14, 2017 06:32
@mojavelinux
Copy link
Contributor Author

Done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants