Default table alignment from theme

[dalemartin]

Currently we have

image:
  align: left | center | right
caption:
  align: left | center | right

Proposing a matching key for tables:

table:
  align: left | center | right

Should be able to override using the current align="left | center | right" property for individual placement.

[mojavelinux]

That seems reasonable to me. Here's where that would go in the code base: /~https://github.com/asciidoctor/asciidoctor-pdf/blob/master/lib/asciidoctor-pdf/converter.rb#L1959

Would you be interested in coding the change? It will need to be accompanied by a test (though with ~350 tests, you should be able to steal one that's already there).

[habamax]

is smth like

    else
       alignment = theme.table.align.to_sym

probably it should be checked if theme has align value:

    else
       alignment = (theme.table.align || :left).to_sym

or even better to add default :left to the base/default theme?.

[mojavelinux]

(theme.table_align || :left).to_sym

or even better to add default :left to the base/default theme?.

you still need the fallback since it has to be defined, so I'd just leave it this way in this case.

[habamax]

Should rspec be used to run tests?

[mojavelinux]

See /~https://github.com/asciidoctor/asciidoctor-pdf#run-the-tests

[habamax]

See /~https://github.com/asciidoctor/asciidoctor-pdf#run-the-tests

Thx. I have made this small change, now I struggle with the test to cover those positions.

Is it correct that I have to generate (manually?) reference pdf file and then to compare the result with it?

    it 'should allow theme to customize default left alignment of table ', integration: true do
      theme_overrides = {
        table_align: left,
      }
      to_file = to_pdf_file <<~'EOS', 'table-align-left.pdf'
      [cols=3*,width=50%]
      |===
      |A1 |B1 |C1
      |A2 |B2 |C2
      |A3 |B3 |C3
      |===
      EOS

      (expect to_file).to visually_match 'table-align-left.pdf'
    end

How do I generate reference pdf?

For now I have just manually took the same src as in test and generated the PDF with default settings, put it to spec/reference and run the table tests... and they have failed :)

Apparently I do smth wrong. BTW, is there a way to see what test generates? The PDF output? I would just visually confirmed that it is correct and put it into reference directory :)

[habamax]

ahh, DEBUG=true bundle exec rspec -t integration... which doesn't work on windows out of the box

[mojavelinux]

In this case, I think you could get away with doing an analyze test. Just put two tables on the page, and ensure that the one that has align=left is more left than the one that has no align (because it will read "right" from the theme perhaps). We just need to make sure that it honors the preference. A visual test is really for something where the only way to be sure is to look at the picture.

[mojavelinux]

(and you check the position by looking for the :x position of the text in the cell).

[habamax]

I have tried so many variants all of them do not work :(

Could you look into the last one, where do I have mistakes:

  context 'Table alignment' do
    it 'should allow theme to customize default alignment of table ' do
      pdf = to_pdf_file <<~'EOS', pdf_theme: { table_align: 'right' }, analyze: :line
      [cols=3*,width=50%]
      |===
      |RIGHT |B1 |C1
      |A2 |B2 |C2
      |A3 |B3 |C3
      |===

      [cols=3*,width=50%,align=left]
      |===
      |LEFT |B1 |C1
      |A2 |B2 |C2
      |A3 |B3 |C3
      |===
      EOS

      cell_right = (pdf.find_text 'RIGHT')[0]
      cell_left = (pdf.find_text 'LEFT')[0]

      (expect cell_right[:x]).to be > cell_left[:x]
    end
  end

[habamax]

Well of course it should be to_pdf :)

[habamax]

@mojavelinux this is the least I can do for you and asciidoctor-pdf :)

[dalemartin]

Would you be interested in coding the change? It will need to be accompanied by a test (though with ~350 tests, you should be able to steal one that's already there).

I would be interested! Unfortunately I can't do this during my regular work hours, and I'm not sure when I'll be able to get to it. Also I have zero Ruby experience, kind of daunted by the task, but willing to learn for sure. Not sure how the tests work either... assuming unit tests written in Ruby?

[dalemartin]

@mojavelinux this is the least I can do for you and asciidoctor-pdf :)

Thanks @habamax for stepping in here! And @mojavelinux for all you do for this project (goes without saying!).

[mojavelinux]

Thanks @habamax!

I'll take a look at your test and get back to you.

[mojavelinux]

@dalemartin For the record, I can't do this during my regular works either and haven't been able to (for the most part) for 10 years. I make time.

Also I have zero Ruby experience...but willing to learn for sure.

This is a safe space for learning. So I'm glad to hear you are willing to put down your fear.

I had zero Ruby experience when I sent my first two dozen patches to Asciidoctor. It doesn't matter. We're software developers, not "insert language here" developers. If you want change, you need to participate. That's how OSS works.

[mojavelinux]

Great job @habamax. You knocked the ⚾ out of the park!