Asciidoctor: Handle snippets in definition lists

[nik9000]

Our lang override processor had trouble with snippets inside of
definition lists because a method provided by Asciidoctor doesn't quite
behave well inside definition lists:
asciidoctor/asciidoctor#3133

That method had behavior that is a little too broad anyway so we
implement what we need directly.

[nik9000]

@estolfo could you have a look at this one? It is 100% ruby and small enough that I think we can iterate on some best practices things here. This in particular makes me like the whole "early return" thing so we don't end up with a ton of indentation.

I know this has issues with the magic perl variables, but I think it'd be best to clean those up separately.

[estolfo]

I don't have any major comments, just some minor suggestions, one of which might be out of the scope of this PR.

[estolfo]

I believe it would be more idiomatic to write return unless next_block_index

[nik9000]

When I wrote it I felt like "This can only be nil or a string so I should be explicit" but I've come around to your way.

[estolfo]

I think I mentioned this in the last PR I reviewed, maybe eventually you can use context and let blocks instead in the specs.
For example, this case would have a context when the snippet is inside a definition list and then the it block would be generates the correct string (or whatever). The actual and expected variables would be let blocks.
But I think that change would be part of a larger rspec refactor and perhaps out of the scope of this PR.

This is an example of how context blocks and let blocks can be used.

[nik9000]

@estolfo, I pushed a change for the nil? thing and a change to use context, let, and subject for that section. If it looks right to you I can do the other sections as I get there.

[estolfo]

I added some rspec comments : )

[estolfo]

This is a good step in the direction of idiomatic Rspec : )

I always favor let instead of subject because it's named. The benefit of this, and of using let in general, is so that you can write embedded tests that reuse the "variables". (I put that in quotes because it's actually a method that creates the memoized variable on the fly, when it's called)

So you could write this as

let(:converted) do
  convert <<~ASCIIDOC
        == Example
        Term::
        Definition
        +
        --
        [source,js]
        ----
        GET /
        ----
        --
      ASCIIDOC
end

let(:regexp) do
   %r{<programlisting language="js" linenumbering="unnumbered">GET /</programlisting>}
end

it "converts the string" do
  expect(converted).to match(regexp)
end

Then, if you wanted to write another test using that converted string, you could easily add something like this below the first it block:

it "doesn't match pizza" do
  expect(converted).not_to match(/pizza/)
end

and you can also nest context blocks that reuse the converted string:

let(:converted) do
  convert <<~ASCIIDOC
        == Example
        Term::
        Definition
        +
        --
        [source,js]
        ----
        GET /
        ----
        --
      ASCIIDOC
end

let(:regexp) do
   %r{<programlisting language="js" linenumbering="unnumbered">GET /</programlisting>}
end

it "converts the string" do
  expect(converted).to match(regexp)
end

context "when it's raining" do

  let(:regexp) do
    # And you can override any "variables" you need to!
    /sun/
  end

  it "converts the string" do
    expect(converted).not_to match(regexp)
  end
end

The converted string will only ever be created once, even when you use it repeatedly in tests.

[nik9000]

@estolfo, I pushed some changes to the rspec to line up a bit better with what you proposed.

[estolfo]

Great! The rspec looks good now.