fix(understand): a DWG the operator has open is exported as it stands, never closed
On the live engine, take_snapshot read a .dwg path by calling Documents.Open(path, read-only), exporting and then Close(False) on what Open returned. If the operator already had that drawing open, that was their document, and closing it unsaved could throw away unsaved work. drawing_understand, drawing_scale_check, drawing_diff and the takeoffs all read a path through this function. The review of drawing_diff raised the risk and did not probe it live, to avoid a modal dialog on the shared AutoCAD. The fix makes the question moot: the path is looked up among the open documents first, case-blind as Windows paths are, and an open drawing is exported as it stands (unsaved edits included) and left open. Only a document opened here is closed here. The new test, against the fake that behaves as measured, fails on the old code with an open and a close in the log. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FKFStjEFzpGyLq5bMpWxTd
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
efa1ea8182
commit
cdff63fce0
@@ -33,7 +33,9 @@ Where the snapshot comes from:
|
|||||||
``drawing_understand``'s docstring says so.
|
``drawing_understand``'s docstring says so.
|
||||||
|
|
||||||
A DWG path on the live engine is opened read-only, exported the same way and
|
A DWG path on the live engine is opened read-only, exported the same way and
|
||||||
closed without saving.
|
closed without saving - unless AutoCAD already has that drawing open: then
|
||||||
|
its open document is exported as it stands and left open, because closing
|
||||||
|
it could throw away the operator's unsaved work.
|
||||||
|
|
||||||
Records hold what the readers need and nothing else: WCS points (OCS is
|
Records hold what the readers need and nothing else: WCS points (OCS is
|
||||||
translated here with ``backends/ocs.py``, so a mirrored ARC or LWPOLYLINE is
|
translated here with ``backends/ocs.py``, so a mirrored ARC or LWPOLYLINE is
|
||||||
@@ -330,6 +332,21 @@ def read_snapshot(path: str) -> Snapshot:
|
|||||||
return _read(str(target), source=str(target))
|
return _read(str(target), source=str(target))
|
||||||
|
|
||||||
|
|
||||||
|
def _already_open(app, path: str):
|
||||||
|
"""The document AutoCAD already has open at ``path``, or None. Opens nothing."""
|
||||||
|
wanted = os.path.normcase(os.path.abspath(path))
|
||||||
|
documents = app.Documents
|
||||||
|
for index in range(int(documents.Count)):
|
||||||
|
doc = documents.Item(index)
|
||||||
|
try:
|
||||||
|
full = str(doc.FullName or "")
|
||||||
|
except Exception: # a document closing under us answers nothing; it is not ours
|
||||||
|
continue
|
||||||
|
if full and os.path.normcase(os.path.abspath(full)) == wanted:
|
||||||
|
return doc
|
||||||
|
return None
|
||||||
|
|
||||||
|
|
||||||
def _com_export(backend, path: str | None) -> tuple[str, str, str]:
|
def _com_export(backend, path: str | None) -> tuple[str, str, str]:
|
||||||
"""(snapshot file, its temporary folder, source) - runs on the COM thread."""
|
"""(snapshot file, its temporary folder, source) - runs on the COM thread."""
|
||||||
from backends import com_backend as com
|
from backends import com_backend as com
|
||||||
@@ -339,6 +356,11 @@ def _com_export(backend, path: str | None) -> tuple[str, str, str]:
|
|||||||
if path is None:
|
if path is None:
|
||||||
doc = com._acad_doc()
|
doc = com._acad_doc()
|
||||||
else:
|
else:
|
||||||
|
# A drawing the operator already has open is exported as it stands and
|
||||||
|
# left open: opening it again and closing "it" unsaved could close their
|
||||||
|
# copy with its unsaved work. Only a document opened here is closed here.
|
||||||
|
doc = _already_open(app, str(path))
|
||||||
|
if doc is None:
|
||||||
opened = app.Documents.Open(str(path), True) # read-only
|
opened = app.Documents.Open(str(path), True) # read-only
|
||||||
doc = opened
|
doc = opened
|
||||||
folder = tempfile.mkdtemp(prefix="acadmcp_snap_")
|
folder = tempfile.mkdtemp(prefix="acadmcp_snap_")
|
||||||
@@ -373,7 +395,9 @@ async def take_snapshot(backend, path: str | None = None) -> Snapshot:
|
|||||||
|
|
||||||
``path`` given: a ``.dxf`` is read directly (on either engine); a ``.dwg``
|
``path`` given: a ``.dxf`` is read directly (on either engine); a ``.dwg``
|
||||||
needs the live engine, which opens it read-only, exports it and closes it -
|
needs the live engine, which opens it read-only, exports it and closes it -
|
||||||
headlessly it is refused with the ``dwg`` capability. ``path`` None: the
|
or, when AutoCAD already has it open, exports that document as it stands
|
||||||
|
(unsaved edits included) and leaves it open - and headlessly it is refused
|
||||||
|
with the ``dwg`` capability. ``path`` None: the
|
||||||
headless engine reads its in-memory document under its own lock; the live
|
headless engine reads its in-memory document under its own lock; the live
|
||||||
engine exports the active document to a temporary DXF (see the module
|
engine exports the active document to a temporary DXF (see the module
|
||||||
docstring for what was measured) and deletes it afterwards. The snapshot of
|
docstring for what was measured) and deletes it afterwards. The snapshot of
|
||||||
|
|||||||
@@ -257,8 +257,9 @@ class _Selections:
|
|||||||
|
|
||||||
|
|
||||||
class _Doc:
|
class _Doc:
|
||||||
def __init__(self, name, log):
|
def __init__(self, name, log, full_name=""):
|
||||||
self.Name = name
|
self.Name = name
|
||||||
|
self.FullName = full_name # "" for a drawing never saved, as AutoCAD answers
|
||||||
self.log = log
|
self.log = log
|
||||||
self.SelectionSets = _Selections(log)
|
self.SelectionSets = _Selections(log)
|
||||||
|
|
||||||
@@ -276,8 +277,16 @@ class _Doc:
|
|||||||
|
|
||||||
|
|
||||||
class _Documents:
|
class _Documents:
|
||||||
def __init__(self, log):
|
def __init__(self, log, open_docs=()):
|
||||||
self.log = log
|
self.log = log
|
||||||
|
self.open_docs = list(open_docs)
|
||||||
|
|
||||||
|
@property
|
||||||
|
def Count(self): # noqa: N802
|
||||||
|
return len(self.open_docs)
|
||||||
|
|
||||||
|
def Item(self, index): # noqa: N802
|
||||||
|
return self.open_docs[index]
|
||||||
|
|
||||||
def Open(self, path, read_only): # noqa: N802
|
def Open(self, path, read_only): # noqa: N802
|
||||||
self.log.append(("open", Path(path).name, read_only))
|
self.log.append(("open", Path(path).name, read_only))
|
||||||
@@ -285,11 +294,11 @@ class _Documents:
|
|||||||
|
|
||||||
|
|
||||||
class _App:
|
class _App:
|
||||||
def __init__(self, log):
|
def __init__(self, log, open_docs=()):
|
||||||
self.Documents = _Documents(log)
|
self.Documents = _Documents(log, open_docs)
|
||||||
|
|
||||||
|
|
||||||
def _live(monkeypatch, log):
|
def _live(monkeypatch, log, open_docs=()):
|
||||||
backend = ComBackend()
|
backend = ComBackend()
|
||||||
|
|
||||||
async def run_inline(func, *args, **kwargs): # bypass the STA executor
|
async def run_inline(func, *args, **kwargs): # bypass the STA executor
|
||||||
@@ -301,7 +310,7 @@ def _live(monkeypatch, log):
|
|||||||
backend._run = run_inline
|
backend._run = run_inline
|
||||||
backend._ensure_document_state = ensure_state
|
backend._ensure_document_state = ensure_state
|
||||||
active = _Doc("Drawing1.dwg", log)
|
active = _Doc("Drawing1.dwg", log)
|
||||||
monkeypatch.setattr(cb, "_acad_app", lambda: _App(log))
|
monkeypatch.setattr(cb, "_acad_app", lambda: _App(log, open_docs))
|
||||||
monkeypatch.setattr(cb, "_acad_doc", lambda: active)
|
monkeypatch.setattr(cb, "_acad_doc", lambda: active)
|
||||||
return backend, active
|
return backend, active
|
||||||
|
|
||||||
@@ -342,3 +351,19 @@ async def test_a_dwg_on_the_live_engine_is_opened_read_only_and_closed_unsaved(
|
|||||||
assert ("open", "plant.dwg", True) in log
|
assert ("open", "plant.dwg", True) in log
|
||||||
assert ("close", False) in log
|
assert ("close", False) in log
|
||||||
assert log[-1] == ("ensure_document_state",)
|
assert log[-1] == ("ensure_document_state",)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_a_dwg_already_open_is_read_from_its_document_and_never_closed(monkeypatch, tmp_path):
|
||||||
|
# Opening a drawing AutoCAD already has open, then closing "it" unsaved, could
|
||||||
|
# close the operator's copy and lose its unsaved work. The open document is
|
||||||
|
# exported as it stands and left open: the server closes only what it opened.
|
||||||
|
path = tmp_path / "plant.dwg"
|
||||||
|
path.write_bytes(b"AC1032")
|
||||||
|
log: list[tuple] = []
|
||||||
|
theirs = _Doc("plant.dwg", log, full_name=str(path))
|
||||||
|
backend, _active = _live(monkeypatch, log, open_docs=[_Doc("Drawing1.dwg", log), theirs])
|
||||||
|
snap = await take_snapshot(backend, str(path))
|
||||||
|
assert snap.source == "live:plant.dwg"
|
||||||
|
assert ("export", "snap", "DXF") in log
|
||||||
|
assert not [event for event in log if event[0] in ("open", "close")]
|
||||||
|
|||||||
Reference in New Issue
Block a user