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

helpers: Consolidate MakeSegment vs MakePathSanitized #5282

Merged
merged 1 commit into from
Oct 3, 2018

Conversation

bep
Copy link
Member

@bep bep commented Oct 3, 2018

In short:

  • Avoid double tolower in MakeSegment
  • Use MakePathSanitized for taxonomies in pageToPermalinkTitle; this matches what MakeSegment does.
  • Move the "double hyphen and space" logic into UnicodeSanitize

The last bullet may be slightly breaking for some that now does not get the "--" in some URLs, but we need to reduce the amount of URL logic.

See #4926

@bep bep mentioned this pull request Oct 3, 2018
@bep bep changed the title helpers: Consolidate MakgeSegment vs MakePathSanitized helpers: Consolidate MakeSegment vs MakePathSanitized Oct 3, 2018
@bep bep force-pushed the make-path-consolidate branch from 9aa3317 to 205dc55 Compare October 3, 2018 10:20
@bep bep requested a review from moorereason October 3, 2018 10:20
@bep bep force-pushed the make-path-consolidate branch 3 times, most recently from 6e0280a to a282492 Compare October 3, 2018 10:37
In short:

* Avoid double tolower in MakeSegment
* Use MakePathSanitized for taxonomies in pageToPermalinkTitle; this matches what MakeSegment does.
* Move the "double hyphen and space" logic into UnicodeSanitize

The last bullet may be slightly breaking for some that now does not get the "--" in some URLs, but we need to reduce the amount of URL logic.

See gohugoio#4926
@bep bep force-pushed the make-path-consolidate branch from a282492 to 628bac4 Compare October 3, 2018 13:04
Copy link
Contributor

@moorereason moorereason left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

@bep bep merged commit e421696 into gohugoio:master Oct 3, 2018
@onedrawingperday
Copy link
Contributor

Everything works fine with this PR on my end. 👍

P.S. Wow! This is the 3rd time today that I compiled Hugo from source because of all the new commits. Your hard work is very much appreciated! 🥇

@max-arnold
Copy link
Contributor

Does this fix #3577?

@moorereason
Copy link
Contributor

@max-arnold, potentially. If you can test it before I do, let me know.

@github-actions
Copy link

github-actions bot commented Feb 2, 2022

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants