Change to not expose unsupported aliases/cmdlets in Unix #3595

iSazonov · 2017-04-19T09:01:25Z

Make changes to not expose the aliases "gin", "gsv", "sasv" and "spsv" in Unix platforms.
Also refactored aliases related tests and added new tests covering the complete list of built-in aliases/cmdlets.

mirichmo · 2017-04-20T00:58:31Z

test/powershell/Modules/Microsoft.PowerShell.Management/Get-ComputerInfo.Tests.ps1

Rather than removing this test, can you skip it for non-windows platforms?

The idea (from @lzybkr) is that we should put all alias tests in one place (Default-Aliases.Tests.ps1) rather than spread on many files.

daxian-dbw · 2017-04-20T01:19:23Z

@iSazonov thanks for the fix!
There are a few other aliases that are not supposed to be exposed on Unix. Do you think they can be taken care of in the same PR? (absolutely not required 😄)

PS /> Get-Alias | ? { gcm $_.Definition -ea SilentlyContinue > $null; !$? }                         

CommandType     Name                                               Version    Source               
-----------     ----                                               -------    ------               
Alias           gin -> Get-ComputerInfo                            3.1.0.0    Microsoft.PowerShe...
Alias           gsv -> Get-Service                                                                 
Alias           sasv -> Start-Service                                                              
Alias           spsv -> Stop-Service

iSazonov · 2017-04-20T09:27:37Z

@daxian-dbw I am glad to fix this aliases too while the ears in this.

Now I catched interensting issue with "gin" alias. Please help me understand. As for Get-Service cmdlet I add the alias in InitialSessionState.cs and remove from .psd1 files. After that I start PowerShell, ipmo .\Build.psm1 and get error "AllScope option cannot be removed from the alias 'gin'". CI tests is failed too as you see. Then I remove [Alias("gin")] from GetComputerInfoCommand.cs - now all work well.

So my question is what is right way? Move aliases to InitialSessionState.cs or add Alias attribute to cmdlets and exclude them on unsupported platform? Or Alias attribute is for third-party cmdlets only and we should remove it from Get-ComputerInfo?
How Get-ComputerInfo is excluded from compiling code on Unix?
Why we only init exported aliases in InitialSessionState.cs but not cmdlets?

daxian-dbw · 2017-04-20T22:48:48Z

So my question is what is right way?

What I would do is:

For cmdlets that are not supposed to work on Unix, like Get-ComputerInfo, exclude the source code from unix build, by enclosing the code with in #if !UNIX and #endif. Please take TimeZoneCommands.cs as an example.
For aliases that declared in InitialSessionState.cs, move the declarations to the #if !UNIX/endif section. You will find the section in the part of code that declares all SessionStateAliasEntry instances.

For cmdlets like *-Clipboard, they have already been excluded from the build in powershell core. You can take a look at the .csproj file of the corresponding project.

Basically, there are also 4 aliases that are currently exposed on unix plats by mistake. You can run the following command to observe.

Get-Alias | ? { gcm $_.Definition -ea SilentlyContinue > $null; !$? }                         

CommandType     Name                                               Version    Source               
-----------     ----                                               -------    ------               
Alias           gin -> Get-ComputerInfo                            3.1.0.0    Microsoft.PowerShe...
Alias           gsv -> Get-Service                                                                 
Alias           sasv -> Start-Service                                                              
Alias           spsv -> Stop-Service

iSazonov · 2017-04-21T11:10:18Z

@daxian-dbw Thanks! I still don't understand

If currently Get-ComputerInfo don't excluded by #if !UNIX and #endif it is bug and we should have tests to control such problems?
What is the preferred way to add an alias in InitialSessionState.cs or by an attribute [Alias("alias")]? In last commits I used first way. We should use the single preferred way to avoid errors (which addressed in the PR). It seems the first way gives a more quick start, the second way is more intuitive.

@lzybkr Could you please comment too?

daxian-dbw · 2017-04-21T16:31:36Z

