Skip to content
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 --name option to change badge left side text #25

Merged
merged 11 commits into from Jun 29, 2022
Merged

Add --name option to change badge left side text #25

merged 11 commits into from Jun 29, 2022

Conversation

nefrob
Copy link
Contributor

@nefrob nefrob commented Jun 25, 2022

Re:

Pending:

  • Pass tests? It seems to be inserting the opposite ref_nbs values as needed. I didn't edit this so not sure if it is something wrong with my local env setup and this is fine. Seems I hadn't installed the newest version of pillow somehow.

@nefrob nefrob marked this pull request as ready for review June 25, 2022 03:32
setup.cfg Show resolved Hide resolved
genbadge/main.py Outdated Show resolved Hide resolved
genbadge/main.py Outdated Show resolved Hide resolved
genbadge/main.py Outdated Show resolved Hide resolved
@smarie
Copy link
Owner

smarie commented Jun 25, 2022

Thanks a lot @nefrob !

I suggested to fix a few typos here and there.

Also, could you please check that the "removing the left text" use case is actually supported, and update the flag documentation accordingly ?

Other than this, the last thing is that for python < 3.6 we should install flake8-html<=0.4.1 (or not install it at all) so that the build on CI passes.
So to go quick for now, the best is probably to change the nox tests session so that this line

session.install_reqs(setup=True, install=True, tests=True, extras=("all",), versions_dct=pkg_specs)

becomes

`session.install_reqs(setup=True, install=True, tests=True, extras=extras, versions_dct=pkg_specs)`

with extras=("all",) on python>=3.6 but is empty extras=() otherwise. I'm pretty sure the extras are not needed in the tests. If by bad luck they are needed, we should install the extras one by one with special version for flake8-html>=0.4,<=0.4.1, which is compliant with the older python versions.

Thanks !

Comment on lines +88 to +90
svg = self.as_svg(use_shields=use_shields)
if clear_left_txt:
svg = svg.replace(">" + self.left_txt + "<", "><")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a little messy. In the case where we keep the left side of the badge but have no text it would be great to pass a tab character. However shields escapes label input so this doesn't work well. Instead we're looking for a strict match of the left txt inside html tags and replacing it with nothing afterwards. A placeholder ### is used to create the svg space (so removing text doesn't remove the left side) in this case. Thoughts on improving this welcome.

@nefrob
Copy link
Contributor Author

nefrob commented Jun 25, 2022

Examples of coverage badges:

  • -n ""

    svgviewer-png-output (1)

  • --noname

    svgviewer-png-output (2)

  • -n "new name"

    svgviewer-png-output

@nefrob
Copy link
Contributor Author

nefrob commented Jun 27, 2022

@smarie tried to remove extras for tests but they are needed for the commands output checks. Going with the flake8-html approach you suggested. Unfortunately not able to test anything but python 3.9 on my current machine so hopefully that works.

@nefrob
Copy link
Contributor Author

nefrob commented Jun 28, 2022

Hmm I can't seem to figure out getting the tests to run at all locally, especially for the breaking python versions. Not sure why they wouldn't be passing now when I didn't tweak anything around dependencies. Maybe per your suggestion we can set extra=("tests","coverage",) since we install flake8 dependencies from flake8-requirements.txt in the next test? Committing that for now but would appreciate it you could check this out!

noxfile.py Outdated Show resolved Hide resolved
@smarie
Copy link
Owner

smarie commented Jun 29, 2022

I made two comments with changes proposals, that I accepted in hope I could see the result (it works locally on my machine). However for some reason github actions does not re-trigger. I'll merge and see. Thanks again @nefrob !

@smarie smarie merged commit 4fd2380 into smarie:main Jun 29, 2022
@nefrob
Copy link
Contributor Author

nefrob commented Jun 29, 2022

Gotcha, thanks for looking into this @smarie. Let me know if I can help with anything else in getting this released.

@smarie
Copy link
Owner

smarie commented Jun 29, 2022

This is now released - 1.1.0 is out :) thanks again !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants