-
Notifications
You must be signed in to change notification settings - Fork 25
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
Rails 7 1 support #71
base: main
Are you sure you want to change the base?
Conversation
See tests fail with following stack: LayoutsTest#test_no_layout: ActionView::Template::Error: undefined method `valid_encoding?' for an instance of ActionView::OutputBuffer kramdown (2.4.0) lib/kramdown/parser/base.rb:92:in `adapt_source' kramdown (2.4.0) lib/kramdown/parser/kramdown.rb:90:in `parse' kramdown-parser-gfm (1.1.0) lib/kramdown/parser/gfm.rb:56:in `parse' kramdown (2.4.0) lib/kramdown/parser/base.rb:69:in `parse' kramdown (2.4.0) lib/kramdown/document.rb:102:in `initialize' /Users/james/Projects/James/maildown/lib/maildown/markdown_engine.rb:53:in `new' /Users/james/Projects/James/maildown/lib/maildown/markdown_engine.rb:53:in `block in default_html_block' /Users/james/Projects/James/maildown/lib/maildown/markdown_engine.rb:24:in `to_html' app/views/user_no_layout_mailer/welcome.md+erb:1 actionview (7.1.2) lib/action_view/base.rb:264:in `public_send' actionview (7.1.2) lib/action_view/base.rb:264:in `_run' actionview (7.1.2) lib/action_view/template.rb:261:in `block in render'
…string. Several String methods are used, but unsupported on the ActionView::OutputBuffer (for example `valid_encoding?` and `encode`).
Hey @schneems, any chance we can get this looked and merged in, if good. We're in the process of upgrading to Rails 7.1 and this is a super useful gem we use to DRY up a tonne of our views. Let me know what I can do to help too. Thanks! |
Handle when `string` is a `ActionView::OutputBuffer` by converting it to a `String` first. Pulled from zombocom#71.
Handle when `string` is a `ActionView::OutputBuffer` by converting it to a `String` first. Pulled from zombocom#71.
Handle when `string` is a `ActionView::OutputBuffer` by converting it to a `String` first. Pulled from zombocom#71.
Adds and addresses the following: * Adds Rails 7.1 to test matrix * Addresses expected String type when rendering (zombocom#71)
would love this merged! |
I too would like to see this merged in. @cowlibob If I may, there is one suggestion I'd like to make for this PR. I think we just need to ensure only strings are passed into |
Rails 7.1 appears to have changed what is passed into the
MailDown::MarkdownEngine#to_html
. What used to be aString
(or String-like object) is now anActionView::OutputBuffer
.ActionView::OutputBuffer
does not respond toencode
,valid_encoding?
and other String methods called inside Kramdown.The simplest fix I could muster is to convert to a
String
before use. Do feel free to consider other solutions though, if you feel it appropriate.Using the gemfiles/rails_7_1_gemfile and it's lock without the fix (commit b520fae) will show the following (truncated) output during tests:
Running the same test suit on (commit 2347d47) will pass.