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

More empty/spacing fixes #2262

Merged
merged 24 commits into from
Dec 14, 2023
Merged

More empty/spacing fixes #2262

merged 24 commits into from
Dec 14, 2023

Conversation

brucemiller
Copy link
Owner

@brucemiller brucemiller commented Oct 31, 2023

Some tweaks dealing with "emptiness" and spaces. LaTeX tabular (& related) will trim only explicit spaces from the rhs of columns, but not space producing things (like \phantom). IsEmpty no longer considers spaces to be empty, but there's a new IsEmptyOrSpace if that's needed.
Fixes #1999

A WiP as I may sneak in some further fixes.

…NOT a LaTeX name), and to only trim explicit spaces, not spacing commands
…or the content when checking emptiness and related tests
…lignment rows/columns that contain spaces if bordered; prepare for CSS padding to deal with spacing
…; use to determine css; simplify \lx@intercol and its usage
@brucemiller brucemiller changed the title [WiP] More empty/spacing fixes More empty/spacing fixes Nov 30, 2023
@brucemiller
Copy link
Owner Author

A lot of tweaks, tuning, and subtle changes; the result is generally much more accurate spacing & padding on various tabular and \halign constructs.

@brucemiller brucemiller requested a review from dginev November 30, 2023 04:55
@brucemiller
Copy link
Owner Author

OK, that should do it; tikz works again.

Copy link
Collaborator

@dginev dginev left a comment

Choose a reason for hiding this comment

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

Impressive!

I spotted 3 mildly suspicious changes to spacing in some tests, left a comment for consideration.

<td align="left" thead="column"><text font="bold">Entity</text></td>
<td align="left" thead="column"><text font="bold">Unicode Name</text></td>
<td align="left" thead="column"><text font="bold">Unicode</text></td>
<td align="left" thead="column">      <text font="bold">Entity</text></td>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are the added lead-in spaces in {supertabular} cells an intended change?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yep, intended; the spaces account for the "unusual" cell padding in the test cases; spaces were expedient vs some clever css scheme. (comment applies to other cases)

<td align="left" class="ltx_nopad_r">[a]</td>
<td align="left" class="ltx_nopad_r">(b)</td>
<td align="left" class="ltx_nopad_r">                              {c}</td>
<td align="left" class="ltx_nopad_r">                              [d]</td>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hm, this is quite a large amount of new lead-in padding for cells {c} and [d]. Intended?

<td align="left">b</td>
</tr>
<tr>
<td align="left">c –</td>
<td align="left">c   –</td>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wasn't the new space here meant for the other side of the dash?

Copy link
Owner Author

Choose a reason for hiding this comment

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

I'm thinking that one of the spaces is due to an \hfil, which should get taken care of separately/eventually.

return 0 unless ($thing->getDefinition eq $STATE->lookupDefinition(T_BEGIN))
&& IsEmpty($thing->getBody->unlist); } } }
# Sneaky Whatsit property for something (an arg) that stands-in for the whatsit's content.
if (my $body = $thing->getProperty('the_body')) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

the_body isn't a very evocative name. I think we had come up with an alternative that had the word box. Was it content_box ?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Ah, yeah; that's the name we came up with; done!

@brucemiller brucemiller merged commit ec9cb83 into master Dec 14, 2023
26 checks passed
@brucemiller brucemiller deleted the more-empty branch December 14, 2023 18:16
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.

empty tabular rows and columns disappear
2 participants