Fix invalid url raising error during Groupie.tokenize #44
Conversation
This file contains 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
In the initial implementation to support URIs, I overlooked that it could raise an exception. This is now handled, in which case we treat the input String as plain text, instead of decomposing it into tokens. The code triggers Rubocop's desire to have shorter methods, so I'll see about reducing the complexity of the method next.
- Extracted `maybe_parse_url` to encapsulate that Strings matched by
gsub might not in fact be valid URls.
- Condensed the `var = (uri part).to_s; var.tr!()` logic required due to
`String#tr!` not returning `self` in case of a no-op to put it in a
`tap {}` block instead. I'm not in love with the solution, but it's
a minor improvement over the previous one. One line now matches to one
part of the url.
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.
What's the problem?
An invalid URL-like String would cause
Groupie.tokenizeto raiseURI::InvalidURIError.How to reproduce it?
What's changed in the solution?
The input that resembles a URL but fails to parse is no longer treated like a real URL, but simply as text that's processed by the rest of the code.
Due to the added responsibility of handling this new concern (is the URL valid or not?) I've extracted a helper method that encapsulates the new behavior and condensed the old code (which improves readability to my eyes) to stay within the line count guidelines that Rubocop enforces by default.
Tokenizer#tokenize_urls!(c48148e)The changelog now points to both PRs for this feature.