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

ViewScope#add refactor #174

Closed
wants to merge 6 commits into from
Closed

ViewScope#add refactor #174

wants to merge 6 commits into from

Conversation

vinniefranco
Copy link
Contributor

Cleans up large conditional by normalizing internal API a bit.

@ryanstout
Copy link
Member

I feel like this would break something like: {{ items.each do |something| }} (with extra spaces) Maybe I'm missing something (or our tests are missing something)

@vinniefranco
Copy link
Contributor Author

This wouldn't break no matter the amount of white space. The test is /each(_with_index)?/ so {{ items.each do |something| }} is A-OK.

@ryanstout
Copy link
Member

Sorry if this is nitpicky. But currently if you did something like .each_pair, it would give you a compile warning (I think), since we don't have an each_pair binding (yet). Also, to be honest this code is a bit harder to follow, imho. What's your reason for wanting to change this around?

@vinniefranco
Copy link
Contributor Author

Not nit picky at all.

I can change it to require at least one bit of white space... that would
solve it. This PR was just start the break up of that enormous conditional

On Tuesday, May 5, 2015, Ryan Stout notifications@github.com wrote:

Sorry if this is nitpicky. But currently if you did something like
.each_pair, it would give you a compile warning (I think), since we don't
have an each_pair binding (yet). Also, to be honest this code is a bit
harder to follow, imho. What's your reason for wanting to change this
around?


Reply to this email directly or view it on GitHub
#174 (comment).

Vincent Franco

"Any intelligent fool can make things bigger and more complex... It takes a
touch of genius --- and a lot of courage to move in the opposite
direction." -- Einstein

@vinniefranco
Copy link
Contributor Author

Too much has changed. I'm considering this stale.

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

2 participants