Is there any better way to do this?

<li<% if controller.controller_name == 'users' && controller.action_name == 'login' %> class="current"<% end %>>

Probably safe syntax. I usually don't like to print things inside of operations. Plus, with this, you won't get any unnecessary whitespace.

But, there is no right way. As long as that Ruby interpreter does it's job and doesn't blow up in your face!

<li <%= ('class="current"') if controller.controller_name == 'users' && controller.action_name == 'login' %>>

First, move "if controller.controller_name == 'users' && controller.action_name == 'login'" into a helper and give it a name, such as login_page_class. It should return nil on the wrong page and 'current' on the right page.

Next, use content_tag to build the entire li. Then if :class => login_page_class contains the nil, the class="" itself will go away in the rendered HTML.

(And put all this under test, because it's logic that you want to keep alive as you upgrade this site!)