-
Notifications
You must be signed in to change notification settings - Fork 600
Document SanitizedAnchorName algorithm, copy implementation. #352
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
Conversation
The goal of this change is to reduce number of non-standard library packages (repositories) that blackfriday imports from 1 to 0, and in turn, reduce the cost of importing blackfriday into other projects. Do so by documenting the algorithm of SanitizedAnchorName, and include a copy of the small function inside blackfriday itself. The same functionality continues to be available in the original location, github.com/shurcooL/sanitized_anchor_name.Create. It can be used by existing users and those that look for a small package, and don't need all of blackfriday functionality. Existing users of blackfriday can use the new SanitizedAnchorName function directly and avoid an extra package import. Resolves #350.
// This algorithm is also implemented in a small standalone package at | ||
// github.com/shurcooL/sanitized_anchor_name. It can be useful for clients | ||
// that want a small package and don't need full functionality of blackfriday. | ||
package blackfriday |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM--Thanks for doing this! |
This is great, thanks! The only complaint that I have is the duplicate paragraphs in README and doc.go. Would it perhaps be better to leave the reference text in doc.go and link to it from the README? |
I did that based on how the go-github package did it. It has similar documentation in godoc and README: And we keep them in sync (google/go-github#397). Personally, I always prefer looking at godoc, but I guess the idea is that beginners to Go may miss that, so putting it in the README helps them catch it. However, I like your idea of linking to the algorithm specification from the README to the godoc. That way, the specification is in one canonical place and cannot get out of sync. I'll apply that change. |
@rtfb, how do you feel about adding an exported version of Also, if we do export it, do you have opinions on whether it should be inside |
I guess that depends on how likely are people to have projects which call blackfriday in one place and SanitizedAnchorName in another. I can certainly imagine that, but the likelihood seems small. I don't have strong opinion either way, both seem to be adequate.
I say, inside. Can't foresee that subpackage growing, and one function packages are a bit weird, especially when the function is so specific. |
This way, the specification has a canonical location and doesn't need to be kept in sync between the README and godoc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
LGTM thanks. |
The goal of this change is to reduce number of non-standard library packages (repositories) that blackfriday imports (not counting imports used only for tests) from 1 to 0, and in turn, reduce the cost of importing blackfriday into other projects. Do so by documenting the algorithm of SanitizedAnchorName, and include a copy of the small function inside blackfriday itself. The same functionality continues to be available in the original location, github.com/shurcooL/sanitized_anchor_name.Create. It can be used by existing users and those that look for a small package, and don't need all of blackfriday functionality. Existing users of blackfriday can use the new SanitizedAnchorName function directly and avoid an extra package import. This change is a port of PR #352 from v1 into v2. Updates #348. Updates #350.
The goal of this change is to reduce number of non-standard library packages (repositories) that blackfriday imports (not counting imports used only for tests) from 1 to 0, and in turn, reduce the cost of importing blackfriday into other projects. Do so by documenting the algorithm of SanitizedAnchorName, and include a copy of the small function inside blackfriday itself. The same functionality continues to be available in the original location, github.com/shurcooL/sanitized_anchor_name.Create. It can be used by existing users and those that look for a small package, and don't need all of blackfriday functionality. Existing users of blackfriday can use the new SanitizedAnchorName function directly and avoid an extra package import. This change is a port of PR #352 from v1 into v2. Updates #348. Updates #350.
…s#352) The goal of this change is to reduce number of non-standard library packages (repositories) that blackfriday imports from 1 to 0, and in turn, reduce the cost of importing blackfriday into other projects. Do so by documenting the algorithm of SanitizedAnchorName, and include a copy of the small function inside blackfriday itself. The same functionality continues to be available in the original location, github.com/shurcooL/sanitized_anchor_name.Create. It can be used by existing users and those that look for a small package, and don't need all of blackfriday functionality. Existing users of blackfriday can use the new SanitizedAnchorName function directly and avoid an extra package import. Resolves russross#350.
The goal of this change is to reduce number of non-standard library packages (repositories) that
blackfriday
imports from 1 to 0, and in turn, reduce the cost of importingblackfriday
into other projects.Do so by documenting the algorithm of
SanitizedAnchorName
, and include a copy of the small function insideblackfriday
itself. The same functionality continues to be available in the original location,github.com/shurcooL/sanitized_anchor_name.Create
. It can be used by existing users and those that look for a small package, and don't need all ofblackfriday
functionality. Existing users ofblackfriday
can use the newSanitizedAnchorName
function directly and avoid an extra package import.Resolves #350.