Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ on:

jobs:
build-and-test:
# Two compilers times two build modes, so four jobs.
# Two platforms, two compilers and two build modes, minus a combination
# that would be a duplicate, so six jobs.
#
# Both compilers, so a warning or a standard-library difference that only
# one of them notices still fails the build.
Expand All @@ -18,11 +19,26 @@ jobs:
# the optimised build is what ships, and the sanitised one is the only
# thing that reports a read past the end of a buffer rather than returning
# whatever was next to it.
#
# Both platforms, because everything the app does outside the standard
# library is POSIX rather than Linux -- termios, ioctl, dirent, wcwidth --
# and "should be portable" is a claim nobody had checked. The macOS runner
# is Apple Silicon, so it also builds for arm64, where `char` is unsigned
# by default and x86 says it is signed. That is a real class of bug this
# cannot otherwise see.
strategy:
fail-fast: false
matrix:
os: [ubuntu-latest, macos-latest]
cxx: [g++, clang++]
mode: [optimised, sanitised]
exclude:
# `g++` on a macOS runner is a shim for Apple clang rather than GCC,
# so this pair would run the clang entry again under another name.
# Real GCC is a `brew install` away; what the macOS runner is here to
# catch is the platform, not a third compiler.
- os: macos-latest
cxx: g++
include:
# -Werror lives here rather than in the Makefile on purpose: a
# warning should stop a change being merged, not stop a contributor
Expand All @@ -40,7 +56,7 @@ jobs:
-fsanitize=address,undefined -fno-sanitize-recover=undefined
-fno-omit-frame-pointer

runs-on: ubuntu-latest
runs-on: ${{ matrix.os }}

# Set for the job rather than passed to each make: the Makefile takes both
# with ?=, and the test and golden targets have to be built with the same
Expand Down
34 changes: 34 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,40 @@
Notable changes per release. Dates are the release date; the PR numbers link the
detail, which is where the reasoning lives.

## Unreleased

### Fixed

- **`FLASHTERM_SEED` now fixes the review order on every platform, not just the
one you built on.** The shuffle went through `std::shuffle`, whose output the
standard does not specify — only that the permutation is uniformly random —
so libstdc++ and libc++ deal the same seeded deck in different orders. It is
now an explicit Fisher–Yates over `std::mt19937`, whose own output *is*
specified exactly, so the seed alone decides the order.

Found by the macOS runner below, on its first run, which is the entire
argument for having added it: every multi-card golden case failed there,
because a scripted session answers cards in the order it expects to meet
them and the transcripts were recorded against libstdc++. Nothing was wrong
with the shuffle's randomness, and no real session is affected — the bug was
in what the seed promised.

### Changed

- **CI builds and tests on macOS as well as Linux.** Everything FlashTerm does
outside the standard library is POSIX rather than Linux — `termios`, `ioctl`,
`dirent`, `wcwidth` — and the time functions have had their `gmtime_r` and
`localtime_r` branches since the log landed, so this was expected to pass.
Expected is not tested, and "runs on a Mac" is the kind of claim a README
should not make on the strength of reading the source.

The macOS runner is Apple Silicon, so it also builds for arm64. That is the
half worth having beyond the platform itself: `char` is unsigned there and
signed on x86, which is a real difference that no amount of running on one
architecture can show. `g++` on macOS is a shim for Apple clang rather than
GCC, so that pair is excluded rather than run as a second clang under another
name — six jobs, not eight.

## 0.3.0 — 2026-08-19

Pictures on cards, sync that puts two machines' reviews back together, and a CI
Expand Down
7 changes: 5 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ FlashTerm is a terminal flashcard app that makes you type the answer. No multipl

## Quick Start

Needs a C++17 compiler (`g++` or `clang++`) and `make`.
Needs a C++17 compiler (`g++` or `clang++`) and `make`. Linux and macOS are
both built and tested on every change; anything else POSIX will very likely
work, since nothing outside the standard library is used that POSIX does not
define.

