fxn / zeitwerk

Efficient and thread-safe code loader for Ruby
MIT License
1.99k stars 118 forks source link

Make _abspath argument optional at the camelize method #296

Closed JuanVqz closed 4 months ago

JuanVqz commented 4 months ago

I noticed that the second argument for the camelize method was required, even when not using an absolute path. Therefore, I suggest making the second parameter of camelize completely optional.

Before

(rdbg) str
"users_controller"
(ruby) Zeitwerk::Inflector.new.camelize(str)
eval error: wrong number of arguments (given 1, expected 2)
  /home/ombu/code/ombu/oss/zeitwerk/lib/zeitwerk/inflector.rb:15:in `camelize'
  (rdbg)//home/ombu/code/ombu/oss/zeitwerk/test/lib/test_inflector.rb:1:in `camelize'
nil
(ruby) Zeitwerk::Inflector.new.camelize(str, nil)
"UsersController"
(ruby) Zeitwerk::Inflector.new.camelize(str, "random")
"UsersController"

After

(rdbg) str
"point_3d_value"
(ruby) Zeitwerk::Inflector.new.camelize(str)
"Point3dValue"
(ruby) Zeitwerk::Inflector.new.camelize(str, nil)
"Point3dValue"
(ruby) Zeitwerk::Inflector.new.camelize(str, "random")
"Point3dValue"

Please, let me know if I have to add a record in the CHANGELOG

BTW, I found rbs project interesting

Thanks for the gem!

fxn commented 4 months ago

Thanks. However, this inflector is meant to be used internally by the loader, or subclassed.

If used internally by the loader, it always gets two arguments. There is no use case for an optional second argument.

If you subclass, you can make the second argument optional if you plan to use the inflector directly.

Which is your use case for accessing the inflector directly?

JuanVqz commented 4 months ago

Thanks. However, this inflector is meant to be used internally by the loader, or subclassed.

If used internally by the loader, it always gets two arguments. There is no use case for an optional second argument.

If you subclass, you can make the second argument optional if you plan to use the inflector directly.

Which is your use case for accessing the inflector directly?

I don't really have a use case, I was playingaround with the Zeitwerk code and thought it could have a default value, feel free to close the PR. 👍

fxn commented 4 months ago

OK, thanks!