Skip to content

fix(replay_bloc): respect a lowered limit - #4902

Open
ChandraChaan wants to merge 2 commits into
felangel:masterfrom
ChandraChaan:fix/replay-bloc-lowered-limit
Open

ChandraChaan wants to merge 2 commits into
felangel:masterfrom
ChandraChaan:fix/replay-bloc-lowered-limit

Conversation

@ChandraChaan

Copy link
Copy Markdown

Status

READY

Breaking Changes

NO

Description

limit can be changed after construction through the public limit setter, but _ChangeStack only enforced it partly:

  • Setting limit didn't trim the existing history, so undo() could still go back past the new limit.
  • add removed at most one entry per change (if instead of while), so after lowering the limit the history never shrank to it.

_ChangeStack.limit is now a setter that trims the history right away, and add uses the same trimming, which removes entries until the history fits. Behaviour for an unchanged limit is the same as before: null or a negative limit keeps unlimited history, and 0 records nothing.

Added two tests: undo after lowering the limit, and the next change after lowering the limit.

Fixes #4901

Type of Change

  • ✨ New feature (non-breaking change which adds functionality)
  • 🛠️ Bug fix (non-breaking change which fixes an issue)
  • ❌ Breaking change (fix or feature that would cause existing functionality to change)
  • 🧹 Code refactor
  • ✅ Build configuration change
  • 📝 Documentation
  • 🗑️ Chore

@uveshm003 uveshm003 left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

redo() adds back to _history without trimming, so redoing after lowering the limit goes past it:

cubit
  ..increment()..increment()..increment()
  ..undo()..undo()
  ..limit = 1
  ..redo()..redo()
  ..undo()..undo()..undo(); // 3 undos with limit 1 -> states end [..., 2, 1, 0]

Calling _trimHistory() at the end of redo() fixes it.

@ChandraChaan

Copy link
Copy Markdown
Author

Good catch @uveshm003, thanks! Reproduced it: with your sequence the states ended [1, 2, 3, 2, 1, 2, 3, 2, 1, 0], so the redos put the history back above the lowered limit.

Fixed in b67c143: redo() now calls _trimHistory() after moving the change back onto the history, and I added your sequence as a test (trims history to a lowered limit on redo), which now ends at [1, 2, 3, 2, 1, 2, 3, 2]. All 87 replay_bloc tests pass and lib is still at 100% coverage.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: replay_bloc keeps more undo history than limit after limit is lowered

2 participants