tkellogg / Jump-Location

Powershell `cd` that reads your mind
MIT License
467 stars 26 forks source link

Support ~ with the j command. #51

Closed drmohundro closed 9 years ago

drmohundro commented 9 years ago

Shortly after installing Jump-Location, I realized that I couldn't fully replace my cd usage with j because I often use cd ~.

To resolve this, I've changed the j alias to instead be a function, so that I can call Resolve-Path if the path is an actual path instead of a shortcut. This way, commands like j ~ will work.

One drawback to what I've currently got is that only the alias supports this - I'd have to change the actual Set-JumpLocation command otherwise. I didn't know if it was worth it for that or not.

I'm open to thoughts, criticisms, etc. Thanks so much!

vors commented 9 years ago

Great request @drmohundro ! There is a place in source code, that checks does query represents an Absolute path. It's much better to fix this logic to treat '~' as a legal absolute path. Here is place in code: https://github.com/tkellogg/Jump-Location/blob/master/Jump.Location/SetJumpLocationCommand.cs#L68

drmohundro commented 9 years ago

Sounds good @vors - I'll modify the code so the check is there instead. Thanks!

drmohundro commented 9 years ago

See latest commit - I introduced a call to GetUnresolvedProviderPathFromPSPath that tries to resolve the path prior to checking if the path is rooted.

vors commented 9 years ago

It changing behavior: Let's say you are in C:\foo and there is C:\foo\bar folder with weight 1 and C:\bar folder with weight 2. You typing j bar

Before: going to C:\bar

With GetUnresolvedProviderPathFromPSPath: going to C:\foo\bar

drmohundro commented 9 years ago

I went ahead and squashed the 3 commits into one to clean up the history and also cleared up the Load.ps1 changes. It should be a lot cleaner now.

Regarding your notes about the changed behavior, I guess the issue is that my changes make a local directory higher priority than the higher weighted one. I should be able to resolve that by moving the path resolution after the matching instead.

I'll look into that next!

vors commented 9 years ago

thank you for squashing! Right, local dir should not gain priority. In general, I suggest keep it simple and use if last == "~" instead of some generic solution with GetUnresolvedProviderPathFromPSPath. Am I right, that the scenario that you are personally interesting in is j ~ ?

drmohundro commented 9 years ago

Yeah, that's the main use case, but it'd be nice if to be able to handle things like ~/dev or other directories in my home directory which is why I wanted to use GetUnresolvedProviderPathFromPSPath. What I might try for now, though, just for simplicity's sake is to see if the path starts with "~". That'd be less likely to introduce other behaviors, too.

vors commented 9 years ago

Why type j ~\dev, if you can just type j dev?

drmohundro commented 9 years ago

I've reordered the logic so that path resolution only happens as a last resort. This keeps the behavior consistent in terms of favoring any matches first.

To answer your question, yes j dev is simpler, but it'd be nice if it could support the same paths that PowerShell does (e.g. "../path", "~/path", etc.). I was more hoping for consistency with existing PowerShell commands. I definitely agree that the majority scenario is a single search term versus a relative or fully qualified path, though.

I think this gets it, though, and the change is the simplest thus far!

If you think this looks good, I can go ahead and squash these commits down to one as the next step.

vors commented 9 years ago

Looks good to me

drmohundro commented 9 years ago

Okay, down to one commit - let me know if there's anything else I can do to help out!

vors commented 9 years ago

Thank you!