Stop a deck file losing the lines that are not cards - #27
Merged
Conversation
Three ways a deck file could be damaged or a path mistyped, all found by pointing the app at files nobody had thought to point it at. Lines that did not parse as a card were dropped on load, so the next save -- after one answered card -- wrote the deck back without them. A heading, a blank separator, a line with a typo in it: gone, with nothing on screen to say so. The worst version was a mistyped path: `FlashTerm ~/notes.txt` opened happily, said "Loaded 0 flashcards", and replaced the whole file with the first card added to it. Such lines are now carried through load and save untouched, anchored to the card they sat above so a heading stays above its section and notes stay at the bottom. Blank lines are carried the same way, so a deck with sections in it round trips byte for byte and the no-op save check still holds. Opening a deck that has any says so once, on the way in, which is also what tells you the file was never a deck. A deck path that cannot hold a deck is refused before the menu rather than at the first save. Naming a directory, or a file under a directory that does not exist, both loaded an empty deck and opened as normal; the problem only surfaced after a card had been typed in. CRLF decks load clean. The carriage return was kept as part of each answer, invisible on screen but written back as a quoted "hello\r" and there forever after. Stripped where the line is parsed, so imports are covered too, and writing normalises to \n.
Merged
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.
Three ways a deck file could be damaged or a path mistyped, all found by pointing the app at files nobody had thought to point it at. Found while auditing before an announcement — the first one is the sort of thing that becomes the comment in the thread.
Lines that are not cards were silently deleted
Anything in a deck file that did not parse as a card was dropped on load, and the next save — after a single answered card — wrote the deck back without it. A heading, a note to yourself, a line with a typo in it: gone, with nothing on screen to say so.
The worst version was a mistyped path.
FlashTerm ~/notes.txtopened happily, reportedLoaded 0 flashcards, and replaced the file's entire contents with the first card you added to it.Non-card lines are now carried through load and save untouched, anchored to the card they sat above so headings stay above their section and notes stay at the bottom. Blank lines are carried the same way, so a deck with sections in it round trips byte for byte and the 0.3.1 no-op-save check still holds on it. Opening a deck that has any says so once, on the way in — which is also what tells you the file was never a deck:
Deliberately not a comment syntax:
#would have been the obvious choice and it would have broken#include,preprocessor directivein exactly the kind of deck this project ships examples for. Preservation needs no format change, and covers lines nobody would have thought to mark.A deck path that cannot hold a deck is refused up front
Naming a directory loaded an empty deck and opened as normal; so did naming a file under a directory that does not exist. Either way the problem only surfaced as a failed save, after a card had been typed in. Both are checked before the deck is opened now, for every mode, and exit 2 with the reason.
CRLF decks load clean
A deck written on Windows, or exported by a spreadsheet, kept the carriage return as part of each answer. Invisible on screen, but it went back to disk as a quoted
"hello\r"and stayed there. Stripped where the line is parsed, so imports are covered as well as decks, and writing normalises to\n.Tests
Five unit tests and three golden cases, all covering behaviour that had no coverage at all:
test_foreign_lines_survive_saving— the round trip, including that an untouched file with comments in it is still not rewrittentest_non_deck_file_is_not_emptied— the mistyped-path casetest_foreign_lines_outlive_their_cards— anchors point at cards, so deleting every card must leave the lines somewhere rather than nowheretest_crlf_deck_loads_clean,test_deck_path_errordeck-foreign-lines(two cards answered, all three foreign lines still in place in the transcript's file dump),deck-path-is-a-directory,deck-path-no-directory1233 unit checks and 50 golden cases pass, in both the optimised and the
-fsanitize=address,undefinedbuild.