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

Emit relative path to output #8692

Merged
merged 5 commits into from
Apr 25, 2023

Conversation

rainersigwald
Copy link
Member

A top comment from the folks who've seen this is that the absolute paths take up a bunch of space and making them relative would be a UX nicety. This is the simplest possible approach for that: string prefix truncation with no path comparison anything.

@baronfel
Copy link
Member

Here's a short recording of what it looks like:

fsac-build-shortoutput

It does help a lot over using absolute paths! This is a reasonable sized terminal window (~1100x600) and it was still legible for this small, 7ish project repo.

src/MSBuild/LiveLogger/LiveLogger.cs Outdated Show resolved Hide resolved
src/MSBuild/LiveLogger/LiveLogger.cs Outdated Show resolved Hide resolved

// If the output path is under the initial working directory, make the console output relative to that to save space.
if (outputPathSpan.StartsWith(_initialWorkingDirectory.AsSpan(), FileUtilities.PathComparison)
&& (outputPathSpan[_initialWorkingDirectory.Length] == Path.DirectorySeparatorChar
Copy link
Member

Choose a reason for hiding this comment

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

It's not possible for outputPathSpan to be the same as _initialWorkingDirectory to cause a crash here, is it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hm. Not normally, but we can't control what projects might emit, so we should definitely guard against this.

@rainersigwald rainersigwald added the merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now. label Apr 25, 2023
@rainersigwald rainersigwald enabled auto-merge (squash) April 25, 2023 22:09
@rainersigwald rainersigwald merged commit 093fbaf into dotnet:main Apr 25, 2023
@rainersigwald rainersigwald deleted the livelogger-relative-path branch April 25, 2023 23:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants