Repository navigation
Layout backups: skip a widget whose app is missing, name blank labels, plain errors - #461
Conversation
…, and show plain errors
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd77b3cc69
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| private var skippedWidgets = 0 | ||
| private fun savedMessage(base: String) = if (skippedWidgets > 0) | ||
| base + " " + activity.resources.getQuantityString(R.plurals.backup_widgets_left_out, skippedWidgets, skippedWidgets) else base |
There was a problem hiding this comment.
When the activity is recreated while the document picker is open—for example after a rotation, fold, or process restart—restore() resumes the persisted export without calling encodeBackup(), but this field is reinitialized to zero. The file still omits the unavailable widgets, while savedMessage() no longer warns how many were omitted; persist the count alongside KEY_EXPORT or bind it to the pending export payload.
Useful? React with 👍 / 👎.
| val root = JSONObject(raw) | ||
| val version = root.strictInt("version") | ||
| require(version in 1..LAYOUT_BACKUP_VERSION) { "Unsupported layout backup version" } | ||
| require(version in 1..LAYOUT_BACKUP_VERSION) { BACKUP_NEWER_VERSION } |
There was a problem hiding this comment.
This assigns BACKUP_NEWER_VERSION to every value outside 1..LAYOUT_BACKUP_VERSION, including zero and negative versions. A malformed or unsupported-old file therefore tells the user to update Folio even though updating cannot make it valid; only versions above the supported maximum should receive the newer-version message, while lower values should use the ordinary invalid-backup error.
Useful? React with 👍 / 👎.
| * requirement", "Widget 3 is unavailable"). The raw text goes to Diagnostics, where it can be read and shared on purpose. | ||
| */ | ||
| private fun failureMessage(error: Throwable, fallback: Int): String { | ||
| caught("Backup: ${error::class.java.simpleName}: ${error.message}") { throw error } |
There was a problem hiding this comment.
When an I/O, provider, or parsing exception includes a file path, package, or user-supplied value, interpolating error.message into where causes Diagnostics.caught() to persist that value in the diagnostic trail. This bypasses the privacy behavior documented in Diagnostics.kt, where exception messages are deliberately excluded because the trail may be viewed or shared; pass a stable operation name and let caught record only the exception type.
Useful? React with 👍 / 👎.
1 participant
Fixes three problems in Settings > Backup from the 20 Oct code review.
Checked: LayoutBackupTest has new cases for each (unit tests and lint pass). Not tried on the phone.