If currently Get-ComputerInfo don't excluded by #if !UNIX and #endif it is bug and we should have tests to control such problems?

Yes, we can have a test for it. Basically, we need to get the list of types that shouldn't be available on different platforms, for example, [Microsoft.PowerShell.Commands.GetComputerInfoCommand] should fail on Unix plats.

What is the preferred way to add an alias in InitialSessionState.cs or by an attribute [Alias("alias")]? In last commits I used first way. We should use the single preferred way to avoid errors (which addressed in the PR). It seems the first way gives a more quick start, the second way is more intuitive.

Declaration of aliases in InitialSessionState.cs came from the days before [Alias] attribute was made supported for command types (previously [Alias] was only supported for parameters). So they are legacy code. For new cmdlet code, AliasAttribute should be used.

For the legacy code, I'm inclined to keep them that way, as I'm not sure if changing them would cause some unobvious regression. For new cmdlet implementation, I think AliasAttribute is the way to go.

iSazonov · 2017-04-21T19:28:27Z

Can we create new attribute like Platform("Unix") to mark cmdlet classes? Can this approach make our tests easier?

What unobvious regression you expect with aliases? I believe that there is no functional code which we can break down, no new code. On the other hand, if we move the aliases in cmdlets ([Alias]) we get rid of some of the complexities that are trying to fix here.

daxian-dbw · 2017-04-21T22:09:59Z

What unobvious regression you expect with aliases?

Those alias declarations are publicly accessible, via InitialSessionState.Commands. For example:

PS> $iis = [initialsessionstate]::CreateDefault()
PS> $iis.Commands.Name | ? { $_ -eq 'gps' }
gps

So it's hard to anticipate how people will depend on them ... that's why I said unobviously regression.

Can we create new attribute like Platform("Unix") to mark cmdlet classes? Can this approach make our tests easier?

A class that should be excluded from a platform may not be a cmdlet, so I don't think a new attribute will help.

Not excluding a file that is supposed to be excluded on a platform won't be a big issue really. I think we can just fix it when it comes up.

iSazonov · 2017-04-22T10:59:34Z

Clear about new attribute.

I tried to add gin alias to InitialSessionState.cs and use Alias('gin')] (the code add cmdlet aliases in a session) - in both cases, I see the alias in a session state. Both options work equally. So it looks safe if we move the cmdlet aliases in cmdlets and exclude the files (or the aliases) with #if !UNIX and #endif if we need. The only public change will be (not breaking change) that we will remove aliases that should not be there (aliases for not ported cmdlets or aliases unwanted on Unix) and add the ones we want to appear. So users get right thinks with [initialsessionstate]::CreateDefault().

daxian-dbw · 2017-04-24T23:04:38Z

@iSazonov thanks for the extra validation exercise! Then I'm fine moving the aliases from InitialSessionState to individual cmdlets. Do you want to do that in a separete PR or address all in this PR?

iSazonov · 2017-04-25T13:11:39Z

@daxian-dbw Thanks for approve! I believe we can continue here to preserve the usefull discussion.

I have already started to prepare new tests.

iSazonov · 2017-04-25T15:30:24Z

I created new file with tests to check default alias and cmdlet lists.
Alias tests is from Default-Aliases.Tests.ps1. Later we should remove the test from Default-Aliases.Tests.ps1.
I would use the session state to get default alias and cmdlet lists instead of Get-Alias and Get-Command but latest is more simple use.

@daxian-dbw Please review the test. After that the way will be open for safe code edits.

daxian-dbw · 2017-04-25T17:09:36Z

test/powershell/Modules/Microsoft.PowerShell.Utility/Default-Aliases.Tests.ps1

"nwsn -> New-PSWorkflowSession" = $FullCLR

I don't see this alias on windows powershell ...

