From 5aedb472b4807322842ac53e70d387295ca359c2 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Thu, 28 Jun 2018 00:11:32 +0100 Subject: [PATCH 01/20] Simple (not working yet) prototype --- .../engine/SessionState.cs | 2 +- .../engine/SessionStateLocationAPIs.cs | 14 ++++++- .../engine/Utils.cs | 39 ++++++++++++++++++- 3 files changed, 51 insertions(+), 4 deletions(-) diff --git a/src/System.Management.Automation/engine/SessionState.cs b/src/System.Management.Automation/engine/SessionState.cs index 516828c743d..c6e2aecdb3e 100644 --- a/src/System.Management.Automation/engine/SessionState.cs +++ b/src/System.Management.Automation/engine/SessionState.cs @@ -69,7 +69,7 @@ internal SessionStateInternal(SessionStateInternal parent, bool linkToGlobal, Ex // Conservative choice to limit the Set-Location history in order to limit memory impact in case of a regression. const int locationHistoryLimit = 20; - _SetLocationHistory = new BoundedStack(locationHistoryLimit); + _SetLocationHistory = new HistoryStack(locationHistoryLimit); GlobalScope = new SessionStateScope(null); ModuleScope = GlobalScope; diff --git a/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs b/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs index 60f9f8affa1..50bb74edb3e 100644 --- a/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs +++ b/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs @@ -239,7 +239,17 @@ internal PathInfo SetLocation(string path, CmdletProviderContext context) { throw new InvalidOperationException(SessionStateStrings.SetContentToLastLocationWhenHistoryIsEmpty); } - var previousLocation = _SetLocationHistory.Pop(); + var previousLocation = _SetLocationHistory.Undo(); + path = previousLocation.Path; + pushNextLocation = false; + } + if (originalPath.Equals("+", StringComparison.OrdinalIgnoreCase)) + { + if (_SetLocationHistory.Count <= 0) + { + throw new InvalidOperationException(SessionStateStrings.SetContentToLastLocationWhenHistoryIsEmpty); + } + var previousLocation = _SetLocationHistory.Redo(); path = previousLocation.Path; pushNextLocation = false; } @@ -805,7 +815,7 @@ CmdletProviderContext normalizePathContext /// /// A bounded stack for the location history of Set-Location /// - private BoundedStack _SetLocationHistory; + private HistoryStack _SetLocationHistory; /// /// A stack of the most recently pushed locations diff --git a/src/System.Management.Automation/engine/Utils.cs b/src/System.Management.Automation/engine/Utils.cs index 2cc4b9602cc..ed6937f434b 100644 --- a/src/System.Management.Automation/engine/Utils.cs +++ b/src/System.Management.Automation/engine/Utils.cs @@ -1460,8 +1460,45 @@ public static void SetTestHook(string property, object value) } } + internal class HistoryStack + { + private BoundedStack _boundedUndoStack; + private BoundedStack _boundedRedoStack; + + internal HistoryStack(int capacity) + { + _boundedUndoStack = new BoundedStack(capacity); + _boundedRedoStack = new BoundedStack(capacity); + } + + internal void Push(T item) + { + _boundedUndoStack.Push(item); + } + + internal T Undo() + { + var item = _boundedUndoStack.Pop(); + _boundedRedoStack.Push(item); + return item; + } + + internal T Redo() + { + return _boundedRedoStack.Pop(); + } + + internal int Count + { + get + { + return _boundedUndoStack.Count; + } + } + } + /// - /// An bounded stack based on a linked list. + /// A bounded stack based on a linked list. /// internal class BoundedStack : LinkedList { From 30b0149c71ccd992313baa720999a81fa5c01171 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Thu, 28 Jun 2018 00:40:13 +0100 Subject: [PATCH 02/20] first working prototype --- .../engine/SessionStateLocationAPIs.cs | 2 +- src/System.Management.Automation/engine/Utils.cs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs b/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs index 50bb74edb3e..8a50fb34971 100644 --- a/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs +++ b/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs @@ -239,7 +239,7 @@ internal PathInfo SetLocation(string path, CmdletProviderContext context) { throw new InvalidOperationException(SessionStateStrings.SetContentToLastLocationWhenHistoryIsEmpty); } - var previousLocation = _SetLocationHistory.Undo(); + var previousLocation = _SetLocationHistory.Undo(this.CurrentLocation); path = previousLocation.Path; pushNextLocation = false; } diff --git a/src/System.Management.Automation/engine/Utils.cs b/src/System.Management.Automation/engine/Utils.cs index ed6937f434b..539dcc4b190 100644 --- a/src/System.Management.Automation/engine/Utils.cs +++ b/src/System.Management.Automation/engine/Utils.cs @@ -1476,10 +1476,10 @@ internal void Push(T item) _boundedUndoStack.Push(item); } - internal T Undo() + internal T Undo(T currentItem) { var item = _boundedUndoStack.Pop(); - _boundedRedoStack.Push(item); + _boundedRedoStack.Push(currentItem); return item; } From f422ce291773feedebef2a9b07001ceb1f931693 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Thu, 28 Jun 2018 22:09:36 +0100 Subject: [PATCH 03/20] Limit Redo count. TODO: empty redo stack when performing certain actions --- .../engine/SessionStateLocationAPIs.cs | 8 ++++---- src/System.Management.Automation/engine/Utils.cs | 14 +++++++++++++- .../resources/SessionStateStrings.resx | 5 ++++- 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs b/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs index 8a50fb34971..d2bb1a8d26a 100644 --- a/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs +++ b/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs @@ -235,9 +235,9 @@ internal PathInfo SetLocation(string path, CmdletProviderContext context) bool pushNextLocation = true; if (originalPath.Equals("-", StringComparison.OrdinalIgnoreCase)) { - if (_SetLocationHistory.Count <= 0) + if (_SetLocationHistory.UndoCount <= 0) { - throw new InvalidOperationException(SessionStateStrings.SetContentToLastLocationWhenHistoryIsEmpty); + throw new InvalidOperationException(SessionStateStrings.SetContentToLastLocationWhenHistoryUndoStackIsEmpty); } var previousLocation = _SetLocationHistory.Undo(this.CurrentLocation); path = previousLocation.Path; @@ -245,9 +245,9 @@ internal PathInfo SetLocation(string path, CmdletProviderContext context) } if (originalPath.Equals("+", StringComparison.OrdinalIgnoreCase)) { - if (_SetLocationHistory.Count <= 0) + if (_SetLocationHistory.RedoCount <= 0) { - throw new InvalidOperationException(SessionStateStrings.SetContentToLastLocationWhenHistoryIsEmpty); + throw new InvalidOperationException(SessionStateStrings.SetContentToLastLocationWhenHistoryRedoStackIsEmpty); } var previousLocation = _SetLocationHistory.Redo(); path = previousLocation.Path; diff --git a/src/System.Management.Automation/engine/Utils.cs b/src/System.Management.Automation/engine/Utils.cs index 539dcc4b190..f74d3365711 100644 --- a/src/System.Management.Automation/engine/Utils.cs +++ b/src/System.Management.Automation/engine/Utils.cs @@ -1488,13 +1488,21 @@ internal T Redo() return _boundedRedoStack.Pop(); } - internal int Count + internal int UndoCount { get { return _boundedUndoStack.Count; } } + + internal int RedoCount + { + get + { + return _boundedRedoStack.Count; + } + } } /// @@ -1533,6 +1541,10 @@ internal void Push(T item) /// internal T Pop() { + if (this.First == null) + { + throw new InvalidOperationException(SessionStateStrings.BoundedStackIsEmpty); + } var item = this.First.Value; try { diff --git a/src/System.Management.Automation/resources/SessionStateStrings.resx b/src/System.Management.Automation/resources/SessionStateStrings.resx index 7fe2a4fee8b..38da181ab0c 100644 --- a/src/System.Management.Automation/resources/SessionStateStrings.resx +++ b/src/System.Management.Automation/resources/SessionStateStrings.resx @@ -279,9 +279,12 @@ The dynamic parameters for the GetContentWriter operation cannot be retrieved for the '{0}' provider for path '{1}'. {2} - + There is no location history left to navigate backwards. + + There is no location history left to navigate forwards. + The BoundedStack is empty. From f6682158110cd5e8bf87827bc9f662aa75ecf103 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Thu, 28 Jun 2018 22:23:55 +0100 Subject: [PATCH 04/20] add Pester tests --- .../Set-Location.Tests.ps1 | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Location.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Location.Tests.ps1 index faff8ae43d1..2f682a99737 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Location.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Location.Tests.ps1 @@ -104,6 +104,16 @@ Describe "Set-Location" -Tags "CI" { (Get-Location).Path | Should -Be ($initialLocation).Path } + It 'Should go to last location and back when specifying minus as a path and then plus' { + $initialLocation = Get-Location + Set-Location ([System.IO.Path]::GetTempPath()) + $tempPath = (Get-Location).Path + Set-Location - + (Get-Location).Path | Should -Be ($initialLocation).Path + Set-Location + + (Get-Location).Path | Should -Be $tempPath + } + It 'Should go back to previous locations when specifying minus twice' { $initialLocation = (Get-Location).Path Set-Location ([System.IO.Path]::GetTempPath()) @@ -121,11 +131,18 @@ Describe "Set-Location" -Tags "CI" { foreach ($i in 1..$maximumLocationHistory) { Set-Location ([System.IO.Path]::GetTempPath()) } + # Go back up to the maximum foreach ($i in 1..$maximumLocationHistory) { Set-Location - } (Get-Location).Path | Should Be $initialLocation { Set-Location - } | Should -Throw -ErrorId 'System.InvalidOperationException,Microsoft.PowerShell.Commands.SetLocationCommand' + # Go forwards up to the maximum + foreach ($i in 1..$maximumLocationHistory) { + Set-Location + + } + (Get-Location).Path | Should -Be $initialLocation + { Set-Location - } | Should -Throw -ErrorId 'System.InvalidOperationException,Microsoft.PowerShell.Commands.SetLocationCommand' } } } From a1672f6ba94eadd0544103aacfd5533449a6a895 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Thu, 28 Jun 2018 23:15:29 +0100 Subject: [PATCH 05/20] Tweak behaviour by invalidating the redo stack and making a cd + push the stack as well for more intuitive ways of going back --- .../engine/SessionStateLocationAPIs.cs | 5 +++++ src/System.Management.Automation/engine/Utils.cs | 5 +++++ .../Set-Location.Tests.ps1 | 11 +++++++---- 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs b/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs index d2bb1a8d26a..80f3f13729d 100644 --- a/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs +++ b/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs @@ -250,6 +250,7 @@ internal PathInfo SetLocation(string path, CmdletProviderContext context) throw new InvalidOperationException(SessionStateStrings.SetContentToLastLocationWhenHistoryRedoStackIsEmpty); } var previousLocation = _SetLocationHistory.Redo(); + _SetLocationHistory.Push(this.CurrentLocation); path = previousLocation.Path; pushNextLocation = false; } @@ -258,6 +259,10 @@ internal PathInfo SetLocation(string path, CmdletProviderContext context) { var newPushPathInfo = GetNewPushPathInfo(); _SetLocationHistory.Push(newPushPathInfo); + if (_SetLocationHistory.RedoCount >= 0) + { + _SetLocationHistory.InvalidateRedoStack(); + } } PSDriveInfo previousWorkingDrive = CurrentDrive; diff --git a/src/System.Management.Automation/engine/Utils.cs b/src/System.Management.Automation/engine/Utils.cs index f74d3365711..d5cebfa4229 100644 --- a/src/System.Management.Automation/engine/Utils.cs +++ b/src/System.Management.Automation/engine/Utils.cs @@ -1488,6 +1488,11 @@ internal T Redo() return _boundedRedoStack.Pop(); } + internal void InvalidateRedoStack() + { + _boundedRedoStack.Clear(); + } + internal int UndoCount { get diff --git a/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Location.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Location.Tests.ps1 index 2f682a99737..00f87b17c22 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Location.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Location.Tests.ps1 @@ -104,7 +104,7 @@ Describe "Set-Location" -Tags "CI" { (Get-Location).Path | Should -Be ($initialLocation).Path } - It 'Should go to last location and back when specifying minus as a path and then plus' { + It 'Should go to last location back, forth and back again when specifying minus, plus and minus as a path' { $initialLocation = Get-Location Set-Location ([System.IO.Path]::GetTempPath()) $tempPath = (Get-Location).Path @@ -112,6 +112,8 @@ Describe "Set-Location" -Tags "CI" { (Get-Location).Path | Should -Be ($initialLocation).Path Set-Location + (Get-Location).Path | Should -Be $tempPath + Set-Location - + (Get-Location).Path | Should -Be ($initialLocation).Path } It 'Should go back to previous locations when specifying minus twice' { @@ -131,6 +133,7 @@ Describe "Set-Location" -Tags "CI" { foreach ($i in 1..$maximumLocationHistory) { Set-Location ([System.IO.Path]::GetTempPath()) } + $tempPath = (Get-Location).Path # Go back up to the maximum foreach ($i in 1..$maximumLocationHistory) { Set-Location - @@ -138,11 +141,11 @@ Describe "Set-Location" -Tags "CI" { (Get-Location).Path | Should Be $initialLocation { Set-Location - } | Should -Throw -ErrorId 'System.InvalidOperationException,Microsoft.PowerShell.Commands.SetLocationCommand' # Go forwards up to the maximum - foreach ($i in 1..$maximumLocationHistory) { + foreach ($i in 1..($maximumLocationHistory)) { Set-Location + } - (Get-Location).Path | Should -Be $initialLocation - { Set-Location - } | Should -Throw -ErrorId 'System.InvalidOperationException,Microsoft.PowerShell.Commands.SetLocationCommand' + (Get-Location).Path | Should -Be $tempPath # Question: Is this really the expected behaviour? This is at the moment because 'cd +' also pushed the stack + { Set-Location + } | Should -Throw -ErrorId 'System.InvalidOperationException,Microsoft.PowerShell.Commands.SetLocationCommand' } } } From 806665ee97cef5af60031904b43de912e1e0ef18 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Thu, 28 Jun 2018 23:16:54 +0100 Subject: [PATCH 06/20] remove comment as it is clear now why the expected behaviour is like that --- .../Microsoft.PowerShell.Management/Set-Location.Tests.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Location.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Location.Tests.ps1 index 00f87b17c22..b521b58bd61 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Location.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Location.Tests.ps1 @@ -144,7 +144,7 @@ Describe "Set-Location" -Tags "CI" { foreach ($i in 1..($maximumLocationHistory)) { Set-Location + } - (Get-Location).Path | Should -Be $tempPath # Question: Is this really the expected behaviour? This is at the moment because 'cd +' also pushed the stack + (Get-Location).Path | Should -Be $tempPath { Set-Location + } | Should -Throw -ErrorId 'System.InvalidOperationException,Microsoft.PowerShell.Commands.SetLocationCommand' } } From 82f9594bbe4233a6f3769937b0e063b27c3dda18 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Thu, 28 Jun 2018 23:56:35 +0100 Subject: [PATCH 07/20] address space/newline CodeFactor issues and add comment --- .../engine/SessionStateLocationAPIs.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs b/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs index 80f3f13729d..5ecbb21acb9 100644 --- a/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs +++ b/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs @@ -230,9 +230,9 @@ internal PathInfo SetLocation(string path, CmdletProviderContext context) string driveName = null; ProviderInfo provider = null; string providerId = null; + bool pushNextLocation = true; // Replace path with last working directory when '-' was passed. - bool pushNextLocation = true; if (originalPath.Equals("-", StringComparison.OrdinalIgnoreCase)) { if (_SetLocationHistory.UndoCount <= 0) @@ -243,13 +243,15 @@ internal PathInfo SetLocation(string path, CmdletProviderContext context) path = previousLocation.Path; pushNextLocation = false; } + + // Replace path with last working directory from redo stack and push to the undo stack when '+' was passed. if (originalPath.Equals("+", StringComparison.OrdinalIgnoreCase)) { if (_SetLocationHistory.RedoCount <= 0) { throw new InvalidOperationException(SessionStateStrings.SetContentToLastLocationWhenHistoryRedoStackIsEmpty); } - var previousLocation = _SetLocationHistory.Redo(); + var previousLocation = _SetLocationHistory.Redo(); _SetLocationHistory.Push(this.CurrentLocation); path = previousLocation.Path; pushNextLocation = false; From ef781a6cf3f13fa8084c0ec2c76196c364b6c02b Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Fri, 29 Jun 2018 22:13:45 +0100 Subject: [PATCH 08/20] address PR feedback (style issues and use switch statement --- .../engine/SessionStateLocationAPIs.cs | 59 +++++++++---------- .../engine/Utils.cs | 1 + 2 files changed, 28 insertions(+), 32 deletions(-) diff --git a/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs b/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs index 5ecbb21acb9..fa21da81320 100644 --- a/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs +++ b/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs @@ -230,41 +230,36 @@ internal PathInfo SetLocation(string path, CmdletProviderContext context) string driveName = null; ProviderInfo provider = null; string providerId = null; - bool pushNextLocation = true; - // Replace path with last working directory when '-' was passed. - if (originalPath.Equals("-", StringComparison.OrdinalIgnoreCase)) + switch (originalPath) { - if (_SetLocationHistory.UndoCount <= 0) - { - throw new InvalidOperationException(SessionStateStrings.SetContentToLastLocationWhenHistoryUndoStackIsEmpty); - } - var previousLocation = _SetLocationHistory.Undo(this.CurrentLocation); - path = previousLocation.Path; - pushNextLocation = false; - } - - // Replace path with last working directory from redo stack and push to the undo stack when '+' was passed. - if (originalPath.Equals("+", StringComparison.OrdinalIgnoreCase)) - { - if (_SetLocationHistory.RedoCount <= 0) - { - throw new InvalidOperationException(SessionStateStrings.SetContentToLastLocationWhenHistoryRedoStackIsEmpty); - } - var previousLocation = _SetLocationHistory.Redo(); - _SetLocationHistory.Push(this.CurrentLocation); - path = previousLocation.Path; - pushNextLocation = false; - } + case var originalPathSwitch when originalPathSwitch.Equals("-", StringComparison.OrdinalIgnoreCase): + if (_SetLocationHistory.UndoCount <= 0) + { + throw new InvalidOperationException(SessionStateStrings.SetContentToLastLocationWhenHistoryUndoStackIsEmpty); + } + var previousLocation = _SetLocationHistory.Undo(this.CurrentLocation); + path = previousLocation.Path; + break; + case var originalPathSwitch when originalPathSwitch.Equals("+", StringComparison.OrdinalIgnoreCase): + if (_SetLocationHistory.RedoCount <= 0) + { + throw new InvalidOperationException(SessionStateStrings.SetContentToLastLocationWhenHistoryRedoStackIsEmpty); + } + previousLocation = _SetLocationHistory.Redo(); + _SetLocationHistory.Push(this.CurrentLocation); + path = previousLocation.Path; + break; + default: + var newPushPathInfo = GetNewPushPathInfo(); + _SetLocationHistory.Push(newPushPathInfo); + + if (_SetLocationHistory.RedoCount >= 0) + { + _SetLocationHistory.InvalidateRedoStack(); + } - if (pushNextLocation) - { - var newPushPathInfo = GetNewPushPathInfo(); - _SetLocationHistory.Push(newPushPathInfo); - if (_SetLocationHistory.RedoCount >= 0) - { - _SetLocationHistory.InvalidateRedoStack(); - } + break; } PSDriveInfo previousWorkingDrive = CurrentDrive; diff --git a/src/System.Management.Automation/engine/Utils.cs b/src/System.Management.Automation/engine/Utils.cs index d5cebfa4229..ecdb41e2e8c 100644 --- a/src/System.Management.Automation/engine/Utils.cs +++ b/src/System.Management.Automation/engine/Utils.cs @@ -1550,6 +1550,7 @@ internal T Pop() { throw new InvalidOperationException(SessionStateStrings.BoundedStackIsEmpty); } + var item = this.First.Value; try { From b015dda0aeb3808bbdd11b97ba1b68b886071dbd Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Fri, 29 Jun 2018 22:17:31 +0100 Subject: [PATCH 09/20] make whitespace consistent --- .../engine/SessionStateLocationAPIs.cs | 2 +- src/System.Management.Automation/engine/Utils.cs | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs b/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs index fa21da81320..f6841820385 100644 --- a/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs +++ b/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs @@ -863,7 +863,7 @@ internal void PushCurrentLocation(string stackName) private PathInfo GetNewPushPathInfo() { - // Create a new instance of the directory/drive pair + // Create a new instance of the directory/drive pair ProviderInfo provider = CurrentDrive.Provider; string mshQualifiedPath = LocationGlobber.GetMshQualifiedPath(CurrentDrive.CurrentLocation, CurrentDrive); diff --git a/src/System.Management.Automation/engine/Utils.cs b/src/System.Management.Automation/engine/Utils.cs index ecdb41e2e8c..45a5361c33c 100644 --- a/src/System.Management.Automation/engine/Utils.cs +++ b/src/System.Management.Automation/engine/Utils.cs @@ -1473,7 +1473,7 @@ internal HistoryStack(int capacity) internal void Push(T item) { - _boundedUndoStack.Push(item); + _boundedUndoStack.Push(item); } internal T Undo(T currentItem) @@ -1482,7 +1482,7 @@ internal T Undo(T currentItem) _boundedRedoStack.Push(currentItem); return item; } - + internal T Redo() { return _boundedRedoStack.Pop(); @@ -1534,7 +1534,7 @@ internal void Push(T item) { this.AddFirst(item); - if(this.Count > _capacity) + if (this.Count > _capacity) { this.RemoveLast(); } @@ -1550,7 +1550,7 @@ internal T Pop() { throw new InvalidOperationException(SessionStateStrings.BoundedStackIsEmpty); } - + var item = this.First.Value; try { From 287a0e03e600f83cc2507c290ec6d300758b224c Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Fri, 29 Jun 2018 23:41:06 +0100 Subject: [PATCH 10/20] Add XUnit test for HistoryStack and use uint for location history to not allow negative values --- .../engine/SessionState.cs | 2 +- .../engine/Utils.cs | 6 ++--- test/csharp/test_Utils.cs | 27 +++++++++++++++++++ 3 files changed, 31 insertions(+), 4 deletions(-) diff --git a/src/System.Management.Automation/engine/SessionState.cs b/src/System.Management.Automation/engine/SessionState.cs index c6e2aecdb3e..6fd89a0ff9b 100644 --- a/src/System.Management.Automation/engine/SessionState.cs +++ b/src/System.Management.Automation/engine/SessionState.cs @@ -68,7 +68,7 @@ internal SessionStateInternal(SessionStateInternal parent, bool linkToGlobal, Ex _workingLocationStack = new Dictionary>(StringComparer.OrdinalIgnoreCase); // Conservative choice to limit the Set-Location history in order to limit memory impact in case of a regression. - const int locationHistoryLimit = 20; + const uint locationHistoryLimit = 20; _SetLocationHistory = new HistoryStack(locationHistoryLimit); GlobalScope = new SessionStateScope(null); diff --git a/src/System.Management.Automation/engine/Utils.cs b/src/System.Management.Automation/engine/Utils.cs index 45a5361c33c..e7b759a1c2d 100644 --- a/src/System.Management.Automation/engine/Utils.cs +++ b/src/System.Management.Automation/engine/Utils.cs @@ -1465,7 +1465,7 @@ internal class HistoryStack private BoundedStack _boundedUndoStack; private BoundedStack _boundedRedoStack; - internal HistoryStack(int capacity) + internal HistoryStack(uint capacity) { _boundedUndoStack = new BoundedStack(capacity); _boundedRedoStack = new BoundedStack(capacity); @@ -1515,13 +1515,13 @@ internal int RedoCount /// internal class BoundedStack : LinkedList { - private readonly int _capacity; + private readonly uint _capacity; /// /// Lazy initialisation, i.e. it sets only its limit but does not allocate the memory for the given capacity. /// /// - internal BoundedStack(int capacity) + internal BoundedStack(uint capacity) { _capacity = capacity; } diff --git a/test/csharp/test_Utils.cs b/test/csharp/test_Utils.cs index 2bd76217f82..319ed9b58df 100644 --- a/test/csharp/test_Utils.cs +++ b/test/csharp/test_Utils.cs @@ -3,6 +3,7 @@ using Xunit; using System; using System.Management.Automation; +using System.Management.Automation.Internal; using System.Reflection; namespace PSTests.Parallel @@ -15,5 +16,31 @@ public static void TestIsWinPEHost() Skip.IfNot(Platform.IsWindows); Assert.False(Utils.IsWinPEHost()); } + + [Fact] + public static void TestHistoryStack() + { + var historyStack = new HistoryStack(20); + Assert.Equal(0, historyStack.UndoCount); + Assert.Equal(0, historyStack.RedoCount); + + historyStack.Push("first item"); + historyStack.Push("second item"); + Assert.Equal(2, historyStack.UndoCount); + Assert.Equal(0, historyStack.RedoCount); + + historyStack.Undo("second item"); + historyStack.Undo("first item"); + Assert.Equal(0, historyStack.UndoCount); + Assert.Equal(2, historyStack.RedoCount); + + historyStack.Redo(); + Assert.Equal(0, historyStack.UndoCount); + Assert.Equal(1, historyStack.RedoCount); + + historyStack.InvalidateRedoStack(); + Assert.Equal(0, historyStack.UndoCount); + Assert.Equal(0, historyStack.RedoCount); + } } } From ad3c184977b47b75c241eaa43d6649de9063ea5a Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Fri, 29 Jun 2018 23:52:40 +0100 Subject: [PATCH 11/20] Add Xunit test for BoundedStack --- test/csharp/test_Utils.cs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/test/csharp/test_Utils.cs b/test/csharp/test_Utils.cs index 319ed9b58df..d310537796e 100644 --- a/test/csharp/test_Utils.cs +++ b/test/csharp/test_Utils.cs @@ -42,5 +42,23 @@ public static void TestHistoryStack() Assert.Equal(0, historyStack.UndoCount); Assert.Equal(0, historyStack.RedoCount); } + + [Fact] + public static void TestBoundedStack() + { + uint capacity = 20; + var boundedStack = new BoundedStack(capacity); + Assert.Throws(() => boundedStack.Pop()); + + for (int i = 0; i < capacity; i++) + { + boundedStack.Push("item"); + } + for (int i = 0; i < capacity; i++) + { + boundedStack.Pop(); + } + Assert.Throws(() => boundedStack.Pop()); + } } } From ee91d9c545cde514dcf481f4e1460b80ede77817 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Sat, 30 Jun 2018 00:01:40 +0100 Subject: [PATCH 12/20] Add more assertions about returned objects in XUnit tests --- test/csharp/test_Utils.cs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/test/csharp/test_Utils.cs b/test/csharp/test_Utils.cs index d310537796e..b0f3cce0d10 100644 --- a/test/csharp/test_Utils.cs +++ b/test/csharp/test_Utils.cs @@ -29,18 +29,21 @@ public static void TestHistoryStack() Assert.Equal(2, historyStack.UndoCount); Assert.Equal(0, historyStack.RedoCount); - historyStack.Undo("second item"); - historyStack.Undo("first item"); + Assert.Equal("second item", historyStack.Undo("second item")); + Assert.Equal("first item", historyStack.Undo("first item")); Assert.Equal(0, historyStack.UndoCount); Assert.Equal(2, historyStack.RedoCount); - historyStack.Redo(); + Assert.Equal("first item", historyStack.Redo()); Assert.Equal(0, historyStack.UndoCount); Assert.Equal(1, historyStack.RedoCount); historyStack.InvalidateRedoStack(); Assert.Equal(0, historyStack.UndoCount); Assert.Equal(0, historyStack.RedoCount); + + Assert.Throws(() => historyStack.Undo("foo")); + Assert.Throws(() => historyStack.Redo()); } [Fact] @@ -52,11 +55,12 @@ public static void TestBoundedStack() for (int i = 0; i < capacity; i++) { - boundedStack.Push("item"); + boundedStack.Push($"{i}"); } for (int i = 0; i < capacity; i++) { - boundedStack.Pop(); + var poppedItem = boundedStack.Pop(); + Assert.Equal($"{20 - 1 - i}", poppedItem); } Assert.Throws(() => boundedStack.Pop()); } From c2583b22bc65aa240ec72afbc396fc5e5fbeefaf Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Sat, 30 Jun 2018 00:06:55 +0100 Subject: [PATCH 13/20] fix codefactor issues in XUnit tests --- test/csharp/test_Utils.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/csharp/test_Utils.cs b/test/csharp/test_Utils.cs index b0f3cce0d10..c2841921b81 100644 --- a/test/csharp/test_Utils.cs +++ b/test/csharp/test_Utils.cs @@ -57,11 +57,13 @@ public static void TestBoundedStack() { boundedStack.Push($"{i}"); } + for (int i = 0; i < capacity; i++) { var poppedItem = boundedStack.Pop(); Assert.Equal($"{20 - 1 - i}", poppedItem); } + Assert.Throws(() => boundedStack.Pop()); } } From 612a882425f08a5de89817d9cf410de4c340f439 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Sat, 30 Jun 2018 08:55:30 +0100 Subject: [PATCH 14/20] Simplify implementation, make Undo/Redo symmetric and move the core logic to the HistoryStack class. --- .../engine/SessionStateLocationAPIs.cs | 13 ++----- .../engine/Utils.cs | 35 +++++++------------ test/csharp/test_Utils.cs | 17 +++++---- 3 files changed, 25 insertions(+), 40 deletions(-) diff --git a/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs b/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs index f6841820385..a37f5e21cfb 100644 --- a/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs +++ b/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs @@ -238,27 +238,18 @@ internal PathInfo SetLocation(string path, CmdletProviderContext context) { throw new InvalidOperationException(SessionStateStrings.SetContentToLastLocationWhenHistoryUndoStackIsEmpty); } - var previousLocation = _SetLocationHistory.Undo(this.CurrentLocation); - path = previousLocation.Path; + path = (_SetLocationHistory.Undo(this.CurrentLocation)).Path; break; case var originalPathSwitch when originalPathSwitch.Equals("+", StringComparison.OrdinalIgnoreCase): if (_SetLocationHistory.RedoCount <= 0) { throw new InvalidOperationException(SessionStateStrings.SetContentToLastLocationWhenHistoryRedoStackIsEmpty); } - previousLocation = _SetLocationHistory.Redo(); - _SetLocationHistory.Push(this.CurrentLocation); - path = previousLocation.Path; + path = (_SetLocationHistory.Redo(this.CurrentLocation)).Path; break; default: var newPushPathInfo = GetNewPushPathInfo(); _SetLocationHistory.Push(newPushPathInfo); - - if (_SetLocationHistory.RedoCount >= 0) - { - _SetLocationHistory.InvalidateRedoStack(); - } - break; } diff --git a/src/System.Management.Automation/engine/Utils.cs b/src/System.Management.Automation/engine/Utils.cs index e7b759a1c2d..a0cb1a714b4 100644 --- a/src/System.Management.Automation/engine/Utils.cs +++ b/src/System.Management.Automation/engine/Utils.cs @@ -1474,40 +1474,29 @@ internal HistoryStack(uint capacity) internal void Push(T item) { _boundedUndoStack.Push(item); + if (RedoCount >= 0) + { + _boundedRedoStack.Clear(); + } } internal T Undo(T currentItem) { - var item = _boundedUndoStack.Pop(); + var previousItem = _boundedUndoStack.Pop(); _boundedRedoStack.Push(currentItem); - return item; + return previousItem; } - internal T Redo() + internal T Redo(T currentItem) { - return _boundedRedoStack.Pop(); + var nextItem = _boundedRedoStack.Pop(); + _boundedUndoStack.Push(currentItem); + return nextItem; } - internal void InvalidateRedoStack() - { - _boundedRedoStack.Clear(); - } - - internal int UndoCount - { - get - { - return _boundedUndoStack.Count; - } - } + internal int UndoCount => _boundedUndoStack.Count; - internal int RedoCount - { - get - { - return _boundedRedoStack.Count; - } - } + internal int RedoCount => _boundedRedoStack.Count; } /// diff --git a/test/csharp/test_Utils.cs b/test/csharp/test_Utils.cs index c2841921b81..2ac7f81b508 100644 --- a/test/csharp/test_Utils.cs +++ b/test/csharp/test_Utils.cs @@ -34,16 +34,21 @@ public static void TestHistoryStack() Assert.Equal(0, historyStack.UndoCount); Assert.Equal(2, historyStack.RedoCount); - Assert.Equal("first item", historyStack.Redo()); - Assert.Equal(0, historyStack.UndoCount); + Assert.Equal("first item", historyStack.Redo("first item")); + Assert.Equal(1, historyStack.UndoCount); Assert.Equal(1, historyStack.RedoCount); - historyStack.InvalidateRedoStack(); - Assert.Equal(0, historyStack.UndoCount); + // Pushing a new item should invalidate the RedoCount + historyStack.Push("third item"); + Assert.Equal(2, historyStack.UndoCount); Assert.Equal(0, historyStack.RedoCount); + // Check for the correct exception when the Redo/Undo stack is empty. + Assert.Throws(() => historyStack.Redo("bar")); + historyStack.Undo("third item"); + historyStack.Undo("first item"); + Assert.Equal(0, historyStack.UndoCount); Assert.Throws(() => historyStack.Undo("foo")); - Assert.Throws(() => historyStack.Redo()); } [Fact] @@ -63,7 +68,7 @@ public static void TestBoundedStack() var poppedItem = boundedStack.Pop(); Assert.Equal($"{20 - 1 - i}", poppedItem); } - + Assert.Throws(() => boundedStack.Pop()); } } From 7e4fd433d525e4b74e2166b2b3e4eaa482c05766 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Sat, 30 Jun 2018 08:58:42 +0100 Subject: [PATCH 15/20] tweak comment --- .../engine/SessionStateLocationAPIs.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs b/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs index a37f5e21cfb..c3007043ef3 100644 --- a/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs +++ b/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs @@ -806,7 +806,7 @@ CmdletProviderContext normalizePathContext #region push-Pop current working directory /// - /// A bounded stack for the location history of Set-Location + /// Location history for Set-Location that supports Undo/Redo using bounded stacks. /// private HistoryStack _SetLocationHistory; From bb9af07f44e0eda6b5d10505dddaf30281ffe717 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Sat, 30 Jun 2018 23:35:58 +0100 Subject: [PATCH 16/20] Address PR style comments --- .../engine/SessionStateLocationAPIs.cs | 4 ++-- src/System.Management.Automation/engine/Utils.cs | 3 +++ .../resources/SessionStateStrings.resx | 4 ++-- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs b/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs index c3007043ef3..5ea308ff0c9 100644 --- a/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs +++ b/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs @@ -236,14 +236,14 @@ internal PathInfo SetLocation(string path, CmdletProviderContext context) case var originalPathSwitch when originalPathSwitch.Equals("-", StringComparison.OrdinalIgnoreCase): if (_SetLocationHistory.UndoCount <= 0) { - throw new InvalidOperationException(SessionStateStrings.SetContentToLastLocationWhenHistoryUndoStackIsEmpty); + throw new InvalidOperationException(SessionStateStrings.LocationUndoStackIsEmpty); } path = (_SetLocationHistory.Undo(this.CurrentLocation)).Path; break; case var originalPathSwitch when originalPathSwitch.Equals("+", StringComparison.OrdinalIgnoreCase): if (_SetLocationHistory.RedoCount <= 0) { - throw new InvalidOperationException(SessionStateStrings.SetContentToLastLocationWhenHistoryRedoStackIsEmpty); + throw new InvalidOperationException(SessionStateStrings.LocationRedoStackIsEmpty); } path = (_SetLocationHistory.Redo(this.CurrentLocation)).Path; break; diff --git a/src/System.Management.Automation/engine/Utils.cs b/src/System.Management.Automation/engine/Utils.cs index a0cb1a714b4..c8747325d03 100644 --- a/src/System.Management.Automation/engine/Utils.cs +++ b/src/System.Management.Automation/engine/Utils.cs @@ -1460,6 +1460,9 @@ public static void SetTestHook(string property, object value) } } + /// + /// Provides undo/redo functionality by using 2 instances of . + /// internal class HistoryStack { private BoundedStack _boundedUndoStack; diff --git a/src/System.Management.Automation/resources/SessionStateStrings.resx b/src/System.Management.Automation/resources/SessionStateStrings.resx index 38da181ab0c..855c7975e30 100644 --- a/src/System.Management.Automation/resources/SessionStateStrings.resx +++ b/src/System.Management.Automation/resources/SessionStateStrings.resx @@ -279,10 +279,10 @@ The dynamic parameters for the GetContentWriter operation cannot be retrieved for the '{0}' provider for path '{1}'. {2} - + There is no location history left to navigate backwards. - + There is no location history left to navigate forwards. From 4854f55e0ba703ba9b93b4a49e8ab728cc92045f Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Sat, 30 Jun 2018 23:39:16 +0100 Subject: [PATCH 17/20] remove unnecessary parenthesis --- .../engine/SessionStateLocationAPIs.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs b/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs index 5ea308ff0c9..70446ae7a74 100644 --- a/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs +++ b/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs @@ -238,14 +238,14 @@ internal PathInfo SetLocation(string path, CmdletProviderContext context) { throw new InvalidOperationException(SessionStateStrings.LocationUndoStackIsEmpty); } - path = (_SetLocationHistory.Undo(this.CurrentLocation)).Path; + path = _SetLocationHistory.Undo(this.CurrentLocation).Path; break; case var originalPathSwitch when originalPathSwitch.Equals("+", StringComparison.OrdinalIgnoreCase): if (_SetLocationHistory.RedoCount <= 0) { throw new InvalidOperationException(SessionStateStrings.LocationRedoStackIsEmpty); } - path = (_SetLocationHistory.Redo(this.CurrentLocation)).Path; + path = _SetLocationHistory.Redo(this.CurrentLocation).Path; break; default: var newPushPathInfo = GetNewPushPathInfo(); From 2ce06dd53cf706989fdaa3134338fcd1fdde5d22 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Thu, 2 Aug 2018 22:59:11 +0100 Subject: [PATCH 18/20] [feature] From 3fce0e6bbb0f5d05f5f3985dbb03b2b392fba2de Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Sun, 2 Sep 2018 18:40:53 +0100 Subject: [PATCH 19/20] Address all comments on PR https://github.com/PowerShell/PowerShell/pull/7206 by @rjmholt --- .../engine/SessionStateLocationAPIs.cs | 12 ++++++------ src/System.Management.Automation/engine/Utils.cs | 16 +++++++++++++--- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs b/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs index 01e0c1496f4..8421473d59f 100644 --- a/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs +++ b/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs @@ -208,14 +208,14 @@ internal PathInfo SetLocation(string path, CmdletProviderContext context) switch (originalPath) { - case var originalPathSwitch when originalPathSwitch.Equals("-", StringComparison.OrdinalIgnoreCase): + case string originalPathSwitch when originalPathSwitch.Equals("-", StringComparison.OrdinalIgnoreCase): if (_SetLocationHistory.UndoCount <= 0) { throw new InvalidOperationException(SessionStateStrings.LocationUndoStackIsEmpty); } path = _SetLocationHistory.Undo(this.CurrentLocation).Path; break; - case var originalPathSwitch when originalPathSwitch.Equals("+", StringComparison.OrdinalIgnoreCase): + case string originalPathSwitch when originalPathSwitch.Equals("+", StringComparison.OrdinalIgnoreCase): if (_SetLocationHistory.RedoCount <= 0) { throw new InvalidOperationException(SessionStateStrings.LocationRedoStackIsEmpty); @@ -223,8 +223,8 @@ internal PathInfo SetLocation(string path, CmdletProviderContext context) path = _SetLocationHistory.Redo(this.CurrentLocation).Path; break; default: - var newPushPathInfo = GetNewPushPathInfo(); - _SetLocationHistory.Push(newPushPathInfo); + var pushPathInfo = GetNewPushPathInfo(); + _SetLocationHistory.Push(pushPathInfo); break; } @@ -819,8 +819,8 @@ internal void PushCurrentLocation(string stackName) } // Push the directory/drive pair onto the stack - var newPushPathInfo = GetNewPushPathInfo(); - locationStack.Push(newPushPathInfo); + var pushPathInfo = GetNewPushPathInfo(); + locationStack.Push(pushPathInfo); } private PathInfo GetNewPushPathInfo() diff --git a/src/System.Management.Automation/engine/Utils.cs b/src/System.Management.Automation/engine/Utils.cs index c48160c8d15..5681ff38023 100644 --- a/src/System.Management.Automation/engine/Utils.cs +++ b/src/System.Management.Automation/engine/Utils.cs @@ -1411,8 +1411,8 @@ public static void SetTestHook(string property, object value) /// internal class HistoryStack { - private BoundedStack _boundedUndoStack; - private BoundedStack _boundedRedoStack; + private readonly BoundedStack _boundedUndoStack; + private readonly BoundedStack _boundedRedoStack; internal HistoryStack(uint capacity) { @@ -1429,13 +1429,23 @@ internal void Push(T item) } } + /// + /// Handles bounded history stacks by pushing the current item to the redoStack and returning the item from the popped undoStack. + /// + /// + /// internal T Undo(T currentItem) { - var previousItem = _boundedUndoStack.Pop(); + T previousItem = _boundedUndoStack.Pop(); _boundedRedoStack.Push(currentItem); return previousItem; } + /// + /// Handles bounded history stacks by pushing the current item to the undoStack and returning the item from the popped redoStack. + /// + /// + /// internal T Redo(T currentItem) { var nextItem = _boundedRedoStack.Pop(); From 44bad7eb610a0d85cf49d300f43066d8ef0f2468 Mon Sep 17 00:00:00 2001 From: Christoph Bergmeister Date: Mon, 3 Sep 2018 23:36:29 +0100 Subject: [PATCH 20/20] Address PR comments by @iSazonov: rename of private variable for more consistency, make member variable readonly, removing redundant comic and minor test syntax improvement --- .../engine/SessionState.cs | 2 +- .../engine/SessionStateLocationAPIs.cs | 12 ++++++------ src/System.Management.Automation/engine/Utils.cs | 4 ---- .../Set-Location.Tests.ps1 | 6 +++--- 4 files changed, 10 insertions(+), 14 deletions(-) diff --git a/src/System.Management.Automation/engine/SessionState.cs b/src/System.Management.Automation/engine/SessionState.cs index 37c161b0eac..ebbbdeef991 100644 --- a/src/System.Management.Automation/engine/SessionState.cs +++ b/src/System.Management.Automation/engine/SessionState.cs @@ -66,7 +66,7 @@ internal SessionStateInternal(SessionStateInternal parent, bool linkToGlobal, Ex // Conservative choice to limit the Set-Location history in order to limit memory impact in case of a regression. const uint locationHistoryLimit = 20; - _SetLocationHistory = new HistoryStack(locationHistoryLimit); + _setLocationHistory = new HistoryStack(locationHistoryLimit); GlobalScope = new SessionStateScope(null); ModuleScope = GlobalScope; diff --git a/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs b/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs index 8421473d59f..fcc850b1fbf 100644 --- a/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs +++ b/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs @@ -209,22 +209,22 @@ internal PathInfo SetLocation(string path, CmdletProviderContext context) switch (originalPath) { case string originalPathSwitch when originalPathSwitch.Equals("-", StringComparison.OrdinalIgnoreCase): - if (_SetLocationHistory.UndoCount <= 0) + if (_setLocationHistory.UndoCount <= 0) { throw new InvalidOperationException(SessionStateStrings.LocationUndoStackIsEmpty); } - path = _SetLocationHistory.Undo(this.CurrentLocation).Path; + path = _setLocationHistory.Undo(this.CurrentLocation).Path; break; case string originalPathSwitch when originalPathSwitch.Equals("+", StringComparison.OrdinalIgnoreCase): - if (_SetLocationHistory.RedoCount <= 0) + if (_setLocationHistory.RedoCount <= 0) { throw new InvalidOperationException(SessionStateStrings.LocationRedoStackIsEmpty); } - path = _SetLocationHistory.Redo(this.CurrentLocation).Path; + path = _setLocationHistory.Redo(this.CurrentLocation).Path; break; default: var pushPathInfo = GetNewPushPathInfo(); - _SetLocationHistory.Push(pushPathInfo); + _setLocationHistory.Push(pushPathInfo); break; } @@ -781,7 +781,7 @@ CmdletProviderContext normalizePathContext /// /// Location history for Set-Location that supports Undo/Redo using bounded stacks. /// - private HistoryStack _SetLocationHistory; + private readonly HistoryStack _setLocationHistory; /// /// A stack of the most recently pushed locations diff --git a/src/System.Management.Automation/engine/Utils.cs b/src/System.Management.Automation/engine/Utils.cs index 5681ff38023..5210755f0bd 100644 --- a/src/System.Management.Automation/engine/Utils.cs +++ b/src/System.Management.Automation/engine/Utils.cs @@ -1432,8 +1432,6 @@ internal void Push(T item) /// /// Handles bounded history stacks by pushing the current item to the redoStack and returning the item from the popped undoStack. /// - /// - /// internal T Undo(T currentItem) { T previousItem = _boundedUndoStack.Pop(); @@ -1444,8 +1442,6 @@ internal T Undo(T currentItem) /// /// Handles bounded history stacks by pushing the current item to the undoStack and returning the item from the popped redoStack. /// - /// - /// internal T Redo(T currentItem) { var nextItem = _boundedRedoStack.Pop(); diff --git a/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Location.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Location.Tests.ps1 index aee329720d1..8228e79bb31 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Location.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Location.Tests.ps1 @@ -121,15 +121,15 @@ Describe "Set-Location" -Tags "CI" { } It 'Should go to last location back, forth and back again when specifying minus, plus and minus as a path' { - $initialLocation = Get-Location + $initialLocation = (Get-Location).Path Set-Location ([System.IO.Path]::GetTempPath()) $tempPath = (Get-Location).Path Set-Location - - (Get-Location).Path | Should -Be ($initialLocation).Path + (Get-Location).Path | Should -Be $initialLocation Set-Location + (Get-Location).Path | Should -Be $tempPath Set-Location - - (Get-Location).Path | Should -Be ($initialLocation).Path + (Get-Location).Path | Should -Be $initialLocation } It 'Should go back to previous locations when specifying minus twice' {