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

format_result should have only 1 parameter #1

Merged
merged 1 commit into from
Aug 3, 2011

Conversation

miaout17
Copy link
Contributor

@miaout17 miaout17 commented Aug 3, 2011

It's a little complex to explain the issue....
In my mind, I think the problem is:

In "ripl way" (hooking methods by including modules)
The parameter number of basis method (defined in ripl core) should not be added.

For example, if I am using hirb with ripl-rc
Hirb will render some data types to table format.
If it can't, it will invoke super and let parent class(module) to do the work.

Thus, the calling chain would be:

  • Ripl::Hirb::format_result(result)
  • Ripl::RC::Color.format_result(result, display = result.inspect)

However, if we pass 2 parameters to format_result, Ripl::Hirb::format_result will raise ArgumentError.
This will also occurs if we have other format_result hooks before ripl-rc.

The solution might be using a auxiliary method.
I proposed a fix, feel free to improve it.

@godfat godfat merged commit 5121c88 into godfat:master Aug 3, 2011
@godfat
Copy link
Owner

godfat commented Aug 3, 2011

Thank you!

I understand that changing arity is really a bad idea,
but since it's working, so I didn't really doubt it very much.

I guess the problem here is, I didn't use any plugin other
than ripl-rc, so I didn't know it was causing problems.

I bet it must have other bugs when mixing plugins...
or not enabling all of the plugins in ripl-rc.

It would be excellent if someone has other use cases than me.

Thanks again.

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

Successfully merging this pull request may close these issues.

2 participants