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

Update ConciseView to remove unnecessary text and not color entire line in red #10724

Merged
merged 4 commits into from Oct 8, 2019

Conversation

SteveL-MSFT
Copy link
Member

@SteveL-MSFT SteveL-MSFT commented Oct 7, 2019

PR Summary

Remove unnecessary text before script path, detect if whole line is emphasized, then don't do any emphasis. Change emphasis color to accent color instead of error color to make it easier to read. Also fixed some typos and disabled strict mode in helper that works with different dynamic objects that puts errors in $error if strict mode is enabled.

Before:

img

After:

img

PR Context

Fix #10716

PR Checklist

don't color the whole line red making it harder to read
@iSazonov
Copy link
Collaborator

iSazonov commented Oct 8, 2019

Is it common practice to paint in red only an error token and keep comment in white-on-black?

@SteveL-MSFT
Copy link
Member Author

@iSazonov the problem with that proposal is that some errors don't have a visible token, like foreach ($a 123) is missing in so nothing would be red.

@iSazonov
Copy link
Collaborator

iSazonov commented Oct 8, 2019

'^' could be red in the case.

@rkeithhill
Copy link
Collaborator

Just my opinion but ... some red is good to draw your attention ... too much text in red is hard to read / undesirable. Finding the sweet spot in between .... that's the challenge. :-)

Copy link
Member

@JamesWTruher JamesWTruher left a comment

Choose a reason for hiding this comment

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

nothing really blocking
I should have caught those other errors in the previous, sorry

@anmenaga anmenaga merged commit bce5b20 into PowerShell:master Oct 8, 2019
@iSazonov iSazonov added this to the 7.0.0-preview.5 milestone Oct 9, 2019
@iSazonov iSazonov added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Oct 9, 2019
kilasuit pushed a commit to kilasuit/PowerShell that referenced this pull request Oct 31, 2019
kilasuit pushed a commit to kilasuit/PowerShell that referenced this pull request Nov 9, 2019
@SteveL-MSFT SteveL-MSFT deleted the erroview-update branch June 6, 2020 02:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ConciseView for $ErrorView does not work in StrictMode
5 participants