Skip to content

Updated Group.c to correct bug with insert into displayio.Group #3214

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

Merged
merged 4 commits into from
Jul 29, 2020

Conversation

kmatch98
Copy link

This is to resolve the bug when inserting an element at the end of an existing group. See issue: #3129 (comment)

In this PR, per your suggestions I check if the requested insert index is equal to the length of the group. If so, then call append, else call insert.

I hunted around the code for the proper type conversions, but I am unsure if I am doing the type conversion correctly.
** Please double-check since I am not familiar with any of the data types that are used here.

I built this and verified on an Itsy-Bitsy NRF52840

Here is how I verified the code:

>>> 
>>> import displayio
>>> group1=displayio.Group(max_size=5)
>>> group2=displayio.Group(max_size=5)
>>> group3=displayio.Group(max_size=5)
>>> group1.insert(0,group2)
>>> group1.insert(1,group3)
>>> len(group1)
2
>>> group1.pop(0)
<Group>
>>> len(group1)
1
>>> group1.insert(1,group2)
>>> len(group1)
2
>>> 

@FoamyGuy whenever this bug is corrected, you can simplify the workaround to the label.py related where you had to double-check the length of the group to decide whether to use group.append or group.insert.

@FoamyGuy
Copy link
Collaborator

I tested this out on a PyGamer. Prior to these changes I get IndexError from this line group1.insert(0,group2). With these changes it is working successfully.

Thank you @kmatch98 for working on this! It will be great to unwind this bit of complexity in display_text

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Please delete the mpy-cross file. The easiest way is to edit the existing commit to remove the file.

This fix looks good otherwise! Just a style comment. Thanks!

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Thank you!

@tannewt tannewt merged commit 05a1519 into adafruit:main Jul 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants