Skip to content
This repository was archived by the owner on Aug 11, 2025. It is now read-only.

Support suffix of id attribute in element of svg #25

Merged
brianquinlan merged 2 commits into from
Nov 7, 2020
Merged

Support suffix of id attribute in element of svg #25

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
56031b8
Add suffix of id attribute in element of svg
ChanTsune Sep 26, 2020
e246a51
Update docstring
ChanTsune Oct 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions pybadges/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ def badge(
whole_title: Optional[str] = None,
left_title: Optional[str] = None,
right_title: Optional[str] = None,
id_suffix: str = '',
Copy link
Contributor

@brianquinlan brianquinlan Oct 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about Optional[str] = None and then we have some code like:

if id_suffix is None:
  id_suffix = str(uuid.uuid4())

?

What do you think? That would mean that the tests need to be updated and the major version of the library bumped?

Or do you think that it is better to leave the default?

Copy link
Contributor Author

@ChanTsune ChanTsune Oct 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your review!

I think adding a random string seems like a good suggestion.

However, the W3C state that:

they cannot start with a digit, two hyphens, or a hyphen followed by a digit.

https://www.w3.org/TR/CSS21/syndata.html#characters.

When using str(uuid.uuid4()), there are cases where a number exists after -.

>>> str(uuid4())
'e489d2a0-39c3-4508-94c5-8337581da1a3'

So I think it's better to remove the - or leave it as it is.

Also, if keep the current PR, you don't have to update the major version, which is a merit I think.

) -> str:
"""Creates a github-style badge as an SVG image.

Expand Down Expand Up @@ -173,6 +174,9 @@ def badge(
right_title: The title attribute to associate with the right part of
the badge.
See https://developer.mozilla.org/en-US/docs/Web/SVG/Element/title.
id_suffix: The suffix of the id attributes used in the SVG's elements.
Use to prevent duplicate ids if several badges are embedded on the
same page.
"""
if measurer is None:
measurer = (
Expand Down Expand Up @@ -200,6 +204,7 @@ def badge(
whole_title=whole_title,
left_title=left_title,
right_title=right_title,
id_suffix=id_suffix,
)
xml = minidom.parseString(svg)
_remove_blanks(xml)
Expand Down
10 changes: 6 additions & 4 deletions pybadges/badge-template-full.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.