Add a Save logs button, and take the obvious identifiers out on the way - #117
Merged
Conversation
Asking somebody to find ~/Library/Logs/OpenTagViewer/exporter.log reliably produces one of two wrong things: a screenshot of an error dialog, which never has the useful part in it, or their export zip - which holds the keys to their tags and cannot be un-shared once it is posted. The button takes Cancel's place, which did exactly what the window's own close button does and so earned none of that space. What it writes is deliberately unmistakable: a `.txt` with `logs` in the name, and a dialog that names both files rather than assuming the difference is obvious. **And it redacts, best effort.** Apple IDs, home directories, peer hashes, device serials, the name somebody gave their phone, keychain attributes printed verbatim, and record UUIDs. Values are numbered rather than blanked, so `<serial-1>` is the same serial everywhere it appears - "these two lines are about one device" is often the whole diagnosis, and blanket asterisks destroy exactly that. The model beside a device name stays, because it is not personal and is usually the point of the line. A library was considered and rejected. scrubadub pulls scikit-learn, textblob and faker; presidio pulls spacy, numpy and pydantic. Both are large in a frozen bundle, and neither knows what an Apple serial, a Cuttlefish peer hash or a keychain acct field looks like - they are built for prose. Everything worth removing here has a known shape. Nothing claims the result is clean, and the dialog says so: the text comes from a library reading Apple's structures, so a field that is harmless on one account may not be on another. Two suites. test_redact.py covers the patterns without a display, including that timestamps, module names, byte counts and model numbers survive - redaction that eats the diagnosis just earns a second request for the original. test_save_logs_button.py drives the real window, because a window writing an unredacted file looks exactly like one writing a redacted file. Every identifier in those fixtures is randomly generated. The first draft used values from a real user's bug report, which is precisely what this feature exists to prevent and would have put them in the history permanently.
CI went red on a test file that passes locally. `import tkinter` at module scope runs during collection, so a Python built without Tk fails the whole session before any fixture can decide to skip - the `pytest.skip` in the window fixture never got a chance. That is the same lesson as "Stop the headless CLI needing a window", which moved code out of wizard.py for exactly this reason, and I did not apply it to the test that came with it. Worth knowing for reading the run: the matrix is fail-fast, so 3.12 failed and cancelled the other three - and `gh pr checks` reported three failures whose every step had succeeded. Only 3.12 had anything wrong with it, and only in that its interpreter had no _tkinter while 3.13's did. `pytest.importorskip` now guards the module, before `exporter.wizard` is imported, since that pulls in tkinter too. Verified by hiding _tkinter from the import system: the file skips rather than erroring, and still runs its eleven tests where Tk exists.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Asking somebody to find
~/Library/Logs/OpenTagViewer/exporter.logreliably produces one of two wrong things: a screenshot of an error dialog, which never has the useful part in it, or their export zip — which holds the keys to their tags and cannot be un-shared once it is posted.The button
It takes Cancel's place, which did exactly what the window's own close button does and so earned none of that space.
What it writes is deliberately unmistakable: a
.txtwithlogsin the name, and a dialog that names both files rather than assuming the difference is obvious.It redacts, best effort
Apple IDs, home directories (which usually carry a real name), peer hashes, device serials, the name somebody gave their phone, keychain attributes printed verbatim, and record UUIDs.
Values are numbered, not blanked —
<serial-1>is the same serial everywhere it appears. "These two lines are about one device" is often the whole diagnosis, and blanket asterisks destroy exactly that. The model beside a device name survives, because it is not personal and is usually the point of the line.On a real user's log from #89 it replaced 2 devices, 1 email, 3 peers, 2 serials and 1 username.
Why not a library
Considered and rejected on evidence, not taste:
scrubadubpulls scikit-learn, textblob, dateparser and fakerpresidio-analyzerpulls spacy, numpy and pydanticBoth are large in a frozen PyInstaller bundle, and neither knows what an Apple serial, a Cuttlefish peer hash or a keychain
acctfield looks like — they are built for prose. Everything worth removing here has a known shape, and a known shape is a regex.Nothing claims the result is clean
The dialog says the matching cannot promise it caught everything, and to read the file before posting it. A user told "this is safe now" will not read it. A test asserts that wording, so it cannot quietly become a promise.
Tests
Two suites, split on purpose.
test_redact.py— the patterns, no display needed. Includes that timestamps, module names, byte counts, model numbers and traceback line numbers survive: redaction that eats the diagnosis just earns a second request for the original.test_save_logs_button.py— drives the real window, because one that writes an unredacted file looks exactly like one that writes a redacted file.Writing them found three bugs: a
$withoutre.MULTILINEmeant the device rule only ever matched the last line of a file;acct=''became a fake<item-1>; and the model lookahead could not span the comma insideiPhone15,2.PR description summarised by Claude Code.