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

Add the compact mode #125

Merged
merged 1 commit into from
Sep 17, 2014
Merged

Add the compact mode #125

merged 1 commit into from
Sep 17, 2014

Conversation

jalkoby
Copy link

@jalkoby jalkoby commented Sep 10, 2014

Add the compact option which strips useless _path suffixes in path helper functions. It should be useful in a situations when a "Routes" object has only path functions, so instead repeat again and again

Routes.users_path()
// it will become
Routes.users()

@le0pard
Copy link
Member

le0pard commented Sep 10, 2014

Hello, @jalkoby I don't know how right now, but this changes broke tests with rails 3.2 (looks like segmentation fault) :)

return "" + #{@options[:url_links].inspect} + this.#{route_name}(#{build_params(required_parts)});
}
JS
end

def generate_route_name(name, is_url = false)
route_name = "#{name.join('_')}_#{is_url ? "url" : "path"}"
def generate_route_name(name, suffix)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in this case better add for suffix default value nil

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need a default value, if all usages of this methods pass all arguments?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just be sure about (:path unless @options[:compact]). But you can leave as is. Just need fix problem with rails 3.2.

@bogdan
Copy link
Collaborator

bogdan commented Sep 11, 2014

I'd not use this option personally: the ability to grep particular route and see its usages is more important to me than avoid "useless" _path.

So, On your place I would think twice before turning it on for your app.

@jalkoby
Copy link
Author

jalkoby commented Sep 11, 2014

@bogdan I don't see any potential problem to find a places where some route helper function is used because all route helper functions are stored into Router object. So, grep "Router.someInvalidRoute" will be enough for 95% use-cases.You see, Rails adds "_path" and "_url" suffix only because it has a lot of other helpers("_tag", "options", "number" families) and it's really hard to determine what is a responsibility of a helper method. But helper methods which are located into an object with a single responsibility don't need useless suffix(for example, rails form builder which has methods text_area, text_field, select, but not a text_area_tag, text_field_tag, etc)

@bogdan
Copy link
Collaborator

bogdan commented Sep 11, 2014

Ok, I am fine with this option. Lets fix the build and we will merge it.

@jalkoby
Copy link
Author

jalkoby commented Sep 11, 2014

The problem in rails 3.2 env is related with errors in a C code. After a short review it seems libv8 was updated from time of the last stable build, so I've locked libv8 gem for 3.2 env

@le0pard
Copy link
Member

le0pard commented Sep 11, 2014

@jalkoby need to fix this for jruby:

platforms :ruby do
  gem "libv8", "<= 3.16.14.3"
end

@le0pard
Copy link
Member

le0pard commented Sep 11, 2014

@jalkoby All looks good. Just need add some descriptions about new option in README.

@jalkoby
Copy link
Author

jalkoby commented Sep 15, 2014

Sorry guys for the delay. I've updated Readme and combined all changes into single commit

@@ -64,6 +64,9 @@ Available options:
* `url_links` (version >= 0.8.9) - Generate `*_url` links (in addition to default `*_path`), where url_links value is beginning of url routes
* Example: http[s]://example.com
* Default: false
* `compact` (version > 0.9.9) - Remove `_path` suffix in path routes(`*_url` routes stay untouched if they were enabled)
* Example: Routes.users() => `/users`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the example fo previous option Example is used a example value of an option. So I would using example here.
And write something like: Sample route call when option is set to true:.

In the compact mode path routes generated without `_path` suffix
bogdan added a commit that referenced this pull request Sep 17, 2014
@bogdan bogdan merged commit b2d9263 into railsware:master Sep 17, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants