Skip to content

Fix extra space in fragment#1064

Merged
ematipico merged 4 commits intowithastro:mainfrom
Trombach:fix-extra-space-in-fragment
Feb 6, 2025
Merged

Fix extra space in fragment#1064
ematipico merged 4 commits intowithastro:mainfrom
Trombach:fix-extra-space-in-fragment

Conversation

@Trombach
Copy link
Copy Markdown
Contributor

@Trombach Trombach commented Feb 4, 2025

Changes

  • closes Fragments are compiled with a space, which fails in TypeScript 5.7 #1053
  • issue was caused by a line break token immediately before the fragment (repro), which caused hasLeadingSpace to be true
  • removing the - 1 in line 709 fixes the issue, however I could not find any reason why it was subtracting 1 in the first place. Possible I've missed something
  • doing this doesn't seem to break any other test cases, but fixes the problem

Testing

  • added test case to check that fragments are printed properly if they are preceded by a line break

Docs

bug fix only

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Feb 4, 2025 •
edited
Loading

🦋 Changeset detected

Latest commit: ab31d4c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@astrojs/compiler Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@ematipico
Copy link
Copy Markdown
Member

ematipico commented Feb 5, 2025

@Trombach

This is PR related to -1 https://github.com/withastro/compiler/pull/783/files

@ematipico
Copy link
Copy Markdown
Member

ematipico commented Feb 5, 2025 •
edited
Loading

If all tests pass, I believe we didn't break existing code. Also, please add a changeset to track the bug fix :)

@Trombach
Copy link
Copy Markdown
Contributor Author

Trombach commented Feb 5, 2025

@ematipico I think that one just fixed a crash when endLoc was negative, but the code to set endLoc = n.Loc[0].Start + len(n.Data) - 1 has been there since the initial commit, so I don't know the reasoning for it. Here the value gets initiales to n.Loc[0].Start + len(n.Data), but I don't understand why it would need a -1 in the other line. It's very possible I'm missing something 🤷

@Trombach
Copy link
Copy Markdown
Contributor Author

Trombach commented Feb 5, 2025

Sorry, I only saw your second message after I wrote my response :) I've added a changeset, hope the wording is acceptable

@ematipico
Copy link
Copy Markdown
Member

ematipico commented Feb 6, 2025

All tests pass, so I believe your fix is correct. Worst case scenario can revert it, so it's not a huge deal. Thank you for taking the time to fix the problem

ematipico
ematipico reviewed Feb 6, 2025
View reviewed changes
@ematipico ematipico merged commit 6b6a134 into withastro:main Feb 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ematipico ematipico ematipico approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Fragments are compiled with a space, which fails in TypeScript 5.7

2 participants

@Trombach @ematipico