Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Implement -version parameter in console host (address part of https:/…#3115

Merged
daxian-dbw merged 2 commits into
PowerShell:masterPowerShell/PowerShell:masterfrom
JamesWTruher:jameswtruher/ShowVersionJamesWTruher/PowerShell-1:jameswtruher/ShowVersionCopy head branch name to clipboard
Feb 23, 2017
Merged

Implement -version parameter in console host (address part of https:/…#3115
daxian-dbw merged 2 commits into
PowerShell:masterPowerShell/PowerShell:masterfrom
JamesWTruher:jameswtruher/ShowVersionJamesWTruher/PowerShell-1:jameswtruher/ShowVersionCopy head branch name to clipboard

Conversation

@JamesWTruher

@JamesWTruher JamesWTruher commented Feb 8, 2017

Copy link
Copy Markdown
Collaborator

#1084

This does not support providing a specific version to run, but
like most other *nix commands, -version will now return the version
of the PowerShell Engine. 'powershell' is prepended to the output to
match other *nix commands. We are using gitcommitid which includes more
info about the build.

@PowerShellTeam PowerShellTeam added the Review - Needed The PR is being reviewed label Feb 8, 2017
@SteveL-MSFT SteveL-MSFT added the Review - Committee The PR/Issue needs a review from the PowerShell Committee label Feb 8, 2017
@joeyaiello joeyaiello 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 Feb 9, 2017
@joeyaiello

Copy link
Copy Markdown
Contributor

@PowerShell/powershell-committee is good with all of this. 👍

@vors vors Feb 9, 2017

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.

redundant extra-line before else #Resolved

@vors vors Feb 9, 2017

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.

why not simple { get; private set; } #WontFix

@daxian-dbw daxian-dbw Feb 9, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it's fine to follow the existing code pattern here. We can convert the style using a tool later. #ByDesign

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.

to match the current code style


In reply to: 100213197 [](ancestors = 100213197)

@vors vors Feb 9, 2017

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 wonder how it's going to interact with existing in Windows PowerShell
powershell -version 2.0

Since we are stopping to build Windows powershell, and core edition doesn't have any -version 2.0 support, it's probably okey. #ByDesign

@SteveL-MSFT SteveL-MSFT Feb 9, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@vors, the @PowerShell/powershell-committee discussed this and that is pretty much the thinking: -version for Windows PowerShell really only supported 2.0 and we're moving away from that. Aligning with Linux convention makes sense. PSCore supports side-by-side intrinsically and using file paths to launch different versions is completely acceptable. #ByDesign

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Furthermore we can always do something like powershell -version:2 if people really demand that functionality in the future (and actually @lzybkr and @BrucePay agreed that it would even be acceptable to do powershell -version 2 as it's highly unlikely that anyone wants to echo one int when they shell out to PowerShell).

@daxian-dbw daxian-dbw Feb 9, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why are you setting _commandLineCommand here? #Resolved

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.

vestige of earlier implementation - removed


In reply to: 100401276 [](ancestors = 100401276)

@daxian-dbw daxian-dbw Feb 9, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I recommend to use PSVersionInfo.GetPSVersionTable()["GitCommitId"], or even better, add an internal static property GitCommitId right after the PSVersion property at here, and then use PSVersionInfo.GitCommitId here.

Reason: PSVersionInfo has a type initializer, which calls GetCommitInfo() and store the value in a hashtable. the current implementation of GetCommitInfo() reads git commit information from a file, which is slow and this implementation is subject to change (this method may be gone in near future -- we want to generate code at build time to embed the git commit id info, so as to avoid reading a file at startup time). Since the type initializer must run the first time you access a type, it's a waste to call GetCommitInfo() here to read the file again. #Resolved

@daxian-dbw daxian-dbw Feb 9, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There is an extra blank line here. #Resolved

@daxian-dbw daxian-dbw Feb 9, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

what would happen if I have powershell -noprofile -noexit -version? #WontFix

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.

it would report the version and exit - I can add a test for that explicitly if you like. Rather than testing all of the permutations for the parameters, I chose a representative single case


In reply to: 100405816 [](ancestors = 100405816)

@daxian-dbw daxian-dbw self-assigned this Feb 9, 2017
@daxian-dbw daxian-dbw added Review - Waiting on Author and removed Review - Needed The PR is being reviewed labels Feb 9, 2017
…ell#1084)

This does not support providing a specific version to run, but
like most other *nix commands, -version will now return the version
of the PowerShell Engine. 'powershell' is prepended to the output to
match other *nix commands. We are using gitcommitid which includes more
info about the build.
@PowerShellTeam PowerShellTeam added Review - Needed The PR is being reviewed and removed Review - Waiting on Author labels Feb 17, 2017
…on PSVersionInfo

Remove extraneous blank lines in files
remove line which set the commandline when retrieving the version as it is unneeded
@JamesWTruher JamesWTruher force-pushed the jameswtruher/ShowVersion branch from 39e0991 to 96abb8f Compare February 17, 2017 23:12

@daxian-dbw daxian-dbw left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@daxian-dbw daxian-dbw merged commit 9e27f4a into PowerShell:master Feb 23, 2017
@iSazonov iSazonov removed the Review - Needed The PR is being reviewed label Mar 27, 2017
@JamesWTruher JamesWTruher deleted the jameswtruher/ShowVersion branch September 23, 2023 05:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Committee-Reviewed PS-Committee has reviewed this and made a decision

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.