Skip to content

Conversation

@asukaminato0721
Copy link
Contributor

@asukaminato0721 asukaminato0721 commented Nov 1, 2025

part of #955

Image

pipe to bat, then we have syntax highlight now.

./target/debug/pyrefly check ./tmp.py | bat -l py

@meta-cla meta-cla bot added the cla signed label Nov 1, 2025
@asukaminato0721 asukaminato0721 changed the title make cli overlaod error have newline make cli overload error have newline Nov 1, 2025
@asukaminato0721 asukaminato0721 marked this pull request as ready for review November 2, 2025 17:10
@stroxler stroxler requested a review from rchen152 November 3, 2025 15:41
@rchen152
Copy link
Contributor

rchen152 commented Nov 4, 2025

I'm a little worried about how long this would make the [no-matching-overload] error message. I agree that the line breaks make the individual signatures more readable, but this could easily turn, say, a 10-line message into a 100-line one. And I assume the syntax highlighting doesn't show up unless you do an extra step of piping to bat, so it wouldn't be there by default to help with parsing the output?

I wonder if it would be better to instead implement the other suggestion in #955, hiding unrelated fields. That could make these messages more readable without increasing their length.

Copy link
Contributor

@rchen152 rchen152 left a comment

Choose a reason for hiding this comment

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

Marking this as "Changes requested", since I don't think we can merge it as-is. My main concern is about error message length, but looking more closely at the PR, it also puts the # [closest match] comment in an odd place, so that would need to be fixed as well.

@rchen152 rchen152 removed their assignment Nov 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants