Skip to content

Update build as we move help content to PowerShell-Docs #1537

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

Merged
merged 5 commits into from
May 19, 2020

Conversation

daxian-dbw
Copy link
Member

@daxian-dbw daxian-dbw commented May 18, 2020

The help content in PSReadLine repo has been synced to PowerShell-Docs repo at https://github.com/MicrosoftDocs/PowerShell-Docs/tree/staging/reference/7.1/PSReadLine
The most recent updates on the predictive suggestion feature were synced to the doc repo by the following 3 PRs:

This PR is to remove the docs folder in this repo and update build script to check help content only in release build.
The help content check is again the help content retrieved by Update-Help.

For daily development builds, we don't check help content because we don't want the development in this repo to be blocked by the doc deployment.
A PR template is added to make sure we track if documentation needs to be updated.

Changes in this PR:

  • Remove docs and az-ci folders
  • Update the build script to not generate Maml help and about topic and remove the dependency on platyPS
  • Update the build script to check help content only when requested
  • Update release build yaml file to check the help content.

Currently, the deployed PSReadLine help content is missing following things:

Function not documented: AcceptSuggestion
Function not documented: AcceptNextSuggestionWord
Parameter Chord not documented in cmdlet Get-PSReadLineKeyHandler
Parameter PredictionSource not documented in cmdlet Set-PSReadLineOption

Those changes have been synced to the PowerShell-Doc repo by the PRs mentioned above. So the next deployment will get us the latest help content.

Microsoft Reviewers: Open in CodeFlow

@daxian-dbw daxian-dbw requested review from lzybkr and sdwheeler May 18, 2020 19:07
Copy link
Contributor

@lzybkr lzybkr left a comment

Choose a reason for hiding this comment

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

This seems mostly reasonable. Note that people have complained about being confused by docs that document unreleased work, so expect more of that considering this new process. I'm guessing it's unavoidable though since docs go live like almost immediately?

At any rate, when generating the about topic, the key bindings were generated automatically. Since this is no longer automated, the check script should parse the about topic and validate the key bindings are as expected.

@ghost ghost removed the Needs-Author Feedback label May 19, 2020
@daxian-dbw
Copy link
Member Author

@lzybkr Comments are addressed.
I updated CheckHelp.ps1 to check the default key bindings in about_PSReadLine.help.txt. There are a dozen of mismatches today, and all but 2 will be addressed by MicrosoftDocs/PowerShell-Docs#5964

The following 2 mismatches are sort of special:

Default bindings from code

BackwardChar
*
-   Cmd: <LeftArrow>
-   Emacs: <LeftArrow>, <Ctrl+b>

ForwardChar
*
-   Cmd: <RightArrow>
-   Emacs: <RightArrow>, <Ctrl+f>

Bindings from 'about_PSReadLine.help.txt'

BackwardChar

Move the cursor one character to the left. This may move the cursor to the
previous line of multi-line input.

-   Cmd: <LeftArrow>
-   Emacs: <LeftArrow>, <Ctrl+b>
-   Vi insert mode: <LeftArrow>
-   Vi command mode: <LeftArrow>, <Backspace>, <h>

ForwardChar

Move the cursor one character to the right. This may move the cursor to the
next line of multi-line input.

-   Cmd: <RightArrow>
-   Emacs: <RightArrow>, <Ctrl+f>
-   Vi insert mode: <RightArrow>
-   Vi command mode: <RightArrow>, <Space>, <l>

This is because ViBackwardChar and ViForwardChar are not exposed by code as bindable functions, so the about topic doc put the bindings for those 2 functions under BackwardChar and ForwardChar.
Is it intentional to hide ViBackwardChar and ViForwardChar? If not, I can update the code to expose them.

Copy link
Contributor

@lzybkr lzybkr left a comment

Choose a reason for hiding this comment

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

I don't have an opinion on the private vi-mode methods, so I guess it's fine to make them public.

}
}

$eol = "`r`n"
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems a little silly to explicitly use `r`n when `n works too. Or you could use [Environment]::NewLine to make it "obvious" that it works cross platform even though as written it seems like should work on Linux anyway.

Copy link
Member Author

@daxian-dbw daxian-dbw May 19, 2020

Choose a reason for hiding this comment

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

I use CRLF explicitly here because the about_PSReadLine.help.txt pulled down by Update-Help uses CRLF end-of-line sequence, on both Windows and Unix plats.
If switching to LF, the wildcard matching all always fail. I added a comment there.

@daxian-dbw
Copy link
Member Author

I don't have an opinion on the private vi-mode methods, so I guess it's fine to make them public.

Will make them public in the next PR.

@daxian-dbw daxian-dbw merged commit 2664653 into PowerShell:master May 19, 2020
@daxian-dbw daxian-dbw deleted the build branch May 19, 2020 22:08
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.

2 participants