Skip to content

Fix help paging issues on macOS/Linux and with custom pager that takes arguments #9033

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.

Sign up for GitHub

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

Merged
TravisEz13 merged 6 commits into from
Apr 10, 2019
Merged

Fix help paging issues on macOS/Linux and with custom pager that takes arguments #9033

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f26054f
Pipe help system output through Out-String -Width when not using more
rkeithhill Mar 3, 2019
1f06d43
Set the string output width when sending help to pager app
rkeithhill Mar 3, 2019
790d2e3
Fix #8912 by using PS tokenize to get custom pager cmd & args
rkeithhill Mar 3, 2019
9374c79
Fix macOS/Linux test failures
rkeithhill Mar 3, 2019
9ecd62c
It appears that while running in tests, ConsoleWidth is 0, set min val
rkeithhill Mar 3, 2019
bb62a37
Address PR feedback
rkeithhill Apr 10, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 50 additions & 29 deletions src/System.Management.Automation/engine/InitialSessionState.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4176,40 +4176,61 @@ .FORWARDHELPCATEGORY Cmdlet
}
elseif ($help -ne $null)
{
$customPagerCommand = """"
$customPagerCommandArgs = """"
$customPagerCommandLine = """"

if ($env:PAGER)
{
$customPagerCommandLine = $env:PAGER
# By default use more on Windows and less on Linux.
if ($IsWindows) {
Comment thread
rkeithhill marked this conversation as resolved.
$pagerCommand = 'more.com'
$pagerArgs = $null
}
else {
Comment thread
rkeithhill marked this conversation as resolved.
$pagerCommand = 'less'
$pagerArgs = '-Ps""Page %db?B of %D:.\. Press h for help or q to quit\.$""'
}

# Respect PAGER environment variable which allows user to specify a custom pager.
# Ignore a pure whitespace PAGER value as that would cause the tokenizer to return 0 tokens.
if (![string]::IsNullOrWhitespace($env:PAGER)) {
Comment thread
rkeithhill marked this conversation as resolved.
if (Get-Command $env:PAGER -ErrorAction Ignore) {
Comment thread
rkeithhill marked this conversation as resolved.
# Entire PAGER value corresponds to a single command.
$pagerCommand = $env:PAGER
$pagerArgs = $null
}
else {
Comment thread
rkeithhill marked this conversation as resolved.
# PAGER value is not a valid command, check if PAGER command and arguments have been specified.
# Tokenize the specified $env:PAGER value. Ignore tokenizing errors since any errors may be valid
# argument syntax for the paging utility.
$errs = $null
$tokens = [System.Management.Automation.PSParser]::Tokenize($env:PAGER, [ref]$errs)

@iSazonov iSazonov Mar 3, 2019

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'd expect that Tokenizer is very fast and more fast than Get-Command. So we could always call it and then Get-Command. This simplify code.
We need look $errs and write useful message to user.
Also we could use try-catch for Get-Command and running the command to write more useful messages to user.

@rkeithhill rkeithhill Mar 3, 2019 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I'm not sure we should expose tokenizer errors. We are only using the tokenizer when piping help output to applications. The arguments passed to those applications could very well fail to tokenize by PowerShell. For instance, say an app took an argument like @{HEAD-1}, perfectly valide for the app but gens a tokenizer error:

Missing '=' operator after key in hash literal.

For debugging, I propose we leave the __PSPAGER_ARGS env var so the user can inspect it to see exactly what args we are passing to a pager application.

@iSazonov iSazonov Mar 6, 2019

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm not sure we should expose tokenizer errors.

No expose. I think we could use this to create more useful message to user if it is possible.


$customPagerCommand = $tokens[0].Content
if (!(Get-Command $customPagerCommand -ErrorAction Ignore)) {
Comment thread
rkeithhill marked this conversation as resolved.
# Custom pager command is invalid, issue a warning.
Write-Warning ""Custom-paging utility command not found. Ignoring command specified in `$env:PAGER: $env:PAGER""
}
else {
Comment thread
rkeithhill marked this conversation as resolved.
# This approach will preserve all the pagers args.
$pagerCommand = $customPagerCommand
$pagerArgs = if ($tokens.Count -gt 1) {$env:PAGER.Substring($tokens[1].Start)} else {$null}
}
}
}

# Split the command line into tokens, respecting quoting.
$customPagerCommand, $customPagerCommandArgs = & { Write-Output -- $customPagerCommandLine }
# If the pager is an application, format the output width before sending to the app.
if ((Get-Command $pagerCommand -ErrorAction Ignore).CommandType -eq 'Application') {
Comment thread
rkeithhill marked this conversation as resolved.
$consoleWidth = [System.Math]::Max([System.Console]::WindowWidth, 20)

# See if the first token refers to a known command (executable),
# and if not, see if the command line as a whole is an executable path.
$cmds = Get-Command $customPagerCommand, $customPagerCommandLine -ErrorAction Ignore
if (-not $cmds) {
# Custom command is invalid; ignore it, but issue a warning.
Write-Warning ""Ignoring invalid custom-paging utility command line specified in `$env:PAGER: $customPagerCommandLine""
$customPagerCommand = $null # use default command
if ($pagerArgs) {
Comment thread
rkeithhill marked this conversation as resolved.
# Supply pager arguments to an application without any PowerShell parsing of the arguments.
# Leave environment variable to help user debug arguments supplied in $env:PAGER.
$env:__PSPAGER_ARGS = $pagerArgs
$help | Out-String -Stream -Width ($consoleWidth - 1) | & $pagerCommand --% %__PSPAGER_ARGS%
}
elseif ($cmds.Count -eq 1 -and $cmds[0].Source -eq $customPagerCommandLine) {
# The full command line is an unquoted path to an existing executable
# with embedded spaces.
$customPagerCommand = $customPagerCommandLine
$customPagerCommandArgs = $null
else {
Comment thread
rkeithhill marked this conversation as resolved.
$help | Out-String -Stream -Width ($consoleWidth - 1) | & $pagerCommand
}
}

# Respect PAGER, use more on Windows, and use less on Linux
if ($customPagerCommand) {
$help | & $customPagerCommand $customPagerCommandArgs
} elseif ($IsWindows) {
$help | more.com
} else {
$help | less -Ps""Page %db?B of %D:.\. Press h for help or q to quit\.$""
else {
Comment thread
rkeithhill marked this conversation as resolved.
# The pager command is a PowerShell function, script or alias, so pipe directly into it.
$help | & $pagerCommand $pagerArgs
}
}
";
Expand Down