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

Display preview state and version info in PSIC startup banner #2644

Conversation

rkeithhill
Copy link
Collaborator

@rkeithhill rkeithhill commented Apr 18, 2020

PR Summary

Adds preview status and version info to the PSIC startup banner. The info comes from the extension (not PSES).

Fix #2643

image

PR Checklist

Note: Tick the boxes below that apply to this pull request by putting an x between the square brackets.
Please mark anything not applicable to this PR NA.

  • PR has a meaningful title
  • Summarized changes
  • PR has tests
  • This PR is ready to merge and is not work in progress
    • If the PR is work in progress, please add the prefix WIP: to the beginning of the title and remove the prefix when the PR is ready

@rkeithhill
Copy link
Collaborator Author

Question - I did not adjust the leading whitespace for the additional text. It occurs to me know that maybe that leading whitespace is to make the banner "somewhat" centered. If that was the intent, should I "reduce" the amount of whitespace for the extra characters?

src/session.ts Outdated Show resolved Hide resolved
@TylerLeonhardt
Copy link
Member

I think it would be a good idea to adjust the whitespace a bit. We had some complaints in the past that it's too far to the right anyway...

@rkeithhill
Copy link
Collaborator Author

rkeithhill commented Apr 19, 2020

RE adjusting whitespace. Which of these four do you like? 1 is the original. 2 has four spaces removed from leading whitespace. 3 has no leading whitespace. 4 has no leading whitespace and gets rid of one newline above and one newline below (still leaving one newline above and below).

image

One more, make this #5 (no blank line before, one blank line after):
image

src/session.ts Outdated Show resolved Hide resolved
@TylerLeonhardt
Copy link
Member

I like number 2!

@rkeithhill
Copy link
Collaborator Author

Looking at some other repls:

image

I prefer #5. But I'll go with whatever @TylerLeonhardt and @rjmholt want.

@TylerLeonhardt
Copy link
Member

I'm fine with number 5 too. If we get rid of all the whitespace - no one can complain that there's too much :)

Copy link
Member

@TylerLeonhardt TylerLeonhardt left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@rjmholt rjmholt left a comment

Choose a reason for hiding this comment

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

Love it!

@TylerLeonhardt TylerLeonhardt merged commit 79d7313 into PowerShell:master Apr 20, 2020
@rkeithhill rkeithhill deleted the rkeithhill/display-preview-version-in-psic-banner branch April 20, 2020 22:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Display extension edition & version in PSIC banner
3 participants