PS:4> Get-Alias -Definition New-PSWorkflowSession Get-Alias : This command cannot find a matching alias because an alias with the definition 'New-PSWorkflowSession' does not exist. At line:1 char:1 + Get-Alias -Definition New-PSWorkflowSession + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + CategoryInfo : ObjectNotFound: (New-PSWorkflowSession:String) [Get-Alias], ItemNotFoundException + FullyQualifiedErrorId : ItemNotFoundException,Microsoft.PowerShell.Commands.GetAliasCommand

Environment

Name Value ---- ----- PSVersion 5.1.14393.1066 PSEdition Desktop PSCompatibleVersions {1.0, 2.0, 3.0, 4.0...} BuildVersion 10.0.14393.1066 CLRVersion 4.0.30319.42000 WSManStackVersion 3.0 PSRemotingProtocolVersion 2.3 SerializationVersion 1.1.0.1

😕 I see on 10.0.10240 and on 10.0.16179

My bad. It's defined in the module PSWorkflow which is not loaded by default. Let's keep it in the list.

daxian-dbw · 2017-04-25T17:13:39Z

test/powershell/Modules/Microsoft.PowerShell.Utility/Default-Aliases.Tests.ps1

gtz is also available in windows powershell.

No on 10.0.10240

PS:3> Get-Alias gtz CommandType Name Version Source ----------- ---- ------- ------ Alias gtz -> Get-TimeZone 3.1.0.0 Microsoft.PowerShell.Manag...

Available in 10.0.14393.1066

Name Value ---- ----- PSVersion 5.1.14393.1066 PSEdition Desktop PSCompatibleVersions {1.0, 2.0, 3.0, 4.0...} BuildVersion 10.0.14393.1066 CLRVersion 4.0.30319.42000 WSManStackVersion 3.0 PSRemotingProtocolVersion 2.3 SerializationVersion 1.1.0.1

What version we should use as a base?

daxian-dbw · 2017-04-25T17:16:48Z

test/powershell/Modules/Microsoft.PowerShell.Utility/Default-Aliases.Tests.ps1

don't show full list wrong aliases

doesn't show the list of different aliases

This comment hasn't been addressed yet.

We should remove this from Default-Aliases.Tests.ps1 as the last step in the PR.

sounds good.

daxian-dbw · 2017-04-25T17:17:53Z

@iSazonov the test looks mostly good. I left a few comments.

iSazonov · 2017-04-27T16:03:09Z

@daxian-dbw I fixed the original list of aliases.
Please could you comment:

Currently in InitialSessionState.cs all aliases is added with 'ScopedItemOptions.AllScope' attribute and some one still with ScopedItemOptions.ReadOnly. I don't see any patterns in that - why not all aliases readonly? Ex.: Connect-PSSession - read only but New-PSSession - no?
I could agree that some core aliases should be read only but not all.
Also from [Alias()] an attribute is added with ScopedItemOptions.None. Why no 'ScopedItemOptions.AllScope'? Why no ScopedItemOptions.ReadOnly?
There is exclusion for sls and Select-String - where did it come from?

mirichmo · 2017-05-03T03:34:53Z

@daxian-dbw - Please take another look.

daxian-dbw · 2017-05-12T00:17:24Z

@iSazonov sorry for coming back so late.
The scope options, yes, thanks for bringing it up --

AllScope indicates the alias is copied to any new scopes that are created
ReadOnly indicates the alias cannot be changed

I don't see any patterns in that - why not all aliases readonly? Ex.: Connect-PSSession - read only but New-PSSession - no?
I could agree that some core aliases should be read only but not all.

I agree the alias option settings there seem a bit messed up, for example, aliases for *-PSSession cmdlets should be consistent in my opinion.
However, some of the aliases are made AllScope only for a good reason -- so that user can make them point to customized function/utility as they want. An example here that override the dir alias.

Also from [Alias()] an attribute is added with ScopedItemOptions.None. Why no 'ScopedItemOptions.AllScope'? Why no ScopedItemOptions.ReadOnly?

Yep, AliasAttribute now doesn't set any options to an alias, which seems by design to me, because we don't want the module author to decide if their alias definition should be copied to all scopes or can never be changed by the user. For a module cmdlet, a user should be the one to make that decision. PowerShell defines some alias as 'AllScope' or 'ReadOnly' because they are core cmdlets and probably shouldn't be randomly altered by the user.

There is exclusion for sls and Select-String - where did it come from?

This can be explained by the same dir example I gave above. It sounds like very common for people to set sls to point to a different function/utility. With AllScope, you have to write Set-Alias ... -Option AllScope to make it work, which is annoying. I think that's why it was made None option.

It turns out there are good reasons to keep alias declarations in InitialSessionState.cs. I suggest we go back to the proposal I made in #3595 (comment)

mirichmo · 2017-05-12T16:38:47Z

@iSazonov - Please update the PR title and description to reflect the current state of the PR. This PR has slowly moved beyond the original intent and I'd like that reflected in the title and description.

Fix 'gsv','sasv' and 'spsv' Fix 'fhx' alias Fix 'cfs' alias Fix 'gcb' and 'scb' aliases Fix 'gtz' and 'stz' aliases Fix 'gin'

iSazonov · 2017-05-15T11:45:18Z

@mirichmo the PR title and description was updated.

@daxian-dbw Thanks for great comments! I moved aliases to InitialSessionState.cs
I did not fix alias 'ReadOnly' because we need make review for all aliases. Maybe open new Issue?

daxian-dbw · 2017-05-15T17:30:29Z

src/Microsoft.PowerShell.Commands.Management/commands/management/GetComputerInfoCommand.cs

    /// </summary>
    [Cmdlet(VerbsCommon.Get, "ComputerInfo",
        HelpUri = "https://go.microsoft.com/fwlink/?LinkId=799466")]
-    [Alias("gin")]


I don't think we should move gin to InitialSessionState.cs. I think for all aliases that are declared using Alias() attribute, we should keep them that way. They are not core cmdlets, and I don't see a problem to have them with 'None' option.

For those aliases that are already declared in InitialSessionState.cs, I prefer to keep them there for now, for the sake of backward compatibility.
For those aliases that are declared with AliasAttribute, I also prefer to keep them that way unless it's a core cmdlet that should be set with options other than 'None'.

I have found no other [Alias()] in the repo. So we should keep consistency and use the same approach - place aliases in InitialSessionState.cs.
Also I set ReadOnly because later we can replace it on None based on feedback but changing None to ReadOnly will be breaking change.

For those aliases that are already declared in InitialSessionState.cs, we should keep them there for backward compatibility. For new aliases, I believe they should be declared using AliasAttribute, unless there is an explicit reason to set it with an option other than None.

Should we ignore here performance of PowerShell Core start-up?

My opinion doesn't stem from the performance concern. I just think we should no longer keep stuffing unnecessary settings to InitialSessionState.cs. Keep alias declaration in individual cmdlet is good for at least 2 reasons:

alias declaration is together with the cmdlet definition -- easy to read/discover and the alias exists in the cmdlet as a metadata information which can be analyzed using reflection by tools (if there is any tool doing so).

we want to stop the pattern of declaring aliases in a centralized location, so that when multiple people working on different cmdlets at the same time, they will have less conflicts that are resulted from editing the same file.

For those existing alias declarations in InitialSessionState.cs, we cannot move them out due to the ScopeItemOption settings (BTW, I think for some of the aliases there, the authors probably just copied the existing options without knowing if they should actually be applied). But for new aliases, I think it's better to have them coupled with the cmdlet unless an option other than 'none' is really desired.

There are other benefits to the Alias attribute:

AllScope is terrible for performance - there is little benefit to have most of these aliases as AllScope

Consistency - if a non-default runspace is constructed and imports one of these modules, that runspace would not get the aliases defined by default. For example:

function DoIt($createArg) { [powershell]::Create($createArg). AddScript('$ExecutionContext.InvokeCommand.GetCommand("clc", "Alias")'). Invoke() } DoIt ([System.Management.Automation.RunspaceMode]::NewRunspace) DoIt ([System.Management.Automation.RunspaceMode]::CurrentRunspace) $iss = [InitialSessionState]::Create() $iss.ImportPSModule('Microsoft.PowerShell.Management') $iss.LanguageMode = 'FullLanguage' DoIt $iss

This prints:

CommandType Name Version Source ----------- ---- ------- ------ Alias clc -> Clear-Content Alias clc -> Clear-Content

Note - only 2 results instead of 3. This is a little weird because Clear-Content is in Microsoft.PowerShell.Management, so why isn't clc?

We should perhaps consider adding support for AllScope/ReadOnly in the Alias attribute, perhaps as an internal only option so that it isn't abused.

@daxian-dbw @lzybkr Thanks for clarify!
So a plan is:
1.In the PR make a fix as @daxian-dbw requested
2.Open new Issue (to get feedback for removing AllScope) and then new PR to add internal only option to add support for AllScope/ReadOnly in the Alias attribute and migrate (1) alias and (2) cmdlet declarations from InitialSessionState.cs to modules.

Is it Ok?

Sounds like a plan. When creating the new issue, please make sure all our discussion here are captured in the issue. Thanks @iSazonov!

daxian-dbw · 2017-05-15T17:31:37Z

src/Microsoft.PowerShell.Commands.Management/commands/management/TimeZoneCommands.cs

    /// </summary>
    [Cmdlet(VerbsCommon.Get, "TimeZone", DefaultParameterSetName = "Name",
        HelpUri = "https://go.microsoft.com/fwlink/?LinkId=799468")]
-    [Alias("gtz")]


Same to gtz and stz in TimeZoneCommands.cs

daxian-dbw · 2017-05-15T17:35:10Z

