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.
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
Add html5gum as alternative link extractor #480
Add html5gum as alternative link extractor #480
Changes from all commits
0491833
6c051a5
8087067
5438744
3a951bc
8bca503
f9403ce
de91aeb
68c67af
539406d
e6b3312
afd488b
e92721b
1b0f650
9b02f58
a8bb51b
ca359d8
283c020
ef3ecd6
57b7cdd
9aa724a
57b3057
aac8610
ccedcec
9ce7e8d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
I think we could
#[derive(Default)]
forLinkExtractor
and this code goes away?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.
I did that now, and it makes
new
non-const asDefault
is not constThere 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.
element
is also calledtag
orname
in this function. Maybe we should stick to one word?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.
it's html5gum (html spec) naming clashing with lychee naming
other candidates for rename are
last_start_tag
but they are not used by lycheeit's unclear where to draw the line to me because you clearly can't pick different method names
i chose to do just the rename you proposed (as that's the attribute we use for link extraction) but now the discrepancy is visible in the struct definitionbest i can do is do a mass rename, accept that in things like
set_last_start_tag
the variable names are inconsistent and document that elem and tag are the same thing