GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Add support for 'cd +' by bergmeister · Pull Request #7206 · PowerShell/PowerShell

Conversation

Christoph Bergmeister (bergmeister) commented Jun 28, 2018 •

edited by iSazonov

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

PR Summary

Closes #7189

cd + adds the ability to revert cd - by having another bounded stack with the same limit for 'redo' actions.
Behaviour is similar to back/forward navigation in explorer.exe
When the location is set to a path and the redo stack is non-empty then the redo stack gets flushed.
When cd + happens, then the redo stack is pushed to as well for a more intuitive way of navigating backwards and forwards.
All the work is done of course on the Set-location cmdlet, which is aliased to cd on all platforms.

PR Checklist

PR has a meaningful title
- Use the present tense and imperative mood when describing your changes
Summarized changes
Change is not breaking
Make sure all .h, .cpp, .cs, .ps1 and .psm1 files have the correct copyright header
This PR is ready to merge and is not Work in Progress.
- If the PR is work in progress, please add the prefix WIP: to the beginning of the title and remove the prefix when the PR is ready.
User-facing changes
- Not Applicable
- OR
- User-facing Documentation needed
  - Issue filed - Issue link: We should add this into the existing issue for cd - here: 'cd -' capability added in PowerShell 6.1 MicrosoftDocs/PowerShell-Docs#2476
Testing - New and feature
- Not Applicable or can only be tested interactively
- OR
- Make sure you've added a new test if existing tests do not effectively test the code changed
  - Add [feature] if the change is significant or affects feature tests

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

iSazonov reacted with thumbs up emoji

All reactions

👍 1 reaction

Christoph Bergmeister (bergmeister) added 8 commits

June 28, 2018 00:11


          Simple (not working yet) prototype

5aedb47


          first working prototype

30b0149


          Merge branch 'master' of https://github.com/PowerShell/PowerShell int…

b82ae2c

…o CdPlus


          Merge branch 'master' of https://github.com/PowerShell/PowerShell int…

d94218d

…o CdPlus


          Limit Redo count. TODO: empty redo stack when performing certain actions

f422ce2


          add Pester tests

f668215


          Tweak behaviour by invalidating the redo stack and making a cd + push…

a1672f6

… the stack as well for more intuitive ways of going back


          remove comment as it is clear now why the expected behaviour is like …

806665e

…that

Christoph Bergmeister (bergmeister) requested review from Bruce Payette (BrucePay) and Dongbo Wang (daxian-dbw) as code owners

June 28, 2018 22:27


          address space/newline CodeFactor issues and add comment

82f9594

Christoph Bergmeister (bergmeister) changed the title ~~WIP: Add support for 'cd +'~~ Add support for 'cd +'

Jun 28, 2018

Ilya (iSazonov) reviewed

Jun 29, 2018

View reviewed changes

Comment thread

src/System.Management.Automation/engine/SessionStateLocationAPIs.cs Outdated

		}

		// Replace path with last working directory from redo stack and push to the undo stack when '+' was passed.
		if (originalPath.Equals("+", StringComparison.OrdinalIgnoreCase))

Ilya (iSazonov) Jun 29, 2018

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

We could use switch with C# 7.0 syntax and exclude pushNextLocation:

switch (originalPath)
{
    case var path when path.Equals("-", StringComparison.OrdinalIgnoreCase): 
        if (_SetLocationHistory.UndoCount <= 0) 
        { 
            throw new InvalidOperationException(SessionStateStrings.SetContentToLastLocationWhenHistoryUndoStackIsEmpty); 
        } 

        var previousLocation =  _SetLocationHistory.Undo(this.CurrentLocation); 
        path = previousLocation.Path; 
        break;
    case var path when path.Equals("+", StringComparison.OrdinalIgnoreCase);
        if (_SetLocationHistory.RedoCount <= 0) 
        { 
            throw new InvalidOperationException(SessionStateStrings.SetContentToLastLocationWhenHistoryRedoStackIsEmpty); 
        } 

        var previousLocation = _SetLocationHistory.Redo(); 
        _SetLocationHistory.Push(this.CurrentLocation); 
        path = previousLocation.Path; 
        break;
    default:
        var newPushPathInfo = GetNewPushPathInfo(); 
        _SetLocationHistory.Push(newPushPathInfo); 

        if (_SetLocationHistory.RedoCount >= 0) 
        { 
            _SetLocationHistory.InvalidateRedoStack(); 
        } 

        break;
}

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bergmeister reacted with thumbs up emoji

