Fix slug generation for uppercase branch names #1396
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…s unslugifiable chars.
…es with uppercase letters.
| slug_length = len(slugified) | ||
|
|
||
| if not slugified: | ||
| return self.fallback_slug |
There was a problem hiding this comment.
I'd also be +1 on replacing these blocks with regex comparisons/substitutions. These two blocks could be a single regex substitution, with a check against the final matches for empty/invalid slug.
There was a problem hiding this comment.
Good idea, that is addressed with the latest push. I've not used regex.sub before that's why I didn't think about it. It reduced the code quite a bit. Thanks for the suggestion!
|
Excellent, looks good |
agjohnson
added a commit
that referenced
this pull request
Jul 6, 2015
Fix slug generation for uppercase branch names
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
There is another issue with slugs that was raised in #1176. The current implementation turns uppercase letters into dashes so that
SomeChar-charclassas branch name results in-ome-har-charclassas slug. However our regex in the URLs don't allow leading dashes.This PR changes the algorithm to strip punctuation from the generated slug. It includes a migration to fix the currently broken slugs. I tested locally that pypy's version page works after applying it.