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

Diagnostic path log relative to current working dir #4807

Merged
merged 7 commits into from
Nov 4, 2024

Conversation

timotheeguerin
Copy link
Member

@timotheeguerin timotheeguerin commented Oct 21, 2024

To improve #4806 need to cleanup the path resolution

close #2274

Before

image

After

  • From parent folder
    image

  • From same folder
    image

@azure-sdk
Copy link
Collaborator

azure-sdk commented Oct 21, 2024

All changed packages have been documented.

  • @typespec/compiler
  • @typespec/http-server-javascript
  • @typespec/protobuf
Show changes

@typespec/compiler - feature ✏️

CLI logs diagnostic source path relative to the CWD.

@typespec/http-server-javascript - internal ✏️

@typespec/protobuf - internal ✏️

@azure-sdk
Copy link
Collaborator

You can try these changes here

🛝 Playground 🌐 Website 📚 Next docs

Copy link
Contributor

@markcowl markcowl left a comment

Choose a reason for hiding this comment

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

Will this impact tsv logs in the azure-rest-api-specs repo? Can we try this change there to ensure no issues (unsure if there ia any output processing going on)

The change itself looks good and also useful

@timotheeguerin
Copy link
Member Author

I don't think so, @mikeharder is that going to be a problem

@mikeharder
Copy link
Contributor

I don't think so, @mikeharder is that going to be a problem

Should be fine for now, since we just show text output to user, we don't parse it.

If we wanted to get fancy, we could try to integrate TypeSpecValidation more with GitHub, where errors from tsp compile inside TSV could be promoted all the way to a "GitHub Error" with file and line numbers, and would appear in your code review diff. I think I'll open a feature request for this. But again, for now this change is fine.

@timotheeguerin timotheeguerin added this pull request to the merge queue Nov 4, 2024
Merged via the queue into microsoft:main with commit 2d7eecf Nov 4, 2024
22 checks passed
@timotheeguerin timotheeguerin deleted the diag-path-relative branch November 4, 2024 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:core Issues for @typespec/compiler
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Should the cli show the errors with relative path instead of absolute
4 participants