ruby-gettext / gettext

Gettext gem is a pure Ruby Localization(L10n) library and tool which is modeled after the GNU gettext package.
https://ruby-gettext.github.io/
69 stars 28 forks source link

Support Pathnames in $LOAD_PATH #32

Closed glacials closed 10 years ago

glacials commented 10 years ago

This allows $LOAD_PATH to contain Pathname objects in addition to strings. Previously Pathnames would break any attempt to precompile assets because of an undefined match method.

kou commented 10 years ago

Your suggestion is reasonable but we should use #to_path instead of #to_s for the case. Could you fix it?

glacials commented 10 years ago

We can't call to_path on strings, so we'd need to have a condition to check for Pathnames and call to_path on them while letting strings through normally.

This would be fine if to_path were more appropriate than to_s, but they're literally the exact same method, so there would be no advantage to using to_path here (any gained semantic clarity would be fogged up by the additional type check imo).

That said, I can still change this if you'd like me to.

kou commented 10 years ago

In your use case, I agree with using #to_path is no advantage. Because you just need to care about String and Pathname.

But gettext gem is a library that is used by more users. They may put a path object that responds to #to_path to $LOAD_PATH. For example, we can put Dir object to $LOAD_PATH. Because Dir#to_path exists.

Because of the above reason, I prefer to use #to_path rather than #to_s.

It is better that using path.respond_to?(:to_path) rather than path.is_a?(Pathname) for checking whether we should call #to_path or not.

If you agree with my opinion, could you use #to_path?

glacials commented 10 years ago

That's a really good point! I hadn't thought of that. Here's the change.

kou commented 10 years ago

Thanks!!! I've merged your pull request. :-)