Skip to content

Refactor navigation state management: unify position and command stacks into a single history vector - #776

Merged
NSoiffer merged 3 commits into
daisy:mainfrom
moritz-gross:refactor/simplify-navigate-using-stack-datastructure
Sep 24, 2026
Merged

NSoiffer merged 3 commits into
daisy:mainfrom
moritz-gross:refactor/simplify-navigate-using-stack-datastructure

Conversation

@moritz-gross

Copy link
Copy Markdown
Collaborator

I have a hard time understanding what's happening in some files, so I'll keep suggesting some ways in how I think it can be done a bit smoother. Either my proposals are good enough to be merged, or I'll find out how I am misunderstanding the code ;)

regarding your comment in the code: I'm 99% sure Vec is the correct datastructure for this. Ideally, you want to use a contiguous memory layout, as opposed to pointer chasing in a linked list.
See for example also VecDeqeue for some other use cases where we might want to manipulate both ends. But for this, Vec seems perfect. Its usage as a stack is also documented in https://doc.rust-lang.org/std/vec/struct.Vec.html

@moritz-gross

Copy link
Copy Markdown
Collaborator Author

the failing test doesn't reproduce locally, so something about the CI setup is off.
See relevant part of the CI logs:

failures:

---- Languages::en::alphabets::cap_cyrillic stdout ----
Error: MathCAT crash! Please report the following information: 'MathCAT crash! Please report the following information: 'MathCAT crash! Please report the following information: 'RefCell already borrowed' at src/speech.rs:2811:0
' at tests/common/mod.rs:56:0' at tests/common/mod.rs:61:0

failures:
Languages::en::alphabets::cap_cyrillic

@moritz-gross moritz-gross added the rust Coding in Rust label Sep 20, 2026
@NSoiffer
NSoiffer merged commit e79dda7 into daisy:main Sep 24, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rust Coding in Rust

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants