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

Added content_for? check, no tests though #662

Closed
wants to merge 2 commits into from

Conversation

mlightner
Copy link
Contributor

No description provided.

@DAddYE
Copy link
Member

DAddYE commented Sep 8, 2011

Sounds much better now! Great feature, try to add almost one spec and we will merge soon.

Thanks man!

@nesquena
Copy link
Member

nesquena commented Sep 8, 2011

Yea this is a good idea, but we need a spec before merge.

@nesquena
Copy link
Member

nesquena commented Sep 8, 2011

Also couldn't we just use blocks.present?

@ujifgc
Copy link
Member

ujifgc commented Sep 10, 2011

Look at this code! It's beautiful!

def content_for?(key)
  content_blocks[key.to_sym].try :any?
end

@nesquena
Copy link
Member

@ujifgc That works too, I am just curious why one would use that over content_blocks[key.to_sym].present? both require activesupport extensions.

@nesquena
Copy link
Member

But .try is not part of ruby... http://rubydoc.info/docs/rails/Object:try or at least not every version

@nesquena
Copy link
Member

Merged with tests for all three major template systems in the above commit. Thanks!

@nesquena nesquena closed this Sep 10, 2011
@ujifgc
Copy link
Member

ujifgc commented Sep 10, 2011

@nesquena oh, good to know, thanks.

@nesquena
Copy link
Member

Sure, thanks for your input and thanks to @mlightner for the patch.

@mlightner
Copy link
Contributor Author

Thanks to all!

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.

None yet

4 participants