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

Account for inline boxes when collapsing after newlines #280

Merged
merged 3 commits into from
Feb 20, 2025

Conversation

nicoburns
Copy link
Contributor

Followup (small bug fix) to #254 which didn't take inline boxes into account. That PR was checking whether "the last character is whitespace" without checking whether the last thing pushed into the layout was text.

So if the sequence of items pushed to a layout was something like:

  • inline box
  • whitespace
  • inline box
  • whitespace
  • inline box
  • whitespace
  • inline box

(an example which would be "status badges" in Github readme's which are typically images separated by whitespace characters)

Then all but the first "whitespace" would get collapsed as it would be incorrectly detected as directly adjacent to the preceding whitespace (even though it shouldn't because there is an inline box in between).

This PR fixes this issue by tracking the kind of the last item pushed.

@nicoburns nicoburns added the bug Something isn't working label Feb 17, 2025
@nicoburns nicoburns added this to the 0.3 Release milestone Feb 17, 2025
@nicoburns nicoburns requested review from wfdewith and tomcur February 17, 2025 22:49
@nicoburns
Copy link
Contributor Author

Hmm.. the windows "restore lfs cache" task is generating a different cache key to linux/mac and is then not finding the cached LFS files.

@nicoburns nicoburns force-pushed the whitespace-collapsing-fix branch from f51d789 to 5cdaa45 Compare February 18, 2025 10:27
Signed-off-by: Nico Burns <[email protected]>
@nicoburns nicoburns force-pushed the whitespace-collapsing-fix branch from 5cdaa45 to 5e7b1fb Compare February 18, 2025 10:30
Signed-off-by: Nico Burns <[email protected]>
@DJMcNab
Copy link
Member

DJMcNab commented Feb 18, 2025

@nicoburns nicoburns requested a review from wfdewith February 20, 2025 10:43
Copy link
Contributor

@wfdewith wfdewith left a comment

Choose a reason for hiding this comment

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

It makes sense to me, and is similar in structure to the line breaker. Should we derive Debug on those internal structs?

@nicoburns nicoburns added this pull request to the merge queue Feb 20, 2025
Merged via the queue into linebender:main with commit 7c8983c Feb 20, 2025
21 checks passed
@nicoburns nicoburns deleted the whitespace-collapsing-fix branch February 20, 2025 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants