Add linkify module, wraps uri as html links #19
Conversation
|
I'm not sure this fits with the rest of the utilities in yelp-uri which generally deal with plain text. We have an internal module yelp-html which I would imagine this fits into better? shrugs |
|
I rebased this onto #20 since the same merge conflicts/fixes are needed. Not sure if this pull request needs to be edited to reflect this. |
|
@asottile It was my idea to add this. Most of the code here is meant to help implement this function, but it's not entirely trivial add you can see. While I agree it's not the most natural fit ever, it's also not a bad fit. I'd like to merge this. |
|
Would this not fit better in yelp-html? |
|
re yelp-html: probably! I didn't know it existed. It must be internal-only: https://github.com/Yelp/yelp_html |
|
No it doesn't currently, but easy to add :) It does however use yelp-markupsafe (also internal) already. |
| path_query_fragment = uri_match.group('path_query_fragment') | ||
| line_number = '' | ||
| if path_query_fragment is not None: | ||
| _, line_number = re.match(r'^(.+?)((?::\d+)*)$', path_query_fragment).groups('') |
There was a problem hiding this comment.
.groups('') is weird. I haven't seen that before.
Are you sure that return match().groups() isn't the same behavior?
I think ((?::\d+)*) would be simpler as (:[:0-9]+).
It's slightly better to make the regex object a global constant and reuse it.
There was a problem hiding this comment.
.groups('') is weird. I haven't seen that before.
Are you sure that return match().groups() isn't the same behavior?
This sets the default value if no matching group. The default is None, docs.
I think ((?::\d+)*) would be simpler as (:[:0-9]+).
I wanted to match multiple groups, such as line number + column (for example, file.txt:100:12). And I didn't want to capture the individual groups, just everything as a whole in a single group. I'll read the docs again though, maybe I'm not doing capturing groups correctly.
It's slightly better to make the regex object a global constant and reuse it.
Gotcha, will fix.
| # Strip trailing line numbers | ||
| url = uri_match.group()[:-len(line_number)] | ||
|
|
||
| return url, line_number |
There was a problem hiding this comment.
I suspect this can be done in one or two lines rather than eleven.
|
Closing this PR and moving the linkify functions to yelp-html, as suggested. I've opened pull request #23 that cherry picks the general improvements we made along the way. |
Detects uri in strings and wrap them in html anchors. Returns markup safe string for display in web page.