Reduce the amount of startup banner text #16516

rkeithhill · 2021-11-25T05:06:44Z

PR Summary

This PR does not set -nologo as the default, instead it removes the
copyright line, the two "how to get help" lines and two blank lines
around the "how to get help" lines.

The following is the resulting startup banner when there is no update available OR the user has disabled updating checking via the POWERSHELL_UPDATECHECK environment variable:

compare this with the current startup banner:

When there is an update available AND the user has not disabled update checking, the reduced startup banner looks like this:

If we decide that we want to keep the Type 'help' to get help. text, I propose something like this:

BTW here are some other startup banners for comparison:

As you can see, bash and nodejs have no startup banner text at all and Python's is a bit wordy. :-)

PR Context

Addresses #15644

Regarding documentation impact, I think this would only impact images/text blocks where the PowerShell startup banner is shown. But what I'm seeing so far in the online help is images of Windows PowerShell startup banner text. @sdwheeler are there areas of the online help where PowerShell v6/7 startup banner text is shown?

This PR currently leaves the profile timing text as-is.

The PR is meant to give folks a way to test drive a reduced startup banner.

PR Checklist

This PR does not set -nologo as the default, instead it removes the copyright line, the two "how to get help" lines and two blank lines around the "how to get help" lines.

iSazonov · 2021-11-25T08:03:21Z

Convert to draft until PowerShell Committee conclusion.

sdwheeler · 2021-11-25T14:09:51Z

There are 3 articles:

about_pwsh - documents -NoLogo but does not show an example
about_environment_variables - documents $env:POWERSHELL_UPDATECHECK but not really affected by this PR
about_update_notifications - documents how Update Notification works but not really affected by this PR

ghost · 2021-12-14T11:00:44Z

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 15 days. It will be closed if no further activity occurs within 10 days of this comment.

SteveL-MSFT · 2021-12-15T20:56:30Z

@rkeithhill would you mind adding a test?

SteveL-MSFT · 2021-12-15T23:19:17Z

@PowerShell/powershell-committee reviewed this, we are ok taking this PR with just the PowerShell <version> logo and getting user feedback. We still need to follow-up with Microsoft to ensure we are ok without the copyright notice, but that's something we can address later and doesn't block this PR.

rkeithhill · 2021-12-16T01:19:45Z

@SteveL-MSFT I can add look at adding a test. Are there any existing tests for the startup banner text?

src/Microsoft.PowerShell.ConsoleHost/resources/ManagedEntranceStrings.resx

src/Microsoft.PowerShell.ConsoleHost/host/msh/UpdatesNotification.cs

iSazonov · 2021-12-16T03:48:31Z

@rkeithhill We have xUnit tests in test_CommandLineParser.cs. You could add new tests in the file.

iSazonov · 2021-12-16T03:57:49Z

@PowerShell/powershell-committee reviewed this, we are ok taking this PR with just the PowerShell <version> logo and getting user feedback. We still need to follow-up with Microsoft to ensure we are ok without the copyright notice, but that's something we can address later and doesn't block this PR.

There is a related problems with "". I'd ask the team and PowerShell Committee to look #15607 (comment) so that we could continue with #15603.

ghost · 2021-12-24T02:00:48Z

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

rkeithhill · 2021-12-24T19:12:06Z

@SteveL-MSFT I think the best place for tests is in ConsoleHost.Tests.ps1. However, I'm a bit stumped on how to test this. All of the other tests in this file appear to use either -File or -Command in order to run pwsh, execute script, exit and capture the output. In those cases, no startup banner is output. If you run without those parameters, you get the startup banner in the new pwsh session but that session doesn't exit and getting the startup banner text from that session isn't exactly straightforward.

rkeithhill · 2022-01-07T19:31:57Z

FYI, I'm trying to sort out the Linux Pester test failure but the line number the log shows seems odd. Anyway, trying to build & run my tests on WSL/Ubuntu to see if I can repro locally.

rkeithhill · 2022-01-07T23:17:47Z

@iSazonov @adityapatwardhan Any idea why I would be getting this ANSI in my output from the Azure Pipelines / PowerShell-CI-macos & Azure Pipelines / PowerShell-CI-linux tests?

WARNING: pwsh output is: 'PowerShell 7.3.0-preview.2\nPS /home/vsts/work/1/s> 1hexit\n1l'

What's with the 1h, extra newline and final 1l? I suppose I need to disable ANSI for the new pwsh process? Now I'm wanting a -NoAnsi switch to pwsh.exe. Update: ah, this seems to be DECCKM enable/disable.

iSazonov · 2022-01-08T11:53:11Z

@rkeithhill You could set env variable TERM=dumb to disable VT mode.

rkeithhill · 2022-01-08T19:19:23Z

@iSazonov Thanks but I've found something that works. I'm not keen to set the TERM=dumb on the Pester process side. And doing it in the new pwsh process isn't much different than my current fix which is to first execute: $PSStyle.OutputRendering = 'PlainText'.

iSazonov · 2022-01-09T07:12:07Z

TERM=dumb turns off DECCKM too and simplify the test code.

test/powershell/Host/ConsoleHost.Tests.ps1

pull-request-quantifier-deprecated · 2022-01-09T17:46:20Z

This PR has 52 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!

Quantification details

Label      : Small
Size       : +46 -6
Percentile : 20.8%

Total files changed: 4

Change summary by file extension:
.cs : +1 -1
.resx : +2 -5
.ps1 : +43 -0

Change counts above are quantified counts, based on the PullRequestQuantifier customizations.

Why proper sizing of changes matters

Optimal pull request sizes drive a better predictable PR flow as they strike a
balance between between PR complexity and PR review overhead. PRs within the
optimal size (typical small, or medium sized PRs) mean:

Fast and predictable releases to production:
- Optimal size changes are more likely to be reviewed faster with fewer
  iterations.
- Similarity in low PR complexity drives similar review times.
Review quality is likely higher as complexity is lower:
- Bugs are more likely to be detected.
- Code inconsistencies are more likely to be detetcted.
Knowledge sharing is improved within the participants:
- Small portions can be assimilated better.
Better engineering practices are exercised:
- Solving big problems by dividing them in well contained, smaller problems.
- Exercising separation of concerns within the code changes.

What can I do to optimize my changes

