Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix checking out branch names with / chars #113

Merged
merged 2 commits into from
Oct 12, 2020

Conversation

docwhat
Copy link
Contributor

@docwhat docwhat commented Oct 12, 2020

If git can't automatically figure out if the argument is a branch or
a path, it will need to be told explicitly by using --:

Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'

Fixes: #106

If git can't automatically figure out if the argument is a branch or
a path, it will need to be told explicitly by using `--`:

    Use '--' to separate paths from revisions, like this:
    'git <command> [<revision>...] -- [<file>...]'

Fixes: stefanzweifel#106
@docwhat
Copy link
Contributor Author

docwhat commented Oct 12, 2020

Here's a version of #106 that keeps using git checkout instead of git switch. 😃

@stefanzweifel
Copy link
Owner

Thanks!

@stefanzweifel stefanzweifel merged commit 8700c3b into stefanzweifel:master Oct 12, 2020
@docwhat
Copy link
Contributor Author

docwhat commented Oct 12, 2020

Sorry about not updating the tests. I didn't realize you were mocking git.

@docwhat docwhat deleted the pr/complex-branches-2 branch October 12, 2020 14:36
@stefanzweifel
Copy link
Owner

@docwhat No problem. Those tests are all really new (and a bit complicated). I don't expect contributors to update those.

@docwhat
Copy link
Contributor Author

docwhat commented Oct 12, 2020

I would recommend not mocking git and instead use real git and a known repository instead. e.g. your own repo.

That way you are testing the functionality of the code and can prevent regressions.

@stefanzweifel
Copy link
Owner

@docwhat Thanks for the advice. I'm aware of that problem.

I even mentioned it in the PR which added the tests (#109) and it has become painfully clear in this PR that it's not the way to go forward. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants