Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
030c93d
Implement location history feature for Set-Location.
bergmeister Oct 6, 2017
10bae7e
Use an existing stack implementation instead of relying on an environ…
bergmeister Oct 10, 2017
61d0c2f
Use a bounded stack with a limit of 1000 and adapt tests.
bergmeister Oct 29, 2017
bdb2b31
Merge branch 'master' of https://github.com/PowerShell/PowerShell int…
bergmeister Oct 30, 2017
552c3ab
Fix test: use get-location explicitly to avoid subtle difference in t…
bergmeister Oct 30, 2017
c9e84e7
Merge branch 'master' of https://github.com/PowerShell/PowerShell int…
bergmeister Nov 13, 2017
0cceb96
Move initialization of _workingLocationHistoryStack into constructor …
bergmeister Nov 14, 2017
c3fad99
Simple syntax fix (C# comments are //, not # as in PowerShell)
bergmeister Nov 15, 2017
527c2a3
Fix 'Location History is limited' test for the new limit, which is 20
bergmeister Nov 15, 2017
61c405b
Merge branch 'master' of https://github.com/PowerShell/PowerShell int…
bergmeister Dec 19, 2017
3780ff8
Merge branch 'master' of https://github.com/PowerShell/PowerShell int…
bergmeister Feb 19, 2018
7115276
Merge branch 'master' of https://github.com/PowerShell/PowerShell int…
bergmeister Apr 1, 2018
08cf9b5
Make new BoundedStack class in engine utils internal
bergmeister Apr 1, 2018
ea7c4ba
Merge branch 'master' of https://github.com/PowerShell/PowerShell int…
bergmeister Apr 25, 2018
4565bfe
Merge branch 'master' of https://github.com/PowerShell/PowerShell int…
bergmeister May 26, 2018
aaa9195
upgrade pester tests to v4 and replace deprecated pester method
bergmeister May 26, 2018
1d6aadc
[feature] Address PR comments by ISazonov about newlines and OrdinalI…
bergmeister May 26, 2018
7dc6c80
Merge branch 'master' of https://github.com/PowerShell/PowerShell int…
bergmeister May 30, 2018
45d75af
[feature]
bergmeister May 30, 2018
96b6334
Merge branch 'master' of https://github.com/bergmeister/PowerShell in…
bergmeister Jun 22, 2018
96af36f
Address PR feedback by @rjmholt and upstream branch
bergmeister Jun 22, 2018
604cd58
Address naming and comment recommendations by @BrucePay
bergmeister Jun 22, 2018
8b06d37
Address last comment by @BrucePay to give more specific error message…
bergmeister Jun 22, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/System.Management.Automation/engine/SessionState.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Diagnostics;
using System.Management.Automation.Internal;
using System.Management.Automation.Runspaces;
using Dbg = System.Management.Automation;
using System.Diagnostics.CodeAnalysis;
Expand Down Expand Up @@ -66,6 +67,10 @@ internal SessionStateInternal(SessionStateInternal parent, bool linkToGlobal, Ex

_workingLocationStack = new Dictionary<String, Stack<PathInfo>>(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;
_SetLocationHistory = new BoundedStack<PathInfo>(locationHistoryLimit);

GlobalScope = new SessionStateScope(null);
ModuleScope = GlobalScope;
_currentScope = GlobalScope;
Expand Down
57 changes: 42 additions & 15 deletions src/System.Management.Automation/engine/SessionStateLocationAPIs.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.IO;
using System.Management.Automation.Internal;
using System.Management.Automation.Provider;
using Dbg = System.Management.Automation;

Expand Down Expand Up @@ -230,10 +231,28 @@ internal PathInfo SetLocation(string path, CmdletProviderContext context)
ProviderInfo provider = null;
string providerId = null;

// Replace path with last working directory when '-' was passed.
bool pushNextLocation = true;
if (originalPath.Equals("-", StringComparison.OrdinalIgnoreCase))
{
if (_SetLocationHistory.Count <= 0)
{
throw new InvalidOperationException(SessionStateStrings.SetContentToLastLocationWhenHistoryIsEmpty);
}
var previousLocation = _SetLocationHistory.Pop();
path = previousLocation.Path;
pushNextLocation = false;
}

if (pushNextLocation)
{
var newPushPathInfo = GetNewPushPathInfo();
_SetLocationHistory.Push(newPushPathInfo);
}

PSDriveInfo previousWorkingDrive = CurrentDrive;

// First check to see if the path is a home path

if (LocationGlobber.IsHomePath(path))
{
path = Globber.GetHomeRelativePath(path);
Expand Down Expand Up @@ -783,6 +802,11 @@ CmdletProviderContext normalizePathContext

#region push-Pop current working directory

/// <summary>
/// A bounded stack for the location history of Set-Location
/// </summary>
private BoundedStack<PathInfo> _SetLocationHistory;

/// <summary>
/// A stack of the most recently pushed locations
/// </summary>
Expand Down Expand Up @@ -811,8 +835,23 @@ internal void PushCurrentLocation(string stackName)
stackName = _defaultStackName;
}

// Create a new instance of the directory/drive pair
// Get the location stack from the hashtable
Stack<PathInfo> locationStack = null;

if (!_workingLocationStack.TryGetValue(stackName, out locationStack))
{
locationStack = new Stack<PathInfo>();
_workingLocationStack[stackName] = locationStack;
}

// Push the directory/drive pair onto the stack
var newPushPathInfo = GetNewPushPathInfo();
locationStack.Push(newPushPathInfo);
}

private PathInfo GetNewPushPathInfo()
{
// Create a new instance of the directory/drive pair
ProviderInfo provider = CurrentDrive.Provider;
string mshQualifiedPath =
LocationGlobber.GetMshQualifiedPath(CurrentDrive.CurrentLocation, CurrentDrive);
Expand All @@ -829,19 +868,7 @@ internal void PushCurrentLocation(string stackName)
CurrentDrive.Name,
mshQualifiedPath);

// Get the location stack from the hashtable

Stack<PathInfo> locationStack = null;

if (!_workingLocationStack.TryGetValue(stackName, out locationStack))
{
locationStack = new Stack<PathInfo>();
_workingLocationStack[stackName] = locationStack;
}

// Push the directory/drive pair onto the stack

locationStack.Push(newPushLocation);
return newPushLocation;
}

/// <summary>
Expand Down
49 changes: 49 additions & 0 deletions src/System.Management.Automation/engine/Utils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1458,4 +1458,53 @@ public static void SetTestHook(string property, object value)
}
}
}

