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

Fix extra space in fragment #1064

Merged
merged 4 commits into from
Feb 6, 2025

Conversation

Trombach
Copy link
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

Copy link

changeset-bot bot commented Feb 4, 2025

🦋 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
Member

@Trombach

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

@ematipico
Copy link
Member

ematipico commented Feb 5, 2025

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
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
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
Member

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 merged commit 6b6a134 into withastro:main Feb 6, 2025
4 of 5 checks passed
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.

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