codeplant/simple-navigation

rendering trouble

Closed this issue · 14 comments

render_navigation produces a strange looking link:
Label#SimpleNavigation::Item:0x7f59c26fb6c0

Label is ok, but the rest is odd. Could it be that I am trying to run simple-navigation on ruby 1.8? I changed a lot of hash syntax troubles, but it runs, now.
Best regards.

Hi @zdravko could you give us some details about the following points? :)

  • SimpleNavigation version
  • Environment (Rails, Sinatra, ...) and its version
  • Your navigation.rb file

Note that Ruby 1.8 is not supported though ;)

Hi, @simonc !

SimpleNavigation version 3.13.0
Rails 3.2.18
just a simple menu:

SimpleNavigation::Configuration.run do |navigation|
primary.item :patients, 'Label', patients_url(@project)
end

Yes, I know 1.8 is not supported... but somehow I have hopes it will do
the job.

Best regards.

Ok :) But how do you plan to make SN work on 1.8? Do you patch the Hash syntax or something like that?
I don't really know what to say. What renderer to you use?

Patching hash syntax is easy. Sorry for downgrade.
I use html.erb template files.
Still searching for where to check things regarding
this issue. Really strange. Any hint would be welcome.

Regards, Zdravko.

That's why I was asking for the way you try to use SN. Understanding how you patched it could help me understand what could be going wrong.

Would you be able to show your complete patch diff?

Simon, hi!
This looks terribly wrong what I've done. In item.rb in class Item
there is a 'name' method that runs name_generator (bellow)
Originally the call contains 'self' argument, like this:
SimpleNavigation.config.name_generator.call(@name, self)

I have removed the 'self' part and it appears to be working.

def name(options = {})
  options = { :apply_generator => true }.merge(options)
  if (options[:apply_generator])
    SimpleNavigation.config.name_generator.call(@name)
  else
    @name
  end
end

How much did I mess this up?

Regards, Zdravko

On Tue, 2014-07-29 at 00:15 -0700, Simon Courtois wrote:

That's why I was asking for the way you try to use SN. Understanding
how you patched it could help me understand what could be going wrong.

Would you be able to show your complete patch diff?


Reply to this email directly or view it on GitHub.

I'm sorry @zdravko but without a comprehensive diff of your patches I can't tell you much.
The modification you're talking about doesn't say what could be going on :/

I'll make a full diff, alright.
Did you find where I made the change?
What is the function of the 'self' argument to the name_generator?
I'm way to weak in Ruby to say if this is a good fix.
It doesn't look so, it only works.

I didn't make any other patches except syntax one.

Regards, Zdravko
ps. I'll need to fork your repository, so I can make a diff, and I can't
do that immediately.

On Tue, 2014-07-29 at 03:02 -0700, Simon Courtois wrote:

I'm sorry @zdravko but without a comprehensive diff of your patches I
can't tell you much.
The modification you're talking about doesn't say what could be going
on :/


Reply to this email directly or view it on GitHub.

The use of self as an argument for name_generator is to provide a way to generate an item's name based on something else than the config-provided name.

If your patch works for you I'll consider the issue closed if you're ok with it :)

But this self then isn't used in name_generator?
Why?

On Tue, 2014-07-29 at 03:31 -0700, Simon Courtois wrote:

The use of self as an argument for name_generator is to provide a way
to generate an item's name based on something else than the
config-provided name.

If your patch works for you I'll consider the issue closed if you're
ok with it :)


Reply to this email directly or view it on GitHub.

Because it doesn't need to be used by default, the self is provided for more complex uses of name_generator.

You can find some history about this in the following PR #144

Well, then, I can't tell. Please, you decide.
Here is my proposal for @name_generator method
in Configuration class:
def name_generator
@name_generator ||= proc { |name, item| name }
end
I took this from your comments in config file,
where proc has two arguments.
It works, too. (with the 'self' argument we disscussed before.)

Great gem, though. Thanks.
Regards, Zdravko

As long as the patch works for you, it's ok, Maybe it's a matter of blocks arity or something like that.

In any case, nothing will be merged tu support Ruby 1.8. I'm sorry but if you plan on staying with this version you'll have to maintain your own fork of SN :)

I'll close this issue now that you have a working version.
Thank you for your feedback ;)