fix(editor): discard a rich note nobody typed into - #166
Merged
Conversation
Opening a note in the extended editor is enough to make the editor serialize a document — an empty paragraph, with an id — and emptiness was decided by comparing that string against "[]". So a new note left untouched read as meaningful: closeActivity() skipped the "new empty note → delete" branch, needsSave() actively wrote it, and the note showed up in the list and in the statistics. Emptiness now comes from the parsed blocks (EditorDocument): a block counts unless it is a text block whose text is blank, and any file reference counts. Only the empty paragraph is written without the user asking for it, so a delimiter, a table or a tool this build cannot read is kept, and an unreadable document is kept rather than discarded. The attachments check had the same shape of bug — every extended-editor change writes the parsed attachment list, which is the string "[]" when there are no files, and a non-blank field read as "has attachments", which alone kept the empty note alive. It now asks Note.isAttachments(). That leaves the extended-editor branch inside closeActivity() (the one its own comment called wrong) unreachable, since needsSave() already requires content; removed. Closes #165
Test coverage (unit + instrumentation)
Files
|
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.
Fixes #165.
Opening a note in the extended editor is enough to make the editor serialize a document — an empty paragraph, with an id — and emptiness was decided by comparing that string against
"[]". So a new note left untouched read as meaningful:closeActivity()skipped the "new empty note → delete" branch,needsSave()actively wrote it, and the note showed up in the list and in the statistics.What changed
EditorDocument.hasContent()(new) decides emptiness from the parsed blocks. A block counts as content unless it is a text block whose text is blank; any file reference counts, read through the existingEditorAttachmentBlockswalk, sodata.fileanddata.files[]are both covered. Only the empty paragraph is written without the user asking for it, so a delimiter, a table, or a tool this build cannot read is kept, and an unreadable document is kept rather than discarded. Free ofandroid.*, so the rule is exercised by ordinary JVM tests."[]"when there are no files, and "the field is not blank" read that as "this note has attachments" — which alone kept the empty note alive even once the document check was right. It now asksNote.isAttachments(), the same method that already decides which editor a note opens in.closeActivity()that its own comment called wrong. It is unreachable:needsSave()already requires content.Behaviour worth knowing about
An existing note the user empties is no longer saved — the stored text stays and the note is not deleted either. That is what the simple editor has always done, and the parity this issue asks for, but it is a change for rich notes, so
existingNoteEmptiedByTheUserKeepsWhatWasStoredpins it as a deliberate decision rather than an accident.Tests
17 new unit tests.
EditorDocumentTestcovers the shapes the issue lists —[], a single empty paragraph, whitespace and empty markup, empty header and list, an attachment as the only content — plus image/gallery blocks, a delimiter, a table, an unknown tool, and an unreadable document.NotePresenterEmptyExtendedNoteTestcovers the close path end to end; its two "empty note" tests fail against the old check and pass against the new one.Full unit suite,
lintDebugandspotlessCheckare green.