Draft
Conversation
Member
|
Looks good. Neat how compact & fast implementation you got for this. Remaining considerations: should we catch/handle if the same attributes are defined as kwargs too (I believe now you get another attribute by the same name in HTML)? Possibly even restructure html5tagger such that the whole opening tag is written at once instead of appending attributes to HTML snippet the way it currently does. |
Member
|
Needs README update I guess, but is there anything else stopping this from merging? |
|
|
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
The goal of this PR is to come up with a nicer API for assigning HTML
idandclassattributes that clash with Python syntax. One approach is to overload the__getitem__method to allow for this:There are a few considerations:
__getitem__should be a single string, or a tuple of strings:["#id", "class", ...].) prepended:.one.two.threevone two threeThe implementation I proposed answers these as by opting for a single string that follows CSS-style selector. While potentially we could go with
#some-id one two threeand just check the first character for a#, in which case we pop off the one value, this pattern was not chosen because it then means that this lib would be introducing a new pattern that is not found in HTML.Alternatively, we could have multiple values
builder["#some-id","one two three"]. While this is valid Python, it is a bit awkward (granted, any overload of__getitem__is certainly non-standard).Finally, assuming
"#some-id.one.two.three"is preferred over"#some-id one two three", the next logical question is why not arbitrary attribute assignment with[foo=bar]style syntax.As a side benefit, it would allow a
data-foo_bar=thingattribute, if that is desired.