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(graphical): prevent leading newline when no link/code #418

Merged
merged 1 commit into from
Jan 14, 2025

Conversation

unbyte
Copy link
Contributor

@unbyte unbyte commented Jan 8, 2025

#358 introduced a new behavior: if a diagnostic has no code, the rendered report will contain a leading newline.

I'm not sure if this was an intentional change or an unintended side effect, but as a long-time miette user, I wouldn't expect my rendered report to include an extra leading newline - whether I'm outputting to the terminal, displaying in web page, or running snapshot tests. So I opened this PR.

Please let me know if I've misunderstood anything.

@zanieb
Copy link
Contributor

zanieb commented Jan 13, 2025

I believe this bug is the cause of the regression blocking our upgrade in astral-sh/uv#9568 — many snapshots changed :)

@zkat
Copy link
Owner

zkat commented Jan 14, 2025

@zanieb do you think you could run your test suite against this PR and see if it does actually fix the issue? It does seem to be the same thing though but I want to avoid thrash if possible

Copy link
Owner

@zkat zkat left a comment

Choose a reason for hiding this comment

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

I wanna make sure this fixes uv's upgrade but everything looks good! Thank you for taking the time to do this. Sorry it took a while to get to this :)

@zanieb
Copy link
Contributor

zanieb commented Jan 14, 2025

Yep this looks good to me astral-sh/uv#10608

@zkat
Copy link
Owner

zkat commented Jan 14, 2025

@zanieb thanks for taking a look!

@zkat zkat merged commit 1e1938a into zkat:main Jan 14, 2025
6 of 15 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.

3 participants