Support inline tags

[Xanderwot]

Based on issue

Adding support of inline tag declaration
Examples:

h1 'Hello'

will render:

<h1>Hello</h1>

div do
  h1 'Hello', class: 'title'
end

will render:

<div><h1 class="title">Hello</h1></div>

[joeldrapper]

I think we should probably do something more like this:

case content
when Proc
  yield_content(&content)
when String
  plain(content)
else
  raise ArgumentError
end

In the case that we're given a string, we can completely avoid the block which should mean it’s faster.

[Xanderwot]

Good catch.
I moved processing method to SGML class. I hope it's ok.
Tests also improved

[Xanderwot]

Pls let me know if I need to update documentation somewhere. Thanks

[joeldrapper]

I like this change and it’s non-breaking. We’ve never come up with anything else to use the positional argument for so I think this makes sense. I’ve pinged @willcosgrove for his thoughts though as this is a huge change and one that would be difficult to reverse.

[willcosgrove]

@joeldrapper convinced me that this is a good change 😄

I think we should not allow nesting using this though.

h1(em("Hello")) # 💥

And I think wrapping it in a proc (while it does fix the issue of the elements not getting added to the buffer at the right time) is not something we should support.

[Xanderwot]

@joeldrapper convinced me that this is a good change 😄

I think we should not allow nesting using this though.

h1(em("Hello")) # 💥

And I think wrapping it in a proc (while it does fix the issue of the elements not getting added to the buffer at the right time) is not something we should support.

It's actually pretty specific cases what I faced when tests inline tags in my application. I doubt what it frequently used(h1(em("Hello")) I mean). Anyway proc wrapping is not a main idea. Idea of inline tags to make page structure little bit easier in some cases and not to use it in some crazy cases like huge inline tags sequence.
I did include this specific to a tests just to show how it can be solved because of ruby specific but using it delegates to a developers(if they know what they doing)
It's yours decision anyway I'm just suggesting small improvement to you(I hope so) maybe it will be useful for someone.
Actually it comes from our migration from fortitude which supports this inline tags
Main idea not to create extra Proc objects for tables for example:

table do
  tr do
    th 'Name'
    th 'Age' 
  end
  @users.each do |user|
    tr do
      td user.name
      td user.age
    end
  end
end

[joeldrapper]

Okay final feedback on this:

Let's make sure we raise if you use a positional argument and a block, e.g.

h1("Hello") { "World" }

Let’s make sure we raise if you pass nil.

h1(h2("Hi"))

h2 here returns nil so the h1 call should raise. The way we can detect this is by setting the default to a different “null object” that we can specifically check for.

We won’t be able to use nil as the default (content = nil). Instead, let’s define a Phlex::Null and use that as the default.

module Phlex
  Null = Object.new.freeze
end

Then we can check against that

  if Phlex::Null != content
    ...

Let’s also remove the Proc option as @willcosgrove mentioned at least for now. Adding support for this later is non-breaking but we can’t remove it once we add it.

Finally, while extracting methods is generally good practice, I expect we will get better performance if we just inline the stuff in process_inline_content even though there will be some duplication between standard and void elements. This will save a method call for every time we render an element with positional content, which might be thousands of times per page.

[Xanderwot]

@joeldrapper Pls take a look ;)