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 SDoc::Renderer#inline #314

Merged
merged 2 commits into from
Oct 5, 2023
Merged

Conversation

jonathanhefner
Copy link
Member

@jonathanhefner jonathanhefner commented Sep 28, 2023

SDoc::Renderer#inline renders a template to the current output buffer instead of as a separate string. It also allows a block to be specified so that the template can yield to perform interleaved rendering. For example:

<% inline "_partial.erb" do %>
  content
<% end %>
<%# _partial.erb %>
<div><% yield %></div>

Renders:

<div>
  content
</div>

@jonathanhefner jonathanhefner force-pushed the renderer-inline branch 2 times, most recently from 80a5111 to a1b88c2 Compare September 28, 2023 21:19
@jonathanhefner
Copy link
Member Author

The TruffleRuby tests are currently failing due to (I believe) a bug in TruffleRuby. I'm not sure if there is a workaround. Rails does not officially support TruffleRuby, so I am inclined to say that SDoc should not be bound by TruffleRuby.

@jonathanhefner
Copy link
Member Author

To work with TruffleRuby, I have (1) given an explicit name to the block that Renderer#inline receives so that binding at least captures that, and (2) favored block.call over yield for #inline tests, and skipped testing yield altogether when TruffleRuby is active.

Requiring an explicitly named block is not ideal. From an API perspective, having the name as a part of the API is less clean than an anonymous yield. And from a testing perspective, we don't have 100% test coverage of our views, so if yield does get used in a view, it may break TruffleRuby but CI will still pass.

@eregon I see that you added TruffleRuby back to the testing matrix in #202. I don't see any discussion about why it was removed in #199, nor why added it was added back in #202. Is it just to act as a "canary" for TruffleRuby (a la https://github.com/orgs/community/discussions/15452)? Also, is this behavior a known issue for TruffleRuby?

@eregon
Copy link
Contributor

eregon commented Oct 3, 2023

Could you file an issue at https://github.com/oracle/truffleruby/issues with that bug/incompatibility in TruffleRuby?
I do not understand what is the issue from the comment in the PR:

 # There is currently a bug in TruffleRuby wherein `binding` does not
 # capture the block that `yield` will call.

It would very helpful if you can provide a small reproducer.

Re TruffleRuby in CI, I added it back because it was removed due to failing, and we fixed the issue so it's passing again.
It seems TruffleRuby was added in CI in #157.
I suspect it might be useful because sdoc might be a dependency in the Gemfile and might be used outside of generating Rails docs. If it's only ever used for generating Rails docs indeed it might make sense to support CRuby and nothing else then.

There is currently a bug in TruffleRuby ([truffleruby#3285][]) wherein
`Kernel#binding` does not capture a given anonymous block, which breaks
`yield` calls evaluated using that binding.  This bug is preventing us
from extending our rendering API.

Furthermore, this bug was caught by a test unit for said API extension,
but our views themselves aren't tested by CI.  So if such a `yield` call
were added to a view, CI would still pass but the `sdoc` command would
be broken when using TruffleRuby.

Also, Rails itself does not officially support TruffleRuby.

Considering all of the above, this commit removes TruffleRuby from CI.

[truffleruby#3285]: oracle/truffleruby#3285
`SDoc::Renderer#inline` renders a template to the current output buffer
instead of as a separate string.  It also allows a block to be specified
so that the template can `yield` to perform interleaved rendering.  For
example:

  ```erb
  <% inline "_partial.erb" do %>
    content
  <% end %>
  ```

  ```erb
  <%# _partial.erb %>
  <div><% yield %></div>
  ```

Renders:

  ```html
  <div>
    content
  </div>
  ```
@jonathanhefner
Copy link
Member Author

I have filed oracle/truffleruby#3285. For now, I will remove TruffleRuby from CI. Perhaps we can add it back after GitHub implements something like https://github.com/orgs/community/discussions/15452.

@jonathanhefner jonathanhefner merged commit 8e3384e into rails:main Oct 5, 2023
8 checks passed
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.

2 participants