Add tab expansion support for git restore / switch#702
Conversation
| "^restore.* -s\s*(?<ref>\S*)$" { | ||
| gitBranches $matches['ref'] $true | ||
| gitTags $matches['ref'] | ||
| break |
There was a problem hiding this comment.
I wonder if more of these switch cases should be using break. Once we've found a "match" do we need to continue with every subsequent regex match?
There was a problem hiding this comment.
I was actually running into issues with this case myself and ended up matching --staged somehow, and kinda gave up and forgot to look back at it. That said, I think this looks semi-correct. I think this will match "-S" (which is the short-form of staged) followed by anything since we aren't using a case-sensitive match
There was a problem hiding this comment.
@rkeithhill re: break ... when working on my version of this as well as #696 I realized that the fall through would end up matching some of the later cases per the regex, but then return nothing based on the inner function calls. (lots of Write-Host debugging :-p) and was going to suggest we throw in breaks. With that said, I swear I encountered at least one instance where the fall-through actually caught an edge-case.
There was a problem hiding this comment.
I swear I encountered at least one instance where the fall-through actually caught an edge-case.
That's why I'm nervous about throwing in break everywhere. But in this new case, it should break IMO.
RE the above code matching on -S, good catch. I'll make this regex be case-sensitive.
|
@pinkfloydx33 Updated. Can you take another peek? |
rkeithhill
force-pushed
the
rkeithhill/add-switch-restore-tab-completions
branch
from
a605032 to
21857a4
Compare
|
@pinkfloydx33 I'm going to merge this. Can you pull master and try it out. If you find any issues, we can fix them before the final release. |
| conflict = 'merge diff3' | ||
| source = { | ||
| param($ref) | ||
| gitBranches $matches['ref'] $true |
There was a problem hiding this comment.
I don't always review code, but when I do it's six months later. 💪
Did you mean to use $ref here?
There was a problem hiding this comment.
Yup, good catch. I'll submit a PR shortly.
3 participants
Fix #691