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

Update ps1con prompt #1497

Closed
wants to merge 5 commits into from
Closed

Update ps1con prompt #1497

wants to merge 5 commits into from

Conversation

vsalvino
Copy link

@vsalvino vsalvino commented Jul 25, 2020

This change allows a more flexible PowerShell session prompt. The prompt now must start with PS and end with > but the inner contents are not important. This allow simpler prompts such as PS> which are more useful in documentation and more comparable to the Bash $.

Also adding documentation showing examples of how to use the prompts for each supported shell session. This was very difficult to look up previously, as one simply had to know the magic combination of characters that would be parsed as a prompt.

As a result of adding this doc, this change also updates the github action to use the current version of pygments to render the docs. Otherwise it would technically be installing whatever latest version is on pypi/cached, which might not match the master branch. In that case this updated languages doc would render incorrectly.

@Anteru Anteru self-assigned this Jul 27, 2020
@Anteru
Copy link
Collaborator

Anteru commented Jul 27, 2020

Does this work with remote sessions? There have been a few changes related to that recently, specifically adding the space before > which this change removes again -- did you check that those continue to work? Could you please add a few tests for the new syntax to make sure this doesn't break in the future (unit tests, not just documentation, as we don't check the documentation output.)

@vsalvino
Copy link
Author

vsalvino commented Jul 27, 2020

The current unit tests do pass, however I will check and add new tests. The idea was to support a prompt which does not contain a path. I would be OK with adding the space back in if necessary.

@Anteru
Copy link
Collaborator

Anteru commented Aug 5, 2020

Ok -- I'm mostly interested in a test for this, as I can't tell you if the space should be there or not, but if someone ever touches that that piece again, there should be a test which makes sure that today's command line don't break.

@vsalvino
Copy link
Author

vsalvino commented Aug 5, 2020

For reference, the default prompt is PS> https://docs.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_prompts?view=powershell-7#the-default-prompt

I will write some tests. This is my first time contributing to this project - could you link me to a good example test to model after?

@Anteru
Copy link
Collaborator

Anteru commented Aug 5, 2020

Yes :) Take a look here: https://github.com/pygments/pygments/blob/master/tests/test_shell.py#L172 -- there's also a special formatter (testcase) which produces test cases directly, so you can provide a text snippet and get the tokens to test for directly.

@Anteru
Copy link
Collaborator

Anteru commented Aug 5, 2020

Thanks, looks good!

@Anteru Anteru added this to the 2.7 milestone Aug 5, 2020
@Anteru
Copy link
Collaborator

Anteru commented Aug 22, 2020

Manually merged via a57edec. Thanks for your contribution!

@Anteru Anteru closed this Aug 22, 2020
@Anteru Anteru added the changelog-update Items which need to get mentioned in the changelog label Aug 22, 2020
@Anteru Anteru removed the changelog-update Items which need to get mentioned in the changelog label Sep 8, 2020
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