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

Correctly support branch names with slashes #259

Merged

Conversation

@NateEag
Copy link
Contributor

@NateEag NateEag commented Feb 12, 2013

Fixes #250.

Previously, branches with slashes in the name would consume all slashed
segments in a URL, causing the routes to capture incorrect file paths.

This solution is not particularly elegant - anywhere a route used
both a commit-ish identifier and a path, we collapse those two params
into a single param, and parse that param inside the route.

It seems to be working reasonably reliably, but has not seen extensive
testing.

It is also not particularly pretty. If anyone sees ways to improve it,
please, have at it.

Previously, branches with slashes in the name would consume all slashed
segments in a URL, causing the routes to capture incorrect file paths.

This solution is not particularly elegant - anywhere a route used
both a commit-ish identifier and a path, we collapse those two params
into a single param, and parse that param inside the route.

It seems to be working reasonably reliably, but has not seen extensive
testing.

It is also not particularly pretty. If anyone sees ways to improve it,
please, have at it.
klaussilveira added a commit that referenced this pull request Feb 12, 2013
Correctly support branch names with slashes
@klaussilveira klaussilveira merged commit 5ccad64 into klaussilveira:master Feb 12, 2013
1 check passed
1 check passed
default The Travis build passed
Details
@klaussilveira
Copy link
Owner

@klaussilveira klaussilveira commented Feb 12, 2013

It's safe for master, let's just create more test cases surrounding this feature.

}

if ($commitish === null) {
// DEBUG Can you have a repo with no branches? How should we handle

This comment has been minimized.

@klaussilveira

klaussilveira Feb 12, 2013
Owner

We should fallback to the default branch. Take a look at: #61

@NateEag NateEag deleted the NateEag:fix-branches-with-slashes branch Feb 13, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants