Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork34.2k
gh-91167: Fix cloned turtle pen not clearing completely#145406
gh-91167: Fix cloned turtle pen not clearing completely#145406yukarikaname wants to merge 1 commit intopython:mainfrom
Conversation
python-cla-botbot commentedMar 2, 2026 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
All commit authors signed the Contributor License Agreement. |
Most changes to Pythonrequire a NEWS entry. Add one using theblurb_it web app or theblurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Pull request overview
FixesRawTurtle.clone() so that callingclear() on a cloned turtle does not delete drawings owned by the source turtle, by ensuring the clone tracks its own drawing items instead of inheriting source-owned canvas item ids.
Changes:
- Reinitialize
currentLineanditemsfor cloned turtles afterdeepcopy()to avoid shared canvas-item ownership. - Add a regression test ensuring
clone.clear()does not delete items owned by the source turtle.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
Lib/turtle.py | Adjusts clone initialization so the clone’s drawing state (currentLine,items) starts fresh and is independent from the source. |
Lib/test/test_turtle.py | Adds a targeted regression test for clone-vs-source item deletion behavior duringclear(). |
💡Add Copilot custom instructions for smarter, more guided reviews.Learn how to get started.
| screen._shapes[self.turtle.shapeIndex]._data] | ||
| q.currentLineItem=screen._createline() | ||
| q.currentLine= [q._position]ifq._drawingelse [] | ||
| q.items= [q.currentLineItem] |
CopilotAIMar 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
clone() still leaves other screen-item ownership state pointing at the source turtle:drawingLineItem is deep-copied (so animated motion can reuse the source’s transient line item),stampItems is deep-copied (soclone.clear()/clearstamps() can delete stamps created by the source), andundobuffer is deep-copied (soclone.undo() can replay/delete the source’s canvas items). Consider reinitializing these fields forq (and resetting any fill-related item ids like_fillitem/_fillpath if filling) so the clone can’t mutate/delete the source turtle’s canvas items via non-clear() operations.
| q.items= [q.currentLineItem] | |
| q.items= [q.currentLineItem] | |
| # Reset ownership/undo state so the clone cannot affect the source | |
| # turtle's existing canvas items via non-clear operations. | |
| # The transient animation line item will be recreated on demand. | |
| ifhasattr(q,"drawingLineItem"): | |
| q.drawingLineItem=None | |
| # The clone should start with no stamps of its own. | |
| ifhasattr(q,"stampItems"): | |
| q.stampItems= [] | |
| # Clear the clone's undo history so q.undo() cannot replay or | |
| # delete items created before cloning. | |
| ifhasattr(q,"undobuffer"): | |
| reset=getattr(q.undobuffer,"reset",None) | |
| ifcallable(reset): | |
| reset() | |
| # If a fill operation is active on the source, avoid sharing the | |
| # same fill item/path ids with the clone. | |
| ifhasattr(q,"_fillitem")andgetattr(self,"_fillitem",None)isnotNone: | |
| q._fillitem=None | |
| ifhasattr(q,"_fillpath")andgetattr(self,"_fillitem",None)isnotNone: | |
| q._fillpath= [] |
| def test_clone_clear_does_not_delete_source_items(self): | ||
| screen = self.turtle.screen | ||
| screen._delete.reset_mock() | ||
| screen._createline.side_effect = lambda: object() | ||
| self.turtle.circle(20) | ||
| clone = self.turtle.clone() | ||
| source_items = set(self.turtle.items) | ||
| clone.forward(50) | ||
| clone.clear() | ||
| deleted_items = {call.args[0] for call in screen._delete.call_args_list} | ||
| self.assertFalse(source_items & deleted_items) | ||
CopilotAIMar 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This test only asserts thatclone.clear() doesn’t delete items fromself.turtle.items, but the bug class here also involves other clone-owned collections (notablystampItems andundobuffer) that can still contain source-owned canvas ids afterdeepcopy(). Extending the test to cover stamps (e.g., sourcestamp() thenclone.clear()/clearstamps()) and undo behavior (e.g.,clone.undo() doesn’t touch source drawings) would prevent regressions onceclone() is updated to reinitialize those fields too.
When calling clear() on a cloned turtle, it incorrectly cleared the sourceturtle's drawings instead of its own. Fix by reinitializing the clone'scurrentLine and items to track only its own drawing state.
Most changes to Pythonrequire a NEWS entry. Add one using theblurb_it web app or theblurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
Uh oh!
There was an error while loading.Please reload this page.
When calling clear() on a cloned turtle, it incorrectly cleared the source
turtle's drawings instead of its own.
The issue was in RawTurtle.clone(): after deepcopy(), the clone inherited
the source turtle's currentLine and items lists, causing the clone's
clear() to delete items owned by the source turtle.
Fix by reinitializing currentLine and items for the clone to track only
its own drawing state.