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 column width to TableCell nodes #296

Merged
merged 2 commits into from
Mar 9, 2024
Merged

add column width to TableCell nodes #296

merged 2 commits into from
Mar 9, 2024

Conversation

dwyand
Copy link
Contributor

@dwyand dwyand commented Jul 14, 2023

Adds an extra property for column with to TableCell nodes.

The width is determined by the number of dash and colon characters in the separator line of a table, and passed on to the consumer as an integer. This permits consumers of the AST to render tables that are more proportional the width in the source markdown.

No changes have been made to the HTML render to make use of the width information, but the test for this feature demonstrates a possible usage.

Should a cell be created that is further to the right than the parsed separator columns are (i.e., when alignment would be null), then the width will be reported as 0.

The width is determined by the number of dash and colon characters in the
separator line of a table, and passed on to the consumer as an integer.
This permits consumers of the AST to render tables that are more
proportional the width in the source markdown.

No changes have been made to the HTML render to make use of the width
information, but the test for this feature demonstrates a possible usage.

Should a cell be be created that is further to the right than the parsed
separator columns are (i.e., when alignment would be null), then the width
will be reported as 0.
@dwyand dwyand marked this pull request as draft July 17, 2023 18:05
@dwyand dwyand marked this pull request as ready for review July 17, 2023 19:37
@codecov-commenter
Copy link

codecov-commenter commented Jul 23, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.75%. Comparing base (dfd95d0) to head (b134389).
Report is 73 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main     #296      +/-   ##
============================================
+ Coverage     94.28%   94.75%   +0.46%     
- Complexity      227      254      +27     
============================================
  Files           124      131       +7     
  Lines          3625     4135     +510     
  Branches        549      604      +55     
============================================
+ Hits           3418     3918     +500     
- Misses          104      108       +4     
- Partials        103      109       +6     
Files Coverage Δ
.../java/org/commonmark/ext/gfm/tables/TableCell.java 100.00% <100.00%> (ø)
...mark/ext/gfm/tables/internal/TableBlockParser.java 98.79% <100.00%> (+0.10%) ⬆️

... and 38 files with indirect coverage changes

Copy link
Collaborator

@robinst robinst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one comment, otherwise looks good to merge!

@@ -32,6 +33,17 @@ public void setAlignment(Alignment alignment) {
this.alignment = alignment;
}

/**
* @return the cell width
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a description of what this mean here, similar to the PR description?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've done that now..

@robinst robinst merged commit 6385869 into commonmark:main Mar 9, 2024
7 checks passed
@robinst
Copy link
Collaborator

robinst commented Mar 15, 2024

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.

3 participants