All reactions

👍 1 reaction

Christoph Bergmeister (bergmeister) Jun 29, 2018 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Yes, we could. I will take a note of this but would prefer to address style issues like this only at the end once the design/behaviour has been reviewed/approved.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

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

		{
		throw new InvalidOperationException(SessionStateStrings.BoundedStackIsEmpty);
		}
		var item = this.First.Value;

Ilya (iSazonov) Jun 29, 2018

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Please add new line after } (CodeFactor issue).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Christoph Bergmeister (bergmeister) Jun 29, 2018

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I disagree, in order to improve readability I have put the code for cd - and cd +` into 2 blocks that are separated by a newline. This would make it worse, we should not be a slave of all rules, I question the usefulness of this rule. I agree that it can be useful to have a newline after a big block but not after a one-liner and not having braces can be dangerous as well.
I suggest that I could refactor the 2 blocks into a private method of their own instead, what do you think?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Ilya (iSazonov) Jun 29, 2018

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Christoph Bergmeister (@bergmeister) I don't understand why you ask about private method. CodeFactor issue is to add new line after 1552 and before 1553.
There are not many such issues in our code reported by CodeFactor. In other words, it is a style that we always follow.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

rkeithhill reacted with thumbs up emoji

All reactions

👍 1 reaction

Christoph Bergmeister (bergmeister) Jun 29, 2018 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Sorry, I thought your comment was on the other place in the SessionStateLocationAPIs class, my bad.
The idea for the SessionStateLocationAPIs class was to put the 2 code blocks into a private method to avoid making the main method even longer.
Nevertheless, I still find the rule odd for a brace that has only one line in it but I will have to comply with whatever is preferred at the end anyway.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Ilya (iSazonov) Jun 29, 2018

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

No one happy in this situation (style issues) but at least this tool always gives the same result as opposed to human work.
Now the CodeFactor settings are such that we get a minimum of requests for each PR. I hope in a few months we will be able to escape most annoying style issues.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bergmeister reacted with thumbs up emoji

All reactions

👍 1 reaction

Christoph Bergmeister (bergmeister) added 9 commits

June 29, 2018 21:33


          Merge branch 'master' of https://github.com/PowerShell/PowerShell int…

94f3124

…o CdPlus


          address PR feedback (style issues and use switch statement

ef781a6


          make whitespace consistent

b015dda


          Add XUnit test for HistoryStack and use uint for location history to …

287a0e0

…not allow negative values


          Add Xunit test for BoundedStack

ad3c184


          Add more assertions about returned objects in XUnit tests

ee91d9c


          fix codefactor issues in XUnit tests

c2583b2


          Simplify implementation, make Undo/Redo symmetric and move the core l…

612a882

…ogic to the HistoryStack class.


          tweak comment

7e4fd43

Ilya (iSazonov) reviewed

Jun 30, 2018

View reviewed changes

Ilya (iSazonov) left a comment

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

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

		internal class BoundedStack<T> : LinkedList<T>
		{
		private readonly int _capacity;
		private readonly uint _capacity;

Ilya (iSazonov) Jun 30, 2018

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I wonder - how do this related to the PR?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Christoph Bergmeister (bergmeister) Jun 30, 2018 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

As I was writing the XUnit unit tests for the 2 classes, I was thinking about the case of someone passing in a negative number, hence why I made it unsigned to get that for free. We could even make it an ushort but with modern processor architecture and runtimes, there is usually no speed advantage any more.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

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

		}
		}

		internal class HistoryStack<T>

Ilya (iSazonov) Jun 30, 2018

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

BoundedStack<T> below has a public doc comment. I expect we add a public comment here too with an explanation why we use it and what is the difference from BoundedStack<T>.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

src/System.Management.Automation/resources/SessionStateStrings.resx Outdated

		<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">
		<data name="SetContentToLastLocationWhenHistoryUndoStackIsEmpty" xml:space="preserve">

Ilya (iSazonov) Jun 30, 2018

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Could we use more short name? UndoStackIsEmpty? SetContentUndoStackIsEmpty?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

src/System.Management.Automation/resources/SessionStateStrings.resx Outdated

		<data name="SetContentToLastLocationWhenHistoryUndoStackIsEmpty" xml:space="preserve">
		<value>There is no location history left to navigate backwards.</value>
		</data>
		<data name="SetContentToLastLocationWhenHistoryRedoStackIsEmpty" xml:space="preserve">

Ilya (iSazonov) Jun 30, 2018

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The same - very long name.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Christoph Bergmeister (bergmeister) added 2 commits

June 30, 2018 23:35


          Address PR style comments

bb9af07


          remove unnecessary parenthesis

4854f55

stale Bot commented Jul 30, 2018

Copy link

Copy Markdown

This PR has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed if no further activity occurs within 10 days.
Thank you for your contributions.
Community members are welcome to grab these works.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

stale Bot added the Stale label

Jul 30, 2018

Christoph Bergmeister (bergmeister) commented Jul 31, 2018 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor Author

Feedback was addressed and the PR got stale again similar to its previous cd - PR that took 9 months...

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

stale Bot removed the Stale label

Jul 31, 2018

Ilya (iSazonov) commented Jul 31, 2018 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Collaborator

Christoph Bergmeister (@bergmeister) Sorry, MSFT team is hard working on porting Windows modules. I suppose that most of the open PRs will be reviewed only after 6.1 release (~2-3 weeks).

bergmeister reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Andrew (anmenaga) commented Aug 2, 2018

Copy link

Copy Markdown

Christoph Bergmeister (@bergmeister) Please add a commit with [feature] prefix to run a broad set of tests on this change.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Christoph Bergmeister (bergmeister) added 2 commits

August 2, 2018 22:58


          Merge branch 'master' into CdPlus

8a83c45


          [feature]

2ce06dd

Christoph Bergmeister (bergmeister) commented Aug 2, 2018

Copy link

Copy Markdown

Contributor Author

Andrew (@anmenaga) Done and it is still green.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Rob Holt (rjmholt) commented Aug 9, 2018

Copy link

Copy Markdown

Collaborator

Christoph Bergmeister (@bergmeister) Ran into a need for cd - the other day and just wanted to say thanks!

bergmeister reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Rob Holt (rjmholt) reviewed

Aug 9, 2018

View reviewed changes

Comment thread

src/System.Management.Automation/engine/SessionStateLocationAPIs.cs Outdated

		{
		var newPushPathInfo = GetNewPushPathInfo();
		_SetLocationHistory.Push(newPushPathInfo);
		case var originalPathSwitch when originalPathSwitch.Equals("-", StringComparison.OrdinalIgnoreCase):

Rob Holt (rjmholt) Aug 9, 2018

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I would use the type of originalPathSwitch here instead of var, just so it's clear. Same below.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Christoph Bergmeister (bergmeister) Sep 2, 2018

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Ok, addressed (type is string)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

src/System.Management.Automation/engine/SessionStateLocationAPIs.cs Outdated

		path = _SetLocationHistory.Redo(this.CurrentLocation).Path;
		break;
		default:
		var newPushPathInfo = GetNewPushPathInfo();

Rob Holt (rjmholt) Aug 9, 2018

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Since it's not clear on this line what type the method returns, I would prefer not to use var here.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Christoph Bergmeister (bergmeister) Sep 2, 2018

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Ok, I renamed it to pushPathInfo instead, which is the name of the type here. which seemed to be the better compromise after looking at it again (otherwise the word pushpathInfo would've been repeated 3 times in one line)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

src/System.Management.Automation/engine/Utils.cs Outdated

		/// </summary>
		internal class HistoryStack<T>
		{
		private BoundedStack<T> _boundedUndoStack;

Rob Holt (rjmholt) Aug 9, 2018

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

If this are never reassigned they could be made readonly.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Christoph Bergmeister (bergmeister) Sep 2, 2018

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Ok, addressed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

src/System.Management.Automation/engine/Utils.cs Outdated


		internal T Undo(T currentItem)
		{
		var previousItem = _boundedUndoStack.Pop();

Rob Holt (rjmholt) Aug 9, 2018

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

T is shorter and more explicit than var here

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Christoph Bergmeister (bergmeister) Sep 2, 2018

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Ok, addressed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

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

		}
		}

		internal T Undo(T currentItem)

Rob Holt (rjmholt) Aug 9, 2018

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Worth documenting the methods here, mainly since Undo and Redo are not entirely self-explanatory

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Christoph Bergmeister (bergmeister) Sep 2, 2018

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Added comment explaining the level stashing and popping.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

rjmholt reacted with thumbs up emoji

All reactions

👍 1 reaction

Christoph Bergmeister (bergmeister) added 3 commits

September 2, 2018 18:13


          Merge branch 'master' of https://github.com/PowerShell/PowerShell int…

be4fdc2

…o CdPlus


          Merge branch 'CdPlus' of https://github.com/bergmeister/PowerShell in…

09e50f3

…to CdPlus


          Address all comments on PR PowerShell#7206 by @rjmholt

3fce0e6

Rob Holt (rjmholt) approved these changes

Sep 2, 2018

View reviewed changes

Rob Holt (rjmholt) left a comment

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Ilya (iSazonov) reviewed

Sep 3, 2018

View reviewed changes

Comment thread

src/System.Management.Automation/engine/SessionStateLocationAPIs.cs Outdated

		var newPushPathInfo = GetNewPushPathInfo();
		_SetLocationHistory.Push(newPushPathInfo);
		case string originalPathSwitch when originalPathSwitch.Equals("-", StringComparison.OrdinalIgnoreCase):
		if (_SetLocationHistory.UndoCount <= 0)

Ilya (iSazonov) Sep 3, 2018

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I wonder that it is nit _setLocationHistory.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

src/System.Management.Automation/engine/SessionStateLocationAPIs.cs Outdated

		/// Location history for Set-Location that supports Undo/Redo using bounded stacks.
		/// </summary>
		private BoundedStack<PathInfo> _SetLocationHistory;
		private HistoryStack<PathInfo> _SetLocationHistory;

Ilya (iSazonov) Sep 3, 2018

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Please use _setLocationHistory for private.
And seems it is private readonly.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

src/System.Management.Automation/engine/Utils.cs Outdated

		/// Handles bounded history stacks by pushing the current item to the redoStack and returning the item from the popped undoStack.
		/// </summary>
		/// <param name="currentItem"></param>
		/// <returns></returns>

Ilya (iSazonov) Sep 3, 2018

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Please add a value comment or remove the empty tag.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

src/System.Management.Automation/engine/Utils.cs Outdated

		/// Handles bounded history stacks by pushing the current item to the undoStack and returning the item from the popped redoStack.
		/// </summary>
		/// <param name="currentItem"></param>
		/// <returns></returns>

Ilya (iSazonov) Sep 3, 2018

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The same.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

test/powershell/Modules/Microsoft.PowerShell.Management/Set-Location.Tests.ps1 Outdated

		}

		It 'Should go to last location back, forth and back again when specifying minus, plus and minus as a path' {
		$initialLocation = Get-Location

Ilya (iSazonov) Sep 3, 2018

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

What about $initialPath = (Get-Location).Path ? (See next test.)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Address PR comments by @iSazonov: rename of private variable for more…

44bad7e

… consistency, make member variable readonly, removing redundant comic and minor test syntax improvement

Ilya (iSazonov) approved these changes

Sep 4, 2018

View reviewed changes

Christoph Bergmeister (bergmeister) added 2 commits

September 7, 2018 22:42


          Merge branch 'master' of https://github.com/PowerShell/PowerShell int…

0cada08

…o CdPlus


          Merge branch 'master' of https://github.com/PowerShell/PowerShell int…

a561498

…o CdPlus

Dongbo Wang (daxian-dbw) assigned Ilya (iSazonov)

Sep 18, 2018

Dongbo Wang (daxian-dbw) commented Sep 18, 2018

Copy link

Copy Markdown

Member

Ilya (@iSazonov) Since Rob Holt (@rjmholt) and you have approved, feel free to merge as you see appropriate. Thanks!

bergmeister reacted with hooray emoji

All reactions

🎉 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Andrew (anmenaga) assigned Aditya Patwardhan (adityapatwardhan)

Sep 18, 2018

Andrew (anmenaga) commented Sep 18, 2018

Copy link

Copy Markdown

Ilya (@iSazonov) Aditya Patwardhan (@adityapatwardhan) please merge this when you get a chance.

bergmeister reacted with thumbs up emoji

bergmeister reacted with hooray emoji

All reactions

👍 1 reaction
🎉 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Ilya (iSazonov) merged commit 4bc22d9 into PowerShell:master

Sep 19, 2018

Ilya (iSazonov) commented Sep 19, 2018

Copy link

Copy Markdown

Collaborator

Christoph Bergmeister (@bergmeister) Thanks for your contribution!

Sorry for delay - I did wait 6.1 GA when MSFT team get a time to review.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

PRASOON KARUNAN V (kvprasoon) commented Sep 19, 2018

Copy link

Copy Markdown

Contributor

Ilya (@iSazonov) There is a merge conflict in sessionstatestring.resx for #7797 after this commit.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Ilya (iSazonov) commented Sep 19, 2018

Copy link

Copy Markdown

Collaborator

PRASOON KARUNAN V (@kvprasoon) This is a common situation. You should resolve the merge conflict in #7797.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Steve Lee (SteveL-MSFT) mentioned this pull request

Oct 20, 2018

Enable Set-Location -LiteralPath to work with folders named "-" and "+" #8089

Merged

7 of 11 tasks

Rain Sallow (/u/ta11ow) (vexx32) mentioned this pull request

Nov 21, 2019

cd - (switch directories) doesn't work as expected. #11145

Closed

Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request

Aug 6, 2026


          Add support for 'cd +' (PowerShell#7206)

86e21ab

'cd +' adds the ability to revert 'cd -' by having another bounded stack with the same limit for 'redo' actions.
Behaviour is similar to back/forward navigation in explorer.exe
When the location is set to a path and the redo stack is non-empty then the redo stack gets flushed.
When cd + happens, then the redo stack is pushed to as well for a more intuitive way of navigating backwards and forwards.
All the work is done of course on the Set-location cmdlet, which is aliased to cd on all platforms.

Reviewers

Ilya (iSazonov)

iSazonov approved these changes

Bruce Payette (BrucePay)

Awaiting requested review from BrucePay

Dongbo Wang (daxian-dbw)

Awaiting requested review from daxian-dbw

+1 more reviewer

Rob Holt (rjmholt)

rjmholt approved these changes

Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

Aditya Patwardhan (adityapatwardhan)

Ilya (iSazonov)

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add support for 'cd +'

Uh oh!

There was an error while loading. Please reload this page.

7 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

Conversation

Christoph Bergmeister (bergmeister) commented Jun 28, 2018 • edited by iSazonov Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Christoph Bergmeister (bergmeister) Jun 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Christoph Bergmeister (bergmeister) Jun 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ilya (iSazonov) left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Christoph Bergmeister (bergmeister) Jun 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

stale Bot commented Jul 30, 2018

Uh oh!

Christoph Bergmeister (bergmeister) commented Jul 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Ilya (iSazonov) commented Jul 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Andrew (anmenaga) commented Aug 2, 2018

Uh oh!

Christoph Bergmeister (bergmeister) commented Aug 2, 2018

Uh oh!

Rob Holt (rjmholt) commented Aug 9, 2018

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!

Rob Holt (rjmholt) left a comment

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!

Christoph Bergmeister (bergmeister) commented Jun 28, 2018 •

edited by iSazonov

Loading

Uh oh!

There was an error while loading. Please reload this page.

Christoph Bergmeister (bergmeister) Jun 29, 2018 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Christoph Bergmeister (bergmeister) Jun 29, 2018 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Christoph Bergmeister (bergmeister) Jun 30, 2018 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Christoph Bergmeister (bergmeister) commented Jul 31, 2018 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Ilya (iSazonov) commented Jul 31, 2018 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.