```bash
git clone https://github.com/Aduneer/FlashTerm
Expand Down Expand Up @@ -633,7 +636,7 @@ cannot drive an app that insists on a tty.

## Contributing

Issues and pull requests are welcome. `make check` should pass before you open one; CI runs both suites on gcc and clang, in an optimised build and again under `-fsanitize=address,undefined`, all four with `-Werror`. Warnings are deliberately not errors in the Makefile itself, so a warning never stops you building — it stops the change being merged.
Issues and pull requests are welcome. `make check` should pass before you open one; CI runs both suites on Linux and macOS, on gcc and clang, in an optimised build and again under `-fsanitize=address,undefined`, all of them with `-Werror`. Warnings are deliberately not errors in the Makefile itself, so a warning never stops you building — it stops the change being merged.

Released versions and what changed in them are in [CHANGELOG.md](CHANGELOG.md).

Expand Down
50 changes: 48 additions & 2 deletions src/review.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#include <algorithm>
#include <cerrno>
#include <cstdint>
#include <cstdlib>
#include <functional>
#include <iomanip>
Expand Down Expand Up @@ -89,6 +90,51 @@ std::mt19937& rng() {
return generator;
}

// An index in [0, bound), drawn by hand rather than with
// std::uniform_int_distribution, whose mapping from generator output to result
// is not specified and does differ between standard libraries. See
// deterministic_shuffle.
//
// Rejection sampling: mt19937 covers the whole 32-bit range, so draws from the
// last, short block above the largest exact multiple of `bound` are discarded
// rather than folded in, which would make the low values fractionally likelier.
std::uint32_t bounded(std::uint32_t bound) {
const std::uint64_t range = std::uint64_t{1} << 32;
const std::uint64_t limit = range - (range % bound);
std::uint64_t draw;
do {
draw = rng()();
} while (draw >= limit);
return static_cast<std::uint32_t>(draw % bound);
}

// Fisher-Yates, written out rather than calling std::shuffle.
//
// std::shuffle's output is not specified by the standard -- only that the
// result is a uniformly random permutation -- so libstdc++ and libc++ produce
// *different orders from the same seed*. FLASHTERM_SEED is documented as
// fixing the review order, and a promise that holds only on the standard
// library you happened to build against is not the promise it makes. The
// golden suite is what noticed: every multi-card case failed on macOS, because
// a scripted session answers cards in the order it expects to meet them, and
// the transcripts were recorded against libstdc++.
//
// mt19937 itself is safe to keep -- the standard specifies its algorithm
// exactly, down to a required value for the 10000th draw -- so pinning the
// permutation is only a matter of not letting the library choose how the
// numbers become indices.
template <typename Iterator>
void deterministic_shuffle(Iterator first, Iterator last) {
const std::size_t count = static_cast<std::size_t>(last - first);
for (std::size_t i = count; i > 1; --i) {
const std::size_t pick = bounded(static_cast<std::uint32_t>(i));
if (pick != i - 1) {
std::iter_swap(first + static_cast<std::ptrdiff_t>(i - 1),
first + static_cast<std::ptrdiff_t>(pick));
}
}
}

int clamp_box(int box) { return std::min(kMaxBox, std::max(1, box)); }

bool card_has_any_tag(const Flashcard& card,
Expand Down Expand Up @@ -273,7 +319,7 @@ void order_by_box(CardRefs* refs) {
}
refs->clear();
for (int box = 1; box <= kMaxBox; ++box) {
std::shuffle(boxes[box].begin(), boxes[box].end(), rng());
deterministic_shuffle(boxes[box].begin(), boxes[box].end());
refs->insert(refs->end(), boxes[box].begin(), boxes[box].end());
}
}
Expand Down Expand Up @@ -651,7 +697,7 @@ void review_flashcards(Deck& deck) {
return;
}

std::shuffle(matches.begin(), matches.end(), rng());
deterministic_shuffle(matches.begin(), matches.end());
if (session.mode == kModeBox && filters.leitner_box == 0) {
order_by_box(&matches);
} else if (filters.due_only) {
Expand Down
3 changes: 1 addition & 2 deletions tests/golden/cases/review-audio-unplayable/env
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
FLASHTERM_TTS=/bin/false
FLASHTERM_PLAYER=/bin/false
FAKE_AUDIO_FAIL=1
30 changes: 15 additions & 15 deletions tests/golden/cases/review-multi-card/expected
Original file line number Diff line number Diff line change
Expand Up @@ -45,25 +45,25 @@ Next review in 3 days (<DATE1>).
> Progress: [████████░░░░░░░░░░░░] 40% (2/5 cards)

┌──────────────────────────────────────────────────────────────┐
│ Box 1 · new · french;greeting
│ Box 1 · new · french
├──────────────────────────────────────────────────────────────┤
│ │
Bonjour
Au revoir
│ │
└──────────────────────────────────────────────────────────────┘

[Enter] submit [a] play audio [?] hint [q] end session
Your answer: Progress: [████████░░░░░░░░░░░░] 40% (2/5 cards)

┌──────────────────────────────────────────────────────────────┐
│ Box 1 · new · french;greeting
│ Box 1 · new · french
├──────────────────────────────────────────────────────────────┤
│ │
Bonjour
Au revoir
│ │
└──────────────────────────────────────────────────────────────┘

Hint: H····
Hint: G······
[Enter] submit [a] play audio [q] end session
Your answer:
✅ Correct!
Expand All @@ -75,16 +75,16 @@ Next review in 1 day (<DATE2>).
> Progress: [████████████░░░░░░░░] 60% (3/5 cards)

┌──────────────────────────────────────────────────────────────┐
│ Box 1 · new · french
│ Box 1 · new · french;animal
├──────────────────────────────────────────────────────────────┤
│ │
Au revoir
Chien
│ │
└──────────────────────────────────────────────────────────────┘

[Enter] submit [a] play audio [?] hint [q] end session
Your answer:
❌ Incorrect! Correct answer: Goodbye
❌ Incorrect! Correct answer: Dog
Next review in 1 day (<DATE2>).

[Enter] next card [a] hear the question [e] edit this card
Expand All @@ -110,10 +110,10 @@ Next review in 3 days (<DATE1>).
> Progress: [████████████████████] 100% (5/5 cards)

┌──────────────────────────────────────────────────────────────┐
│ Box 1 · new · french;animal
│ Box 1 · new · french;greeting
├──────────────────────────────────────────────────────────────┤
│ │
Chien
Bonjour
│ │
└──────────────────────────────────────────────────────────────┘

Expand Down Expand Up @@ -152,14 +152,14 @@ Next review in 3 days (<DATE1>).
--- exit status ---
0
--- file deck.txt ---
Bonjour,Hello,french;greeting,0,1,1,<DATE3>,<DATE2>,<ID1>
Bonjour,Hello,french;greeting,1,0,2,<DATE3>,<DATE1>,<ID1>
Merci,Thanks|Thank you,french,1,0,2,<DATE3>,<DATE1>,<ID2>
Au revoir,Goodbye,french,0,1,1,<DATE3>,<DATE2>,<ID3>
Chat,Cat,french;animal,1,0,2,<DATE3>,<DATE1>,<ID4>
Chien,Dog,french;animal,1,0,2,<DATE3>,<DATE1>,<ID5>
Chien,Dog,french;animal,0,1,1,<DATE3>,<DATE2>,<ID5>
--- file deck.txt.log ---
<ID6>,<ID2>,<TIME>,n,correct,1,2,
<ID7>,<ID1>,<TIME>,n,partial,1,1,
<ID8>,<ID3>,<TIME>,n,incorrect,1,1,
<ID7>,<ID3>,<TIME>,n,partial,1,1,
<ID8>,<ID5>,<TIME>,n,incorrect,1,1,
<ID9>,<ID4>,<TIME>,n,correct,1,2,
<ID10>,<ID5>,<TIME>,n,correct,1,2,
<ID10>,<ID1>,<TIME>,n,correct,1,2,
4 changes: 2 additions & 2 deletions tests/golden/cases/review-multi-card/input
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@
Thank you

?
Hello
Goodbye

Hola

Cat

Dog
Hello


0
14 changes: 14 additions & 0 deletions tests/golden/fake-audio.sh
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,20 @@
#
# Called as `fake-audio.sh speak <text>`, `fake-audio.sh play <file>`, or
# `fake-audio.sh render <file>` with the text on standard input.
#
# FAKE_AUDIO_FAIL makes it report that it could not play, which is the only way
# a case can reach the "No audio available for this card" path. It exists
# because the alternative -- pointing FLASHTERM_TTS at a command that always
# fails -- is a fact about the machine rather than about FlashTerm, and it was
# wrong: the case named /bin/false, which does not exist on macOS, where
# `false` lives in /usr/bin. FlashTerm rejects an override it cannot execute
# and falls back to the built-in list, so the case quietly stopped testing a
# failure at all and was answered by macOS's own `say` instead. Nothing is
# recorded here, because nothing played.

if [ -n "${FAKE_AUDIO_FAIL:-}" ]; then
exit 1
fi

kind=$1
shift
Expand Down
8 changes: 6 additions & 2 deletions tests/golden/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,12 @@ emit_files() {
# multibyte data" that makes one awk behave differently from another.
# Detected by looking for a NUL, which is what every "is this text" check
# has always come down to.
size=$(wc -c <"$file")
stripped=$(LC_ALL=C tr -d '\000' <"$file" | wc -c)
# wc pads its count with leading spaces on a BSD userland, so on macOS the
# size reached the transcript as "<binary, 597 bytes>". Stripped
# rather than reformatted, since this has to stay a plain integer for the
# comparison below as much as for the printf above it.
size=$(wc -c <"$file" | tr -d '[:space:]')
stripped=$(LC_ALL=C tr -d '\000' <"$file" | wc -c | tr -d '[:space:]')
if [ "$size" -ne "$stripped" ]; then
printf -- '<binary, %s bytes>\n' "$size"
continue
Expand Down
Loading