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

Fixes #1265. Support setting column widths #1270

Merged
merged 4 commits into from
Mar 31, 2024

Conversation

robertpanzer
Copy link
Member

Thank you for opening a pull request and contributing to AsciidoctorJ!

Please take a bit of time giving some details about your pull request:

Kind of change

  • Bug fix
  • New non-breaking feature
  • New breaking feature
  • Documentation update
  • Build improvement

Description

What is the goal of this pull request?

Add support for setting the width of table columns.
While columns have a method setWidth() that sets the attribute width of a column, this does not become effective.
In addition it is required to set the attribute colpcwidth, which Asciidoctor core only sets internally when calling Table#assign_column_widths.

How does it achieve that?

This PR adds a method Table.assignColumnWidths which calls the Ruby method mentioned above.

Are there any alternative ways to implement this?

Very likely.

Are there any implications of this pull request? Anything a user must know?

There should be no change of existing behavior.

Issue

If this PR fixes an open issue, please add a line of the form:

Fixes #1265

This PR is a draft until more tests, in particular for the auto width_cols are added.

Release notes

Please add a corresponding entry to the file CHANGELOG.adoc

@robertpanzer
Copy link
Member Author

I added a test that uses autowidth columns, where the width attribute has a negative value.
Together with that I was considering adding an overloaded implementation for assignColumnWidths that allows to pass the widthBase.
Testing this I realized that I do not understand properly how this works, since the assigned widths were just the absolute widths set on the columns.
I.e., if I set the widths to 2, 3 and -1 for 3 columns, even with a width base of 10 the widths in the resulting html were 2, 3 and not set.
Therefore, I kept it like it is now, with only one implementation for assignColumnWidths.

@robertpanzer robertpanzer marked this pull request as ready for review March 24, 2024 20:03
@robertpanzer robertpanzer merged commit b669438 into asciidoctor:main Mar 31, 2024
12 checks passed
@robertpanzer robertpanzer deleted the support-column-widths branch March 31, 2024 08:35
@robertpanzer
Copy link
Member Author

I can backport this to 2.5.x.

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.

Column#setWidth is ignored
1 participant