gh-158563: Fix various bugs in turtle's undo handling (GH-158564)

(cherry picked from commit 9d3b0b8)

Co-authored-by: Stan Ulbrych <stan@python.org>

Author: StanFromIreland

Lib/test/test_turtle.py

@@ -693,6 +693,34 @@ def test_dot_signature(self):
         self.assertRaises(turtle.TurtleGraphicsError, self.turtle.dot, 0, (0, 257, 0))
         self.assertRaises(turtle.TurtleGraphicsError, self.turtle.dot, 0, 0, 257, 0)
 
+    def test_circle_undo(self):
+        self.turtle.circle(50, 90)
+        self.turtle.undo()
+        self.assertEqual(self.turtle.pos(), (0, 0))
+        self.assertEqual(self.turtle.undobufferentries(), 0)
+
+    def test_undo_sequence_resets_after_exception(self):
+        with unittest.mock.patch.object(self.turtle, "_write",
+                                        side_effect=ValueError):
+            self.assertRaises(ValueError, self.turtle.write, "spam")
+        self.assertFalse(self.turtle.undobuffer.cumulate)
+
+    def test_nested_undo_sequence(self):
+        with self.turtle._undo_sequence():
+            self.turtle.teleport(10, 20)
+            self.turtle.forward(10)
+        self.assertEqual(self.turtle.undobufferentries(), 1)
+        self.turtle.undo()
+        self.assertEqual(self.turtle.pos(), (0, 0))
+
+    def test_stamp_without_undobuffer(self):
+        shape = turtle.Shape("polygon", ((0, 0), (5, 9), (-5, 9)))
+        self.turtle.screen._shapes = {self.turtle.shape(): shape}
+        self.turtle.setundobuffer(None)
+        stamp = self.turtle.stamp()
+        self.turtle.clearstamp(stamp)
+        self.assertEqual(self.turtle.stampItems, [])
+
 class TestModuleLevel(unittest.TestCase):
     def test_all_signatures(self):
         import inspect

Lib/turtle.py

@@ -896,6 +896,15 @@ def pop(self):
                 self.ptr = (self.ptr - 1) % self.bufsize
                 return (item)
 
+    def remove(self, item):
+        if item not in self.buffer:
+            return
+        index = self.buffer.index(item)
+        self.buffer.remove(item)
+        if index <= self.ptr:
+            self.ptr = (self.ptr - 1) % self.bufsize
+        self.buffer.insert((self.ptr+1) % self.bufsize, [None])
+
     def nr_of_items(self):
         return self.bufsize - self.buffer.count([None])
 
@@ -1639,6 +1648,20 @@ def _goto(self, end):
         """Move the turtle to the end position."""
         self._position = end
 
+    @contextmanager
+    def _undo_sequence(self):
+        """Record the enclosed actions as a single undo step."""
+        undobuffer = self.undobuffer
+        if not undobuffer or undobuffer.cumulate:
+            yield
+            return
+        undobuffer.push(["seq"])
+        undobuffer.cumulate = True
+        try:
+            yield
+        finally:
+            undobuffer.cumulate = False
+
     def teleport(self, x=None, y=None, *, fill_gap: bool = False) -> None:
         """To be overwritten by child class RawTurtle.
         Includes no TPen references."""
@@ -1981,38 +2004,34 @@ def circle(self, radius, extent = None, steps = None):
             >>> turtle.circle(50)
             >>> turtle.circle(120, 180)  # draw a semicircle
         """
-        if self.undobuffer:
-            self.undobuffer.push(["seq"])
-            self.undobuffer.cumulate = True
         speed = self.speed()
         if extent is None:
             extent = self._fullcircle
         if steps is None:
             frac = abs(extent)/self._fullcircle
-            steps = 1+int(min(11+abs(radius)/6.0, 59.0)*frac)
+            steps = 1 + int(min(11 + abs(radius) / 6.0, 59.0) * frac)
         w = 1.0 * extent / steps
         w2 = 0.5 * w
         l = 2.0 * radius * math.sin(math.radians(w2)*self._degreesPerAU)
         if radius < 0:
             l, w, w2 = -l, -w, -w2
         tr = self._tracer()
         dl = self._delay()
-        if speed == 0:
-            self._tracer(0, 0)
-        else:
-            self.speed(0)
-        self._rotate(w2)
-        for i in range(steps):
+        with self._undo_sequence():
+            if speed == 0:
+                self._tracer(0, 0)
+            else:
+                self.speed(0)
+            self._rotate(w2)
+            for i in range(steps):
+                self.speed(speed)
+                self._go(l)
+                self.speed(0)
+                self._rotate(w)
+            self._rotate(-w2)
+            if speed == 0:
+                self._tracer(tr, dl)
             self.speed(speed)
-            self._go(l)
-            self.speed(0)
-            self._rotate(w)
-        self._rotate(-w2)
-        if speed == 0:
-            self._tracer(tr, dl)
-        self.speed(speed)
-        if self.undobuffer:
-            self.undobuffer.cumulate = False
 
     # Three dummy methods to be implemented by the child class:
 
@@ -2783,16 +2802,19 @@ def teleport(self, x=None, y=None, *, fill_gap: bool = False) -> None:
         """
         pendown = self.isdown()
         was_filling = self.filling()
