-
Notifications
You must be signed in to change notification settings - Fork 9.2k
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
App error style tweaks, Git output formatting #8964
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We wrap most if not all of the Git errors in ErrorWithMetaData instances now
Auto will be inferred to none since we have a global rule encompassing all elements
outofambit
approved these changes
Feb 6, 2020
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yay! excited to have these fixes in!
This was referenced Jun 5, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #7606
Description
Stumbled upon this as I was investigating #8962 and extracted it from that PR since it's not a regression.
Back in #1107 we added support for detecting when a presented error contained raw output from Git and if it was we used a monospace font. This has since regressed due to us wrapping the majority of our errors in
ErrorWithMetadata
instances thus thwarting the logic from #1107.In addition to fixing that logic I also made it so that when showing Git errors we automatically scroll to the end of the Git output. This is useful because that's almost always where the problem is (Git normally terminates when encountering an error) and because Git outputs can be very long (especially in the case of network operations where progress can take up a substantial amount of the output).
Finally I saw that we were using
cursor: text
which made one believe it was possible to select text in the dialog but due to us usinguser-select: auto
that wasn't actually the case since theuser-select: none
got inherited from our global styles.Screenshots
Before
After
Release notes
Notes:
[Fixed] Text in error dialogs is now selectable
[Improved] Errors containing raw Git output are now formatted using fixed-width font