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

Fix and cleanup output handlers #1441

Merged
merged 10 commits into from Oct 10, 2013
Merged

Fix and cleanup output handlers #1441

merged 10 commits into from Oct 10, 2013

Conversation

ujifgc
Copy link
Member

@ujifgc ujifgc commented Oct 9, 2013

Warning: this disables concatenating of - in Slim and Haml templates #1441 (comment).

@namusyaka
Copy link
Contributor

@ujifgc Thank you!

+1

BTW, do we have to rewrite padrino-admin templates of slim?
I think we should replace - with =. e.g. - form_tag ..

@ujifgc
Copy link
Member Author

ujifgc commented Oct 9, 2013

Yes, padrino-admin pages are broken at this PR.

@namusyaka
Copy link
Contributor

I fixed the broken templates.
Please use it if you like.

@nesquena
Copy link
Member

nesquena commented Oct 9, 2013

Nice job @ujifgc and @namusyaka! really glad to see this cleanup. template handling, especially with capture has been half-broken for a long time now. Good to see this getting solidified for 0.12

@DAddYE
Copy link
Member

DAddYE commented Oct 9, 2013

Indeed awesome work guys. Keep going.

@ujifgc
Copy link
Member Author

ujifgc commented Oct 10, 2013

The last commit 80a0e24 disables concatenating of - in Slim and Haml templates.

If we decide we could implement a flag to enable legacy behavior of Padrino template handlers and concatenate on - (though I would not).

If some legacy app wants to keep wrong behavior, the monkey patch would be:

For HamlHandler:

class Padrino::Helpers::OutputHelpers::HamlHandler
  def concat_to_template(text="")
    template.haml_concat(text)
    nil
  end 
end

For SlimHandler:

class Padrino::Helpers::OutputHelpers::SlimHandler
  def concat_to_template(text="")
    self.output_buffer << text
    nil
  end
end

Please, check your apps with this PR and comment. I think, it's ready.

@ujifgc ujifgc mentioned this pull request Oct 10, 2013
@nesquena
Copy link
Member

If I understand correctly it means almost every padrino app using forms will need to tweak every - to = or their forms will be totally broken. Seems like 0.12.0 is as good a time as any to make this fairly major breaking change. What do you all think @padrino/core-members ?

@ujifgc
Copy link
Member Author

ujifgc commented Oct 10, 2013

Yep. A tweak of template block helpers from - to = or the monkey patch I provided. The monkey patch actually doesn't break anything except the test for concatenating -.

@nesquena
Copy link
Member

Yeah, we should document the monkeypatch in the blog post for 0.12.0 release (#1019)

@dariocravero
Copy link

I think the breaking change makes a lot of sense towards readability and predictability of the code going further. Excellent job guys! :) Thanks so much!!

ujifgc added a commit that referenced this pull request Oct 10, 2013
@ujifgc ujifgc merged commit 8bb8124 into master Oct 10, 2013
@ujifgc ujifgc deleted the fix-output-handlers branch October 10, 2013 20:11
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

5 participants