-        if pendown:
-            self.pen(pendown=False)
-        if was_filling and not fill_gap:
-            self.end_fill()
-        new_x = x if x is not None else self._position[0]
-        new_y = y if y is not None else self._position[1]
-        self._position = Vec2D(new_x, new_y)
-        self.pen(pendown=pendown)
-        if was_filling and not fill_gap:
-            self.begin_fill()
+        with self._undo_sequence():
+            if pendown:
+                self.pen(pendown=False)
+            if was_filling and not fill_gap:
+                self.end_fill()
+            new_x = x if x is not None else self._position[0]
+            new_y = y if y is not None else self._position[1]
+            if self.undobuffer:
+                self.undobuffer.push(("teleport", self._position))
+            self._position = Vec2D(new_x, new_y)
+            self.pen(pendown=pendown)
+            if was_filling and not fill_gap:
+                self.begin_fill()
 
     def clone(self):
         """Create and return a clone of the turtle.
@@ -3143,7 +3165,8 @@ def stamp(self):
                 screen._drawpoly(item, poly, fill=self._cc(fc),
                                  outline=self._cc(oc), width=self._outlinewidth, top=True)
         self.stampItems.append(stitem)
-        self.undobuffer.push(("stamp", stitem))
+        if self.undobuffer:
+            self.undobuffer.push(("stamp", stitem))
         return stitem
 
     def _clearstamp(self, stampid):
@@ -3158,15 +3181,8 @@ def _clearstamp(self, stampid):
             self.stampItems.remove(stampid)
         # Delete stampitem from undobuffer if necessary
         # if clearstamp is called directly.
-        item = ("stamp", stampid)
-        buf = self.undobuffer
-        if item not in buf.buffer:
-            return
-        index = buf.buffer.index(item)
-        buf.buffer.remove(item)
-        if index <= buf.ptr:
-            buf.ptr = (buf.ptr - 1) % buf.bufsize
-        buf.buffer.insert((buf.ptr+1)%buf.bufsize, [None])
+        if self.undobuffer:
+            self.undobuffer.remove(("stamp", stampid))
 
     def clearstamp(self, stampid):
         """Delete stamp with given stampid
@@ -3464,20 +3480,16 @@ def dot(self, size=None, *color):
             color = self._colorstr(color)
         # If screen were to gain a dot function, see GH #104218.
         pen = self.pen()
-        if self.undobuffer:
-            self.undobuffer.push(["seq"])
-            self.undobuffer.cumulate = True
-        try:
-            if self.resizemode() == 'auto':
-                self.ht()
-            self.pendown()
-            self.pensize(size)
-            self.pencolor(color)
-            self.forward(0)
-        finally:
-            self.pen(pen)
-        if self.undobuffer:
-            self.undobuffer.cumulate = False
+        with self._undo_sequence():
+            try:
+                if self.resizemode() == 'auto':
+                    self.ht()
+                self.pendown()
+                self.pensize(size)
+                self.pencolor(color)
+                self.forward(0)
+            finally:
+                self.pen(pen)
 
     def _write(self, txt, align, font):
         """Performs the writing for write()
@@ -3509,15 +3521,11 @@ def write(self, arg, move=False, align="left", font=("Arial", 8, "normal")):
         >>> turtle.write('Home = ', True, align="center")
         >>> turtle.write((0,0), True)
         """
-        if self.undobuffer:
-            self.undobuffer.push(["seq"])
-            self.undobuffer.cumulate = True
-        end = self._write(str(arg), align.lower(), font)
-        if move:
-            x, y = self.pos()
-            self.setpos(end, y)
-        if self.undobuffer:
-            self.undobuffer.cumulate = False
+        with self._undo_sequence():
+            end = self._write(str(arg), align.lower(), font)
+            if move:
+                x, y = self.pos()
+                self.setpos(end, y)
 
     @contextmanager
     def poly(self):
@@ -3705,6 +3713,9 @@ def _undo(self, action, data):
             self.clearstamp(stitem)
         elif action == "go":
             self._undogoto(data)
+        elif action == "teleport":
+            self._position = data[0]
+            self._update()
         elif action in ["wri", "dot"]:
             item = data[0]
             self.screen._delete(item)

Misc/NEWS.d/next/Library/2026-10-01-13-59-02.gh-issue-158563.kR7wQe.rst

@@ -0,0 +1,4 @@
+Fix :func:`turtle.undo` after :func:`turtle.teleport` and after an exception
+in :func:`turtle.circle`, :func:`turtle.dot` or :func:`turtle.write`. Fix
+:func:`turtle.stamp`, :func:`turtle.clearstamp`, :func:`turtle.clear` and
+:func:`turtle.reset` when the undo buffer is disabled.