/// <summary>
/// An bounded stack based on a linked list.
/// </summary>
internal class BoundedStack<T> : LinkedList<T>
{
private readonly int _capacity;

/// <summary>
/// Lazy initialisation, i.e. it sets only its limit but does not allocate the memory for the given capacity.
/// </summary>
/// <param name="capacity"></param>
internal BoundedStack(int capacity)
{
_capacity = capacity;
}

/// <summary>
/// Push item.
/// </summary>
/// <param name="item"></param>
internal void Push(T item)
{
this.AddFirst(item);

if(this.Count > _capacity)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe include braces for the if and drop the this from property references. That seems to be the style convention of the file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, I agree. Fixed.

{
this.RemoveLast();
}
}

/// <summary>
/// Pop item.
/// </summary>
/// <returns></returns>
internal T Pop()
{
var item = this.First.Value;
try
{
this.RemoveFirst();
}
catch (InvalidOperationException)
{
throw new InvalidOperationException(SessionStateStrings.BoundedStackIsEmpty);
}
return item;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,12 @@
<data name="GetContentWriterDynamicParametersProviderException" xml:space="preserve">
<value>The dynamic parameters for the GetContentWriter operation cannot be retrieved for the '{0}' provider for path '{1}'. {2}</value>
</data>
<data name="SetContentToLastLocationWhenHistoryIsEmpty" xml:space="preserve">
<value>There is no location history left to navigate backwards.</value>
</data>
<data name="BoundedStackIsEmpty" xml:space="preserve">
<value>The BoundedStack is empty.</value>
</data>
<data name="ClearContentProviderException" xml:space="preserve">
<value>Attempting to perform the ClearContent operation on the '{0}' provider failed for path '{1}'. {2}</value>
</data>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,4 +93,38 @@ Describe "Set-Location" -Tags "CI" {
Remove-PSDrive -Name 'Z'
}
}

Context 'Set-Location with last location history' {

It 'Should go to last location when specifying minus as a path' {
$initialLocation = Get-Location
Set-Location ([System.IO.Path]::GetTempPath())
Set-Location -
(Get-Location).Path | Should -Be ($initialLocation).Path
}

It 'Should go back to previous locations when specifying minus twice' {
$initialLocation = (Get-Location).Path
Set-Location ([System.IO.Path]::GetTempPath())
$firstLocationChange = (Get-Location).Path
Set-Location ([System.Environment]::GetFolderPath("user"))
Set-Location -
(Get-Location).Path | Should -Be $firstLocationChange
Set-Location -
(Get-Location).Path | Should -Be $initialLocation
}

It 'Location History is limited' {
$initialLocation = (Get-Location).Path
$maximumLocationHistory = 20
foreach ($i in 1..$maximumLocationHistory) {
Set-Location ([System.IO.Path]::GetTempPath())
}
foreach ($i in 1..$maximumLocationHistory) {
Set-Location -
}
(Get-Location).Path | Should Be $initialLocation
{ Set-Location - } | Should -Throw -ErrorId 'System.InvalidOperationException,Microsoft.PowerShell.Commands.SetLocationCommand'
}
}
}