Use the PullRequestQuantifier to quantify your PR accurately
- Create a context profile for your repo using the context generator
- Exclude files that are not necessary to be reviewed or do not increase the review complexity. Example: Autogenerated code, docs, project IDE setting files, binaries, etc. Check out the Excluded section from your prquantifier.yaml context profile.
- Understand your typical change complexity, drive towards the desired complexity by adjusting the label mapping in your prquantifier.yaml context profile.
- Only use the labels that matter to you, see context specification to customize your prquantifier.yaml context profile.
Change your engineering behaviors
- For PRs that fall outside of the desired spectrum, review the details and check if:
  - Your PR could be split in smaller, self-contained PRs instead
  - Your PR only solves one particular issue. (For example, don't refactor and code new features in the same PR).

How to interpret the change counts in git diff output

One line was added: +1 -0
One line was deleted: +0 -1
One line was modified: +1 -1 (git diff doesn't know about modified, it will
interpret that line like one addition plus one deletion)
Change percentiles: Change characteristics (addition, deletion, modification)
of this PR in relation to all other PRs within the repository.

Was this comment helpful? 👍 :ok_hand: :thumbsdown: (Email)
Customize PullRequestQuantifier for this repository.

rkeithhill · 2022-01-09T21:46:08Z

TERM=dumb turns off DECCKM too and simplify the test code.

@iSazonov Done. Thanks.

rkeithhill · 2022-01-10T19:51:39Z

@adityapatwardhan This PR is ready to go (merge).

adityapatwardhan · 2022-01-10T22:41:32Z

test/powershell/Host/ConsoleHost.Tests.ps1

+            # Set TERM to "dumb" to avoid DECCKM codes in the output
+            $oldTERM = $env:TERM
+            $env:TERM = "dumb"


Should this be done only on non-Windows?

It probably only needs to be done non-WIndows since the DECCKM failures I saw were on mac/linux. That said, I don't believe it hurts to set TERM="dumb" on Windows.

adityapatwardhan · 2022-01-10T23:08:22Z

@rkeithhill Thank you for your contribution!

ghost · 2022-02-24T23:33:25Z

🎉v7.3.0-preview.2 has been released which incorporates this pull request.:tada:

Handy links:

Release Notes

DHowett · 2022-02-25T21:04:53Z

I just noticed this. Thank you.

Fix PowerShell#15644 - reduce the amount of startup banner text

0cdc4f8

This PR does not set -nologo as the default, instead it removes the copyright line, the two "how to get help" lines and two blank lines around the "how to get help" lines.

rkeithhill requested review from anmenaga and daxian-dbw as code owners November 25, 2021 05:06

pull-request-quantifier-deprecated bot added the Extra Small label Nov 25, 2021

ghost assigned adityapatwardhan Nov 25, 2021

iSazonov marked this pull request as draft November 25, 2021 08:02

rkeithhill changed the title ~~Fix #15644 - reduce the amount of startup banner text~~ WIP: Fix #15644 - reduce the amount of startup banner text Nov 25, 2021

ghost added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Nov 29, 2021

ghost added the Stale label Dec 14, 2021

daxian-dbw added Review - Committee The PR/Issue needs a review from the PowerShell Committee and removed Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept Stale labels Dec 14, 2021

SteveL-MSFT added Committee-Reviewed PS-Committee has reviewed this and made a decision and removed Review - Committee The PR/Issue needs a review from the PowerShell Committee labels Dec 15, 2021

SteveL-MSFT marked this pull request as ready for review December 15, 2021 23:19

SteveL-MSFT changed the title ~~WIP: Fix #15644 - reduce the amount of startup banner text~~ Reduce the amount of startup banner text Dec 15, 2021

daxian-dbw approved these changes Dec 15, 2021

View reviewed changes

iSazonov reviewed Dec 16, 2021

View reviewed changes

src/Microsoft.PowerShell.ConsoleHost/resources/ManagedEntranceStrings.resx Show resolved Hide resolved

src/Microsoft.PowerShell.ConsoleHost/host/msh/UpdatesNotification.cs Show resolved Hide resolved

ghost added the Review - Needed The PR is being reviewed label Dec 24, 2021

Update -NoLogo parameter description to not mention copyright

89a8f4b

Not sure why the Linux test failing, try guard for removing env var

76a6520

rkeithhill added 2 commits January 7, 2022 12:56

Ah, don't rely on $env:temp on Linux, use Temp: drive instead

afe37a9

Add debug output in order to debug Linux/macOS failures

eb6a9b7

pull-request-quantifier-deprecated bot added Small and removed Extra Small labels Jan 7, 2022

Use Write-Warning instead of Write-Host for debug messages

f1e0411

rkeithhill added 4 commits January 7, 2022 16:38

Disable ANSI in the new pwsh process to eliminate DECCKM cmds in output

4906c6c

Account for possible DECCKM text for first command

d5eea4f

Include ANSI CSI ? chars in regex pattern

d6d7172

Remove warning output used to debug tests

ed082b8

Use $env:TERM="dumb" to avoid DECCKM codes in output

48f80e5

iSazonov approved these changes Jan 9, 2022

View reviewed changes

test/powershell/Host/ConsoleHost.Tests.ps1 Outdated Show resolved Hide resolved

Use -Be instead of -BeExactly for numeric comparison

62abe03

adityapatwardhan reviewed Jan 10, 2022

View reviewed changes

adityapatwardhan approved these changes Jan 10, 2022

View reviewed changes

adityapatwardhan merged commit e2f39fa into PowerShell:master Jan 10, 2022

rkeithhill deleted the rkeithhill/minimal-startup-text branch January 10, 2022 23:27

kilasuit mentioned this pull request Jan 11, 2022

-nologo should be the default #15644

Closed

TrapGodBrim pushed a commit to TrapGodBrim/PowerShell that referenced this pull request Jan 19, 2022

Reduce the amount of startup banner text (PowerShell#16516)

9ea2b40

Reduce the amount of startup banner text #16516

Reduce the amount of startup banner text #16516

Uh oh!

Conversation

rkeithhill commented Nov 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Context

PR Checklist

Uh oh!

iSazonov commented Nov 25, 2021

Uh oh!

sdwheeler commented Nov 25, 2021

Uh oh!

ghost commented Dec 14, 2021

Uh oh!

SteveL-MSFT commented Dec 15, 2021

Uh oh!

SteveL-MSFT commented Dec 15, 2021

Uh oh!

rkeithhill commented Dec 16, 2021

Uh oh!

Uh oh!

Uh oh!

iSazonov commented Dec 16, 2021

Uh oh!

iSazonov commented Dec 16, 2021

Uh oh!

ghost commented Dec 24, 2021

Uh oh!

rkeithhill commented Dec 24, 2021

Uh oh!

rkeithhill commented Jan 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rkeithhill commented Jan 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iSazonov commented Jan 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rkeithhill commented Jan 8, 2022

Uh oh!

iSazonov commented Jan 9, 2022

Uh oh!

Uh oh!

pull-request-quantifier-deprecated bot commented Jan 9, 2022

What can I do to optimize my changes

How to interpret the change counts in git diff output

Uh oh!

rkeithhill commented Jan 9, 2022

Uh oh!

rkeithhill commented Jan 10, 2022

Uh oh!

adityapatwardhan Jan 10, 2022

Choose a reason for hiding this comment

Uh oh!

rkeithhill Jan 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adityapatwardhan commented Jan 10, 2022

Uh oh!

ghost commented Feb 24, 2022

Uh oh!

DHowett commented Feb 25, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

rkeithhill commented Nov 25, 2021 •

edited

Loading

rkeithhill commented Jan 7, 2022 •

edited

Loading

rkeithhill commented Jan 7, 2022 •

edited

Loading

iSazonov commented Jan 8, 2022 •

edited

Loading

rkeithhill Jan 10, 2022 •

edited

Loading