src/System.Management.Automation/engine/InitialSessionState.cs

                        {
+                            // We add aliases from custom modules with 'ScopedItemOptions.None'
+                            // because only a module consumer (not the module author) decides
+                            // whether the alias is read only and available in all scopes.


These comments actually should go to AliasAttribute.

Move the comment or copy?

My bad. This comment should be put here.
However, the comment may need a bit change, since AliasAttribute can be used in built-in module, not custom modules only. Maybe the following is a little better:
"Alias declared by AliasAttribute is set with the option 'ScopedItemOptions.None', because we believe a user of the cmdlet, instead of the author of it, should be the one to decide the option of the alias usage."

Maybe:
"Alias declared by AliasAttribute is set with the option 'ScopedItemOptions.None', because we believe a user of the cmdlet, instead of the author of it, should be the one to decide the option ('ScopedItemOptions.ReadOnly' and/or 'ScopedItemOptions.AllScopes') of the alias usage."

Sounds good 👍

daxian-dbw · 2017-05-15T17:46:45Z

src/Microsoft.PowerShell.Commands.Management/commands/management/GetClipboardCommand.cs

@@ -1,4 +1,6 @@
-using System;
+#if !CORECLR


GetClipboardCommand.cs and SetClipboardCommand.cs are currently not included in the build (see here). We use those excluded compile entries in csproj files as a living document to track what built-in cmdlets are not in powershell core yet. So the if/def CORECLR is not necessary in those files.

iSazonov · 2017-05-16T10:14:49Z

If we shall move alias definitions to multiple files (modules) maybe add tests in DefaultCommands.Tests.ps1 to check alias's ScopedItemOptions?

daxian-dbw · 2017-05-16T17:12:27Z

...rosoft.PowerShell.Commands.Utility/commands/utility/FormatAndOutput/format-hex/Format-Hex.cs

    /// Displays the hexidecimal equivalent of the input data.
    /// </summary>
    [Cmdlet(VerbsCommon.Format, "Hex", SupportsShouldProcess = true, HelpUri ="https://go.microsoft.com/fwlink/?LinkId=526919")]
-    [Alias ("fhx")]


This alias declaration shouldn't be removed.

Oh, sorry. Fixed.

daxian-dbw · 2017-05-16T17:12:43Z

src/Modules/Unix/Microsoft.PowerShell.Utility/Microsoft.PowerShell.Utility.psd1

    "Get-Runspace", "Debug-Runspace", "Enable-RunspaceDebug", "Disable-RunspaceDebug",
    "Get-RunspaceDebug", "Wait-Debugger" , "Get-Uptime", "New-TemporaryFile", "Get-Verb", "Format-Hex"
 FunctionsToExport= "Import-PowerShellDataFile"
-AliasesToExport= "fhx"


This shouldn't be removed.

Why? Currently Format-Hex cmdlet is binary. I believe it was left from the time when the cmdlet was function.
Furthermore we now control all the aliases in the tests and can safely export all aliases by default.

If alias is not declared in .psd1 file, using the alias when the module hasn't been loaded will not trigger the module autoloading. Try the following and you will see it:

PS:44> gmo | rmo PS:45> gmo ModuleType Version Name ExportedCommands ---------- ------- ---- ---------------- Manifest 3.1.0.0 Microsoft.PowerShell.Utility {Add-Member, Add-Type, Clear-Variable, Compare-Object...} PS:46> gin gin : The term 'gin' is not recognized as the name of a cmdlet, function, script file, or operable program. Check the spelling of the name, or if a path was included, verify that the path is correct and try again. At line:1 char:1 + gin + ~~~ + CategoryInfo : ObjectNotFound: (gin:String) [], CommandNotFoundException + FullyQualifiedErrorId : CommandNotFoundException

Is the behavior different from cmdlets export?

If we move aliases from InitialSessionState.cs we should add all ones to psd1?

gcm | ? Source -eq "Microsoft.PowerShell.Management" | % {Get-Alias -Definition $_.Name -ErrorAction SilentlyContinue}

Is the behavior different from cmdlets export?

Not sure what you mean by "cmdlet export".

If we move aliases from InitialSessionState.cs we should add all ones to psd1?

From what I observed, it seems to be so. But it feels natural to me -- module should control what aliases to expose, and it's more discoverable if the .psd1 files always tell the complete truth.

Not sure what you mean by "cmdlet export".

Defaults is AliasesToExport="" and CmdletsToExport="". For first case the module autoload don't works, for second it works?

No, it won't work. If you remove a cmdlet from the "CmdletsToExport" then that cmdlet will not be exposed from the module.

Thanks for clarify! It seems I confused with the IntelliSense behavior.

daxian-dbw · 2017-05-16T17:13:17Z

src/Modules/Windows-Core/Microsoft.PowerShell.Management/Microsoft.PowerShell.Management.psd1

 PowerShellVersion="3.0"
 NestedModules="Microsoft.PowerShell.Commands.Management.dll"
 HelpInfoURI = 'https://go.microsoft.com/fwlink/?linkid=390785'
-AliasesToExport = @("gin", "gtz", "stz")


This shouldn't be removed.

The same - we now control all the aliases in the tests and can safely export all aliases by default.

Same reason as explained above.

daxian-dbw · 2017-05-16T17:13:34Z

src/Modules/Windows-Core/Microsoft.PowerShell.Utility/Microsoft.PowerShell.Utility.psd1

    "Unblock-File", "Get-Runspace", "Debug-Runspace", "Enable-RunspaceDebug", "Disable-RunspaceDebug",
    "Get-RunspaceDebug", "Wait-Debugger" , "Get-Uptime", "Get-Verb", "Format-Hex"
 FunctionsToExport= "ConvertFrom-SddlString"
-AliasesToExport= "fhx"


This shouldn't be removed.

The same - we now control all the aliases in the tests and can safely export all aliases by default.

Same reason as explained above.

daxian-dbw · 2017-05-16T17:13:44Z

src/Modules/Windows-Full/Microsoft.PowerShell.Management/Microsoft.PowerShell.Management.psd1

 CLRVersion="4.0"
 NestedModules="Microsoft.PowerShell.Commands.Management.dll"
 HelpInfoURI = 'https://go.microsoft.com/fwlink/?linkid=390785'
-AliasesToExport = @("gcb", "scb", "gin", "gtz", "stz")


Shouldn't be removed.

The same - we now control all the aliases in the tests and can safely export all aliases by default.

Same reason as explained above.

daxian-dbw · 2017-05-16T17:34:05Z

test/powershell/Modules/Microsoft.PowerShell.Utility/Remove-PSBreakpoint.Tests.ps1

-	    $(Get-PSBreakpoint).Id.length | Should Be ($NumberOfBreakpoints -1)
-	}
-    }
-


Same here. Shouldn't be removed.

Previous test do that so the test is alias related only.

daxian-dbw · 2017-05-16T17:34:17Z

test/powershell/Modules/Microsoft.PowerShell.Utility/Remove-Variable.Tests.ps1

-
-	$var1 | Should Be #nothing.  it should be Nothing at all.
-
-    }


same here. Shouldn't be removed.

Previous test do that so the test is alias related only.

daxian-dbw · 2017-05-16T17:34:37Z

test/powershell/Modules/Microsoft.PowerShell.Utility/Select-String.Tests.ps1

-    it "Should return an array data type when multiple matches are found" {
-        $result = $testinputtwo | Select-String -Pattern "hello"
-        ,$result | Should BeOfType "System.Array"
-    }


Same here. Both tests shouldn't be removed.

The first - Previous test do that so the test is alias related only.
The second is not removed - it is below - diff trick 😕

daxian-dbw · 2017-05-16T17:36:29Z

test/powershell/engine/DefaultCommands.Tests.ps1

+            # instead of Get-Alias and Get-Command
+            # but latest is more simple use.
+            #$iis = [initialsessionstate]::CreateDefault()
+            #$currentCmdletList = $iis.Commands | Where-Object { $_.CommandType -eq "Cmdlet"} | Select-Object -ExpandProperty Name


These comments don't seem necessary to keep.

Yes. Removed.

daxian-dbw · 2017-05-16T17:37:37Z

test/powershell/engine/DefaultCommands.Tests.ps1

+        # if all aliases is Ok we output nothing
+        $result | Write-Host
+        $result | Should Be $null
+    }


Isn't this test the same as the one in Default-Aliases.Tests.ps1? Are you planning to remove Default-Aliases.Tests.ps1 and replace it with this file?

Yes, I'll remove the duplication. I leave it for easy review and for checking new tests in DefaultCommands.Tests.ps1.

daxian-dbw · 2017-05-16T17:42:06Z

If we shall move alias definitions to multiple files (modules) maybe add tests in DefaultCommands.Tests.ps1 to check alias's ScopedItemOptions?

That sounds great.

daxian-dbw · 2017-05-19T18:36:47Z

@iSazonov The changes look good to me, and special thanks for the new tests covering the complete list of built-in aliases/cmdlets!
I think it's good to go as soon as you remove the old Default-Aliases.Tests.ps1.

New commits have been pushed

iSazonov · 2017-05-22T04:30:47Z

@daxian-dbw I removed Default-Aliases.Tests.ps1 and move test for more to DefaultCommands.Tests.ps1 (Remaining tests looks as duplications)

iSazonov · 2017-05-23T17:32:38Z

@daxian-dbw Thanks for help and great comments!

msftclas added the cla-already-signed label Apr 19, 2017

mirichmo previously requested changes Apr 20, 2017

View reviewed changes

mirichmo self-assigned this Apr 20, 2017

iSazonov force-pushed the remove-gin-on-unix branch from f45592d to dd16aa5 Compare April 20, 2017 05:55

iSazonov force-pushed the remove-gin-on-unix branch from 9b3179f to 5546b3b Compare April 20, 2017 10:49

mirichmo requested a review from daxian-dbw April 25, 2017 16:18

daxian-dbw reviewed Apr 25, 2017

View reviewed changes

iSazonov force-pushed the remove-gin-on-unix branch 2 times, most recently from 6f99fd4 to 8bbd6e2 Compare April 27, 2017 13:59

iSazonov added 2 commits May 15, 2017 12:47

Remove 'gin' alias on Unix

8ee5bf0

Fix aliases

501e2f0

Fix 'gsv','sasv' and 'spsv' Fix 'fhx' alias Fix 'cfs' alias Fix 'gcb' and 'scb' aliases Fix 'gtz' and 'stz' aliases Fix 'gin'

iSazonov force-pushed the remove-gin-on-unix branch from 8abdc86 to 880e21f Compare May 15, 2017 09:53

iSazonov added 2 commits May 15, 2017 14:14

Move aliases to InitialSessionState.cs

9dc9ab8

Remove unneeded tests

113e43b

iSazonov force-pushed the remove-gin-on-unix branch from 89d6933 to 113e43b Compare May 15, 2017 11:21

iSazonov changed the title ~~Remove 'gin' alias on Unix~~ Move aliases to InitialSessionState.cs May 15, 2017

Add comment

609649b

daxian-dbw reviewed May 15, 2017

View reviewed changes

iSazonov added 2 commits May 16, 2017 06:49

Remove if/def CORECLR

b40f5a1

Move new aliases to modules

f59b878

daxian-dbw reviewed May 16, 2017

View reviewed changes

iSazonov added 4 commits May 17, 2017 14:24

Fix [Alias ("fhx")]

6ea3980

Remove 'cfs' alias

790ba5a

Remove comment

b74d04c

Remove unneeded alias test

8f4aaff

iSazonov mentioned this pull request May 17, 2017

Add internal only support for ScopeItemOption settings in 'AliasAttribute' #3796

Closed

5 tasks

iSazonov added 2 commits May 18, 2017 09:38

Fix changes in .psd1 files

158cb3b

Add checks for alias options and minor refactoring

705d3de

Remove duplicated alias tests

3bba7ae

daxian-dbw approved these changes May 23, 2017

View reviewed changes

daxian-dbw merged commit a02d129 into PowerShell:master May 23, 2017

daxian-dbw changed the title ~~Move aliases to InitialSessionState.cs~~ Change to not expose unsupported aliases/cmdlets in Unix May 23, 2017

iSazonov deleted the remove-gin-on-unix branch October 25, 2017 11:19

iSazonov mentioned this pull request Oct 30, 2017

Remove AllScope from most default aliases #5268

Merged

Change to not expose unsupported aliases/cmdlets in Unix #3595

Change to not expose unsupported aliases/cmdlets in Unix #3595

Uh oh!

Conversation

iSazonov commented Apr 19, 2017 • edited by daxian-dbw Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daxian-dbw commented Apr 20, 2017

Uh oh!

iSazonov commented Apr 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

daxian-dbw commented Apr 20, 2017

Uh oh!

iSazonov commented Apr 21, 2017

Uh oh!

daxian-dbw commented Apr 21, 2017

Uh oh!

iSazonov commented Apr 21, 2017

Uh oh!

daxian-dbw commented Apr 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iSazonov commented Apr 22, 2017

Uh oh!

daxian-dbw commented Apr 24, 2017

Uh oh!

iSazonov commented Apr 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iSazonov commented Apr 25, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daxian-dbw commented Apr 25, 2017

Uh oh!

iSazonov commented Apr 27, 2017

Uh oh!

mirichmo commented May 3, 2017

Uh oh!

daxian-dbw commented May 12, 2017

Uh oh!

mirichmo commented May 12, 2017

Uh oh!

iSazonov commented May 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

iSazonov commented Apr 19, 2017 •

edited by daxian-dbw

Loading

iSazonov commented Apr 20, 2017 •

edited

Loading

daxian-dbw commented Apr 21, 2017 •

edited

Loading

iSazonov commented Apr 25, 2017 •

edited

Loading

iSazonov commented May 15, 2017 •

edited

Loading

iSazonov commented May 16, 2017 •

edited

Loading