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

fix for overlayed SD folder line #21

Merged
merged 3 commits into from
Sep 16, 2024

Conversation

FoamyGuy
Copy link
Collaborator

@FoamyGuy FoamyGuy commented Sep 12, 2024

resolves: #18

Here is an example of the corrected screenshot generated by this version. This was the example that was linked to in the issue as well.
FunHouse_IOT_Hub_iot_hub

I also confirmed a few other projects get fixed by this include others inside of FunHouse_IOT_Hub as well as FunHouse_Parking which had the same issue before with over-layed SD line.

Also contains a small correction to the README example commands and a new section showing the help command.

@FoamyGuy FoamyGuy marked this pull request as draft September 12, 2024 17:23
@FoamyGuy
Copy link
Collaborator Author

I think this is not the correct fix actually, after updating a few things to make the actions happy and then testing some more I've found some other projects that have that overlayed SD which aren't just single file code.py projects. I'm trying to work out what the common thread between the broken ones is.

@FoamyGuy FoamyGuy changed the title fix for projects with only code.py fix for overlayed SD folder line Sep 12, 2024
@FoamyGuy
Copy link
Collaborator Author

I think the true root cause of this issue is settings.toml existing or not for a given project.

I validated it by modifying this utility to split up the screenshots into with settings and without with this code:

    if context["added_settings_toml"]:
        img.save(f"generated_images/with_settings/{image_name}.png")
    else:
        img.save(f"generated_images/no_settings/{image_name}.png")

All of the ones I checked with settings.toml have the overlayed SD folder, and all the ones I checkout without settings are correct without the overlay issue.

I will add a commit soon with a fix for this as well as the remaining issues that actions is failing for.

…mmit hooks to newer versions and fix issues flagged by new versions.
@FoamyGuy FoamyGuy marked this pull request as ready for review September 12, 2024 18:38
@FoamyGuy
Copy link
Collaborator Author

Okay the main fix here is a change to the logic that determines the position of the SD folder line to make it account for the existence of settings.toml.

The changes in README correct the example commands to use the subcommand learn which is required for their usage. I also added a section to show the --help option which is what I used to figure out why the other commands didn't work.

The other changes:

  • updating the upload-artifacts action to use v4 because v2 was causing an error with a link to a github docs page explaining the deprecation of the older versions
  • updating the versions of pylint and reuse that are used by pre-commit in order to make them able to run under ubuntu 24.04
  • copied in a relevant section from the .gitignore in a library. I was tripped up for a moment locally when reuse was failing on some of this files which are ignored in all of the library repos.
  • changed format() to f-strings and added encoding arguments to file opening in order to satisfy the newer version of pylint

Copy link
Contributor

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

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

Looks good! I did not try to think of pathological cases.

@dhalbert dhalbert merged commit 0dd5ea6 into circuitpython:main Sep 16, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Overlayed text issue
2 participants