This was broken by an SQLite upgrade - previously we received the rows
in ix_cards_sched order, but recent versions use a table scan for that
query when the order is unspecified. Solved by being explicit about the
order we expect results to arrive.
https://forums.ankiweb.net/t/skipping-new-cards/15410
* Enable access to old notetype name
* Set minimum height for ChangeNotetypeDialog
* Add bootstrap icons to change-notetype
* Move alert up and make it collapsible
* Tweak some CSS
- Add variables --sticky-bg and --sticky-border to StickyContainer
- Tweak base.css
* Add translatable string "(Nothing)"
* Rework ChangeNotetype screen
* Initially load option at newIndex and remaining options on focus
Optimization for big notetypes:
Should increase efficiency from O(n²) to O(n). Test on notetype with 500 templates shows significant improvement in load time (~10s down to ~1s).
* Try to satisfy rust test
* Change arrow direction depending on reading direction
+ add 0.5em top padding to main
* Create Alert.svelte
* Introduce CSS variable --pane-bg
* Revert "Initially load option at newIndex and remaining options on focus"
This reverts commit f42beee45c27dba9433d76217fb583b117fb5231.
* Final cleanup
* Refine padding/gutter
* Remove unnecessary stopPropagation of mathjax-overlay events
* Use CodeMirror component for MathjaxHandle
* Refactor ResizeObserver code in MathjaxHandle
* Wrap setRange in CodeMirror in try/catch
* Add Mathjax Editor bottom margin
* Add custom Enter and Shift+Enter shortcuts for the MathjaxHandle
* Format
* Move placeCaretAfter to domlib
* Move focus back to field after editing Mathjax
* Put Cursor after Mathjax after accepting
* Add delete button for Mathjax
* Change border color of mathjax menu
* Refactor into MathjaxMenu
* Put caretKeyword in variable
* Use one ResizeObserver for all Mathjax images
* Add minmimum width for Mathjax editor
* is still smaller than minimal window width
* Add bazel directories to .prettierignore and format from root
* exclude ftl/usage (dae)
the json files that live there are output from our tooling, and
formatting them means an extra step each time we want to update them
also exclude .mypy_cache, which is output by scripts/mypy*
* minor ftl tweak: newline -> new line (dae)
e5e47a31fe (r748827327)
+ switched assert_lower_middle_upper to a macro, so that when it fails,
the reported line number is the original call site, instead of one inside
the helper function
* Canonify import of i18n module
Should always be imported as `tr`, or `tr2` if there is a name collision
(Svelte).
* Add helper for garbage collecting ftl strings
Also add a serializer for ftl asts.
* Add helper for filter-mapping `DirEntry`s
* Fix `i18n_helpers/BUILD.bazel`
* run cargo-raze
* Refactor `garbage_collection.rs`
- Improve helper for file iterating
- Remove unused terms as well
- Fix issue with checking for nested messages by switching to a regex-
based approach (which runs before deleting)
- Some more refactorings and lint fixes
* Fix lints in `serialize.rs`
* Write json pretty and sorted
* Update `serialize.rs` and fix header
* Fix doc and remove `dbg!`
* Add binaries for ftl garbage collection
Also relax type constraints and strip debug tests.
* add rust_binary targets for i18n helpers (dae)
* add scripts to update desktop usage/garbage collect (dae)
Since we've already diverged from 2.1.49, we won't gain anything
from generating a stable json just yet. But once 2.1.50 is released,
we should run 'ftl/update-desktop-usage.sh stable'.
* add keys from AnkiMobile (dae)
* Mention caveats in `remove-unused.sh`
* Remove flooring in v3 scheduler code
It is no longer supposed to be an exact port of the old Python code.
* Rework v3 fuzzing
https://github.com/ankitects/anki/issues/1416#issuecomment-958208149
* Ensure length of fuzz range is larger than 1
Only for new intervals larger than 1 and respecting max review interval.
* add the beginnings of a unit test
* Clarify `fuzz_factor` doc string
* Fix Python tests for 2021 scheduler
* Fix fuzz test
1.0 is not a valid fuzz factor.
* Add tests for fuzzing in Rust
* Use range notation in fuzz factor doc
* Strip redundant tests
* Add description input to fields dialog
QLineEdit seems like the best option, as it saves space and motivates users to keep their descriptions concise.
* Add setDescriptions to note initialization script
Went for the extra function instead of including it in setFields to prevent potential add-on breakages.
* Add tooltip next to field name if description is set
* Refactor code according to suggestions
Set default tooltip placement to right instead of bottom
Use .get() for fld["description"]
Fix tab order in fields dialog
Swap out abbreviation "desc" for full length name to keep consistency
* Update Protobuf and Rust for description
Add description to notetypes.proto and schema11
Co-authored-by: RumovZ <RumovZ@users.noreply.github.com>
* Fix tooltips not updating with description
Remove redundant variable tooltipOptions
Update previousTooltip within reactive function
* Move LabelDescription out of LabelName
Co-authored-by: Henrik Giesel <hgiesel@users.noreply.github.com>
* Decrease icon size and fix alignment
Co-Authored-By: Henrik Giesel <hengiesel@gmail.com>
* the new key needs to be cleared from fields, not the notetype itself
Co-authored-by: RumovZ <RumovZ@users.noreply.github.com>
Co-authored-by: Henrik Giesel <hengiesel@gmail.com>
Co-authored-by: Damien Elmes <gpg@ankiweb.net>
We're getting an enum instead of an int in Qt6
normal/reversed have been renamed to ascending/descending; no add-ons
appear to be using the old versions.
`counts.learning` includes interday learning cards, so it is not
suitable to determine how many cards from the (intraday!) learning queue
are already included in the learning count when updating it.
* Only collect card stats on the backend ...
... instead of rendering an HTML string using askama.
* Add ts page Card Info
* Update test for new `col.card_stats()`
* Remove obsolete CardStats code
* Use new ts page in `CardInfoDialog`
* Align start and end instead of left and right
Curiously, `text-align: start` does not work for `th` tags if assigned
via classes.
* Adopt ts refactorings after rebase
#1405 and #1409
* Clean up `ts/card-info/BUILD.bazel`
* Port card info logic from Rust to TS
* Move repeated field to the top
https://github.com/ankitects/anki/pull/1414#discussion_r725402730
* Convert pseudo classes to interfaces
* CardInfoPage -> CardInfo
* Make revlog in card info optional
* Add legacy support for old card stats
* Check for undefined instead of falsy
* Make Revlog separate component
* drop askama dependency (dae)
* Fix nightmode for legacy card stats
Python's regex engine performs pathologically on regexes like
'<!--.*?-->' when fed a large string of repeating '<!--' clauses.
Thanks to JaimeSlome / security@huntr.dev for the report; closes#1380.
Solved by switching to the Rust implementation, which does not suffer
from this issue.
entsToText(), minimizeHTML(), and the old regex constants have been
removed; they do not appear to be used by any add-ons.
The 'avoid showing learning card twice' logic is now only applied
when the next learning card was already due to be shown. This'll mean
there will be cases where a learning card does get shown twice near
the end, but it makes the behaviour easier to reason about, for both
us and end users.
Matches should arrive in alphabetical order. Currently results are not
capped (JS should be able to handle ~1k tags without too much hassle),
and no reordering based on match location is done. Matches are substring
based, and multiple can be provided, eg "foo::bar" will match
"foof::baz::abbar".
This is not hooked up properly on the frontend at the moment -
updateSuggestions() seems to be missing the most recently typed character,
and is not updating the list of completions half the time.
There were a few issues going on here:
- If some operation had invalidated the queues, they were subsequently
recreated with a call to .get_queues() in the undo handling code. This
could happen after the changes to the card had already been reverted,
leading to a queue state that didn't match our expectations.
- More generally, it's not safe to assume our mutations will apply
cleanly after the queue has been rebuilt. The next card will vary
depending on the number of remaining cards when interspersing cards of
different types, and a queue-invalidating operation will have changed
the learning cutoff.
So rather than rebuilding the queues on demand, we now check that they
already exist, and were created at the time we expect. If not, we
invalidate them and skip applying the mutations, and a subsequent
refresh of the UI should rebuild the queues correctly.
As part of this change, the cutoff snapshot has been moved into the
normal answer update object.
One possible downside here is that adding a note during review may cause
a newly due learning card to appear when undoing a different review.
If this proves to be a problem, we could potentially note down the
learning cutoff and apply it when queues are rebuilt later.
Context: https://forums.ankiweb.net/t/more-cards-today-question-about-v3/12400/10
Previously, interday learning cards and reviews were gathered at the
same time in v3, with the review limit being applied to both of them. The
order cards were gathered in would change the ratio of gathered learning
cards and reviews, but as they were displayed together in a single count,
a changing ratio was not apparent, and no special handling was required
by the deck tree code.
Showing interday learning cards in the learning count, while still
applying a review limit to them, makes things more complicated, as
a changing ratio will result in different counts. The deck tree code
is not able to know which order cards will appear in, so without changes,
we would have had a situation where the deck list may show different counts
to those seen when clicking on a deck.
One way to solve this would have been to introduce a separate limit for
interday learning cards. But this would have meant users needed to
juggle two different limits, instead of having a single one that controls
total number of (non-intraday) cards shown.
Instead, the scheduler now fetches interday cards prior to reviews -
the rationale for that order is that learning cards tend to be more
fragile/urgent than reviews. The option to show learning cards
before/after/mixed with reviews still exists, but it applies only after
cards have been capped to the daily limit.
To ensure the deck tree code matches the counts the scheduler gives,
it too applies limits to interday learning cards first, and reviews
afterwards.
In the old HTML editor, filenames were % escaped before feeding them to
beautifulsoup, causing bare ampersands to be left alone. The new HTML
editor reads content from the DOM, where a bare ampersand has been
transformed into an &, and that gets saved back into the field,
so the media check now needs to deal with it for images as well.
https://forums.ankiweb.net/t/causing-problems-with-image-names/12171
Interday learning cards are now counted in the learning count again,
and are no longer subject to the daily review limit.
The thinking behind the original change was that interday learning cards
are scheduled more like reviews, and counting them in the review count
would allow the learning count to focus on intraday learning - the red
number reflecting the fact that they are the most fragile memories. And
counting them together made it practical to apply the review limit
to both at once.
Since the release, there have been a number of users expecting to see
interday learning cards included in the learning count (the latest being
https://forums.ankiweb.net/t/feedback-and-a-feature-adjustment-request-for-2-1-45/12308),
and a good argument can be made for that too - they are, after all, listed
in the learning steps, and do tend to be harder than reviews. Short of
introducing another count to keep track of interday and intraday learning
separately, moving back to the old behaviour seems like the best move.
This also means it is not really practical to apply the review limit to
interday learning cards anymore, as the limit would be split between two
different numbers, and how much each number is capped would depend on
the order cards are introduced. The scheduler could figure this out, but
the deck list code does not know card order, and would need significant
changes to be able to produce numbers that matched the scheduler. And
even if we ignore implementation complexities, I think it would be more
difficult for users to reason about - the influence of the review limit
on new cards is confusing enough as it is.
The v3 scheduler will delay the final card from being shown twice in
a row, but the overdue case was being treated the same as the no-learning
case, leading to the message being hidden.
Previously we would just use 250% ease for any new card that had no
pre-configured ease, but this will result in decks that have
non-standard ease values to have "set due date" cards in them that don't
match. In order to make this somewhat more efficient, we cache
deckid->ease lookups during this operation.
Ref: <https://forums.ankiweb.net/t/set-due-date-doesnt-obey-default-ease-factor/9184>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Unfortunately a popular note taking tool has been misusing cloze
markers in its deck exports. We may want to add this back in the
future, but we'll probably want to start by warning users, to give
people time to adjust.
In order to split backend.proto into a more manageable size, the protobuf
handling needed to be updated. This took more time than I would have
liked, as each language handles protobuf differently:
- The Python Protobuf code ignores "package" directives, and relies
solely on how the files are laid out on disk. While it would have been
nice to keep the generated files in a private subpackage, Protobuf gets
confused if the files are located in a location that does not match
their original .proto layout, so the old approach of storing them in
_backend/ will not work. They now clutter up pylib/anki instead. I'm
rather annoyed by that, but alternatives seem to be having to add an extra
level to the Protobuf path, making the other languages suffer, or trying
to hack around the issue by munging sys.modules.
- Protobufjs fails to expose packages if they don't start with a capital
letter, despite the fact that lowercase packages are the norm in most
languages :-( This required a patch to fix.
- Rust was the easiest, as Prost is relatively straightforward compared
to Google's tools.
The Protobuf files are now stored in /proto/anki, with a separate package
for each file. I've split backend.proto into a few files as a test, but
the majority of that work is still to come.
The Python Protobuf building is a bit of a hack at the moment, hard-coding
"proto" as the top level folder, but it seems to get the job done for now.
Also changed the workspace name, as there seems to be a number of Bazel
repos moving away from the more awkward reverse DNS naming style.
Instead of calling a method inside the transaction body, routines
can now pass Op::SkipUndo if they wish the changes to be discarded
at the end of the transaction. The advantage of doing it this way is
that the list of changes can still be returned, allowing the sync
indicator to update immediately.
Closes#1252
- changes can now be undone
- the same field can now be mapped to multiple target fields, allowing
fields to be cloned
- the old Qt dialog has been removed
- the old col.models.change() API calls the new code, to avoid
breaking existing consumers. It requires the field map to always
be passed in, but that appears to have been the common case.
- closes#1175
Multiple configs with the same inner id would lead to errors like the
following when trying to open the collection:
DeckConfigInner.interval_multiplier: invalid wire type: StartGroup (expected ThirtyTwoBit)
This makes the review backlog case more expensive, since we end up
shuffling items outside the daily limit, but for the common case it's
about the same speed, and it means we don't need two separate sorting
steps. New cards remain handled the same way, since a backlog
is common there.
Also ensures that interday learning cards honor the deck sorting, and
that the non-default sort orders shuffle at the end.
Like the previous change, avoid exposing the protobuf as a public API
for now. It requires more thought, and is probably better done with
either extra helper accessors like decks.name(), or via a native class.
Combine existing check for unparsable templates with a check for unknown
field names and a check for front sides without any field replacement.
Updating the notetype's fields now mutates the parsed templates, so the
checks can run on the final templates.
This could potentially help us avoid having to refetch the notetype
during study in the future, though updates to Note initialization and
the LaTeX handling would be required first.
- The "unbury deck" option was broken, as it was ignoring child
decks. It would be nice if we could use active_decks instead, but
plugging that into the old scheduler without breaking undo seems a bit
tricky.
- Remove the implicit From impl for decks, so we need to be forced to
think about whether we want child decks or not.
- Daily limits are no longer inherited - each deck limits its own
cards, and the selected deck enforces a maximum limit.
- Fetching of review cards now uses a single query, and sorts in advance.
In collections with a large number of overdue cards and decks, this is
faster than iterating over each deck in turn.
- Include interday learning count in review count & review limit, and
allow them to be buried.
- Warn when parent review limit is lower than child deck in deck options.
- Cap the new card limit to the review limit.
- Add option to control whether new card fetching short-circuits.
Instead of using a separate undo queue, the code now defers checking for
newly-due learning cards until the answering stage, and logs the updated
cutoff time as an undoable change, so that any newly-due learning cards
won't appear instead of a new/review card that was just undone.
Queue redo now uses a similar approach to undo, instead of rebuilding the
queues.
The original rationale was avoiding a possible O(n) insertion if
the learning card was due outside the cutoff, but the increased code
complexity doesn't seem worth it, given that learning cards will
rarely grow above 1000.
Also added a currently-disabled test that demonstrates the current undo
handling behaviour is yielding incorrect counts; that will be reworked
in the next commit, and this change will make that easier.
- split new card fetch order and subsequent sort order; use latter
when building queues
- default to spacing siblings when burying is off, with options to
show each sibling in turn, and shuffle the fetched cards
The bury new/review flags are now pulled from each card's home deck,
instead of using a global setting that had not been hooked up. This
unfortunately means we need to fetch the map of all decks up front, as
we need to be able to look up a deck configuration for cards that are
in filtered decks.
Fixes a "card was modified" error caused by cards being buried during
review, when they weren't removed up-front.
Avoids duplicate work, and is a step towards allowing the next
states to be modified by third-party code.
Also:
- fixed incorrect underlined count, due to reviews being labeled as
learning cards
- fixed reviewer not refreshing when undoing a test review, by splitting
up backend queue rebuilding from frontend reviewer refresh
- moved answering into a CollectionOp
Allows add-on authors to define their own label for a group of undoable
operations. For example:
def mark_and_bury(
*,
parent: QWidget,
card_id: CardId,
) -> CollectionOp[OpChanges]:
def op(col: Collection) -> OpChanges:
target = col.add_custom_undo_entry("Mark and Bury")
col.sched.bury_cards([card_id])
card = col.get_card(card_id)
col.tags.bulk_add(note_ids=[card.nid], tags="marked")
return col.merge_undo_entries(target)
return CollectionOp(parent, op)
The .add_custom_undo_entry() is for adding your own custom actions.
When extending a standard Anki action, instead store `target =
col.undo_status().last_step` after executing the standard operation.
This started out as a bigger refactor that required a separate
.commit_undoable() call to be run after each operation, instead of
having each operation return changes directly. But that proved to be
somewhat cumbersome in unit tests, and ran the risk of unexpected
behaviour if the caller invoked an operation without remembering to
finalize it.
- backend now updates current notetype as part of addition
- frontend no longer implicitly adds, so we can assign a new name and
add in a single operation
- tokio 1.0
- updated reqwest, thanks to Rumo
- other minor dep updates
the reqwest build file has been split into two, as it was awkward
to manually update the combined file, and the platform gate is now
on the target in rslib/
The deck name must be constructed by calling associated functions of
NativeDeckName, unless the name is guaranteed to be valid machine
name (like "Default").
NativeDeckName exposes methods to mutate the deck name and return
the human name.
The storage routines take &strs, but those should be slices of
NativeDeckNames to ensure machine form and normalization.
Instead, fetch the config order on the frontend and pass a builtin
variant into the backend.
That makes the following unnecessary:
* Resolving the config sort in search/mod.rs
* Deserializing the Column enum
* Config accessors for the sort columns
* Remove duplicate backend columns
* Remove duplicate column routines
* Move columns on frontend from state to model
* Generate available columns from Colum enum
* Add second column label for notes mode
- make sure we set flag in changes when config var changed
- move current deck get/set into backend
- set_config() now returns a bool indicating whether a change was
made, so other operations can be gated off it
- active decks generation is deferred until sched.reset()
remove_note() now returns the count of removed cards, allowing us
to unify the tooltip between browser and review screen
I've left the old translation in - we'll need to write a script at
one point that gathers all references to translations in the code,
and shows ones that are unused.
- pass the handler directly
- reviewer special-cases for flags and notes are now applied at
call site
- drop the kind attribute on OpChanges which is not needed
Like notetypes, there is a col.get_deck() routine which caches
fetches, so that successive fetches are cheap. This makes it simpler
to just fetch the deck at the start.
We were also attempting to fetch a deck with id 0 for each row; I've
changed this so that we only fetch it if the id is non-zero.
I18n uses an Arc internally, so it is cheap to clone. This allow us
to drop the lifetime specifiers on the context structures.
The backend knows exactly which op has executed, and it saves us having
to re-implement this logic on each client.
Fixes the browser table refreshing when toggling decks.
Updating a deck via protobuf is now exposed on the backend, but not
currently on the frontend - I suspect we'll be better off writing
separate routines for the actions we need instead, and we get a better
undo description for free.
This is currently causing an ugly redraw in the browse screen, which
will need fixing.
- use strum to generate an iterator for the protobuf enum so we don't
forget to add new labels if extending in the future
- no add-ons appear to be using dynOrderLabels(), so it has been removed
@RumovZ perhaps a similar approach might work for listing the available
browser columns as well?
Older translations will note have the $notetype variable, but that is
not an error in Fluent - it would only cause problems if we tried to
use the new string on older Anki versions.
So, this is fun. Apparently "DeckId" is considered preferable to the
"DeckID" were were using until now, and the latest clippy will start
warning about it. We could of course disable the warning, but probably
better to bite the bullet and switch to the naming that's generally
considered best.
It appears that including the build script as a dependency is not
enough to make the file available at runtime, so the build breaks
if ts/ is built first.
Instead of generating a fluent.proto file with a giant enum, create
a .json file representing the translations that downstream consumers
can use for code generation.
This enables the generation of a separate method for each translation,
with a docstring that shows the actual text, and any required arguments
listed in the function signature.
The codebase is still using the old enum for now; updating it will need
to come in future commits, and the old enum will need to be kept
around, as add-ons are referencing it.
Other changes:
- move translation code into a separate crate
- store the translations on a per-file/module basis, which will allow
us to avoid sending 1000+ strings on each JS page load in the future
- drop the undocumented support for external .ftl files, that we weren't
using
- duplicate strings in translation files are now checked for at build
time
- fix i18n test failing when run outside Bazel
- drop slog dependency in i18n module
- Filtered deck creation now happens as an atomic operation, and is
undoable.
- The logic for initial search text, normalizing searches and so on
has been pushed into the backend.
- Use protobuf to pass the filtered deck to the updated dialog, so
we don't need to deal with untyped JSON.
- Change the "revise your search?" prompt to be a simple info box -
user has access to cancel and build buttons, and doesn't need a separate
prompt. Tweak the wording so the 'show excluded' button should be more
obvious.
- Filtered decks have a time appended to them instead of a number,
primarily because it's easier to implement. No objections going back to
the old behaviour if someone wants to contribute a clean patch.
The standard de-duplication will happen if two decks are created in the
same minute with the same name.
- Tweak the default sort order, and start with two searches. The UI
will still hide the second search by default, but by starting with two,
the frontend doesn't need logic for creating the starting text.
- Search errors now have their own error type, instead of using
InvalidInput, as that was intended mainly for bad API calls. The markdown
conversion is done when the error is converted from the backend, allowing
errors to printed as a string without any special handling by the calling
code.
TODO: when building a new filtered deck, update_active() is clobbering
the undo log when the overview is refreshed
Now behaves the same way as standard find&replace:
- Will match substrings
- Regexs can be used to match multiple items; we no longer split
input on spaces.
- The find&replace dialog has been updated to add tags to the field
list.
We were (ab)using the bulk update routine to do deletions, but that
code was really intended to be used for finding&replacing, where an
exact match is not a requirement.
- clear_unused_tags() is now undoable, and returns the number of removed
notes
- add a new mw.query_op() helper for immutable queries
- decouple "freeze/unfreeze ui state" hooks from the "interface update
required" hook, so that the former is fired even on error, and can be
made re-entrant
- use a 'block_updates' flag in Python, instead of setUpdatesEnabled(),
as the latter has the side-effect of preventing child windows like
tooltips from appearing, and forces a full redrawn when updates are
enabled again. The new behaviour leads to the card list blanking out
when a long-running op is running, but in the future if we cache the
cell values we can just display them from the cache instead.
- we were indiscriminately saving the note with saveNow(), due to the
call to saveTags(). Changed so that it only saves when the tags field
is focused.
- drain the "on_done" queue on main before launching a new background
task, to lower the chances of something in on_done making a small query
to the DB and hanging until a long op finishes
- the duplicate check in the editor was executed after the webview loads,
leading to it hanging until the sidebar finishes loading. Run it at
set_note() time instead, so that the editor loads first.
- don't throw an error when a long-running op started with with_progress()
finishes after the window it was launched from has closed
- don't throw an error when the browser is closed before the sidebar
has finished loading
- Introduced a new transact() method that wraps the return value
in a separate struct that describes the changes that were made.
- Changes are now gathered from the undo log, so we don't need to
guess at what was changed - eg if update_note() is called with identical
note contents, no changes are returned. Card changes will only be set
if cards were actually generated by the update_note() call, and tag
will only be set if a new tag was added.
- mw.perform_op() has been updated to expect the op to return the changes,
or a structure with the changes in it, and it will use them to fire the
change hook, instead of fetching the changes from undo_status(), so there
is no risk of race conditions.
- the various calls to mw.perform_op() have been split into separate
files like card_ops.py. Aside from making the code cleaner, this works
around a rather annoying issue with mypy. Because we run it with
no_strict_optional, mypy is happy to accept an operation that returns None,
despite the type signature saying it requires changes to be returned.
Turning no_strict_optional on for the whole codebase is not practical
at the moment, but we can enable it for individual files.
Still todo:
- The cursor keeps moving back to the start of a field when typing -
we need to ignore the refresh hook when we are the initiator.
- The busy cursor icon should probably be delayed a few hundreds ms.
- Still need to think about a nicer way of handling saveNow()
- op_made_changes(), op_affects_study_queue() might be better embedded
as properties in the object instead
Issues that need fixing:
- when the editor saves the note with perform_op(), if it isn't modified,
no new undo entry is created, and perform_op then returns the changes
made by the previous operation instead
- the approach of fetching the last action in a subsequent backend
method is unsound, as another queued operation may sneak in first before
we have a chance to query the result - it would be better if it were
returned in a single atomic action
- redrawing the current card while editing is likely to make sound
autoplay annoyingly, and it has an unpleasant redraw. We may be better off
fading it out instead
Side note: the editor cursor moves to the start of the field when the
note is updated in another window - it might be nicer to have it move
the cursor to the end instead.
'card modified' covers the common case where we need to rebuild the
study queue, but is also set when changing the card flags. We want to
avoid a queue rebuild in that case, as it causes UI flicker, and may
result in a different card being shown. Note marking doesn't trigger
a queue build, but still causes flicker, and may return the user back
to the front side when they were looking at the answer.
I still think entity-based change tracking is the simplest in the
common case, but to solve the above, I've introduced an enum describing
the last operation that was taken. This currently is not trying to list
out all possible operations, and just describes the ones we want to
special-case.
Other changes:
- Fire the old 'state_did_reset' hook after an operation is performed,
so legacy code can refresh itself after an operation is performed.
- Fire the new `operation_did_execute` hook when mw.reset() is called,
so that as the UI is updated to the use the new hook, it will still
be able to refresh after legacy code calls mw.reset()
- Update the deck browser, overview and review screens to listen to
the new hook, instead of relying on the main window to call moveToState()
- Add a 'set flag' backend action, so we can distinguish it from a
normal card update.
- Drop the separate added/modified entries in the change list in
favour of a single entry per entity.
- Add typing to mw.state
- Tweak perform_op()
- Convert a few more actions to use perform_op()
Basic proof of concept, where the 'delete note' operation in the
reviewer has been updated to use mw.perform_op(). Instead of manually
calling .reset() afterwards, a summary of the changes is returned as
part of the undo status query, and various parts of the GUI can listen
to gui_hooks.operation_did_execute and decide whether they want to
redraw based on the scope of the changes. This should allow the sidebar
to selectively redraw just the tags area in the future for example.
Currently we're just listing out all possible areas that might be changed;
in the future we could theoretically inspect the specific changes in the
undo log to provide a more accurate report (avoiding refreshing the tags
list when no tags were added for example).
You can test it out by opening the browse screen while studying, and
then deleting the current card - the browser should update to show (deleted)
on the cards due the earlier change.
If going ahead with this, aside from updating all the screens that currently
listen for resets, some thought will be required on how we can integrate
it with legacy code that expects to called when resets are made, and expects
to call .reset() when it makes changes.
Thoughts?
Fixes the following issue:
- some code directly modifies the database, causing modified_in_python
to be set to true
- an undoable operation is run, which calls autosave() at the end
- autosave() notices there's an undoable operation, and commits immediately
- because modified_in_python was true, col.mtime was bumped in Python
- that invalidated the undo queue, preventing the operation from being
undone
This correct the probably rare bug as follow:
I got a note type with a field whose name is "\".
When I made any change to this note type, even unrelated, I get a message stating that there is an empty field. This is
strange because I can see it to be false. Investigating show that "\" is normalized to empty field. This ensure that
it's shown
Rust requires all methods of impl Trait to be in a single file, which
means we had a giant backend/mod.rs covering all exposed methods. By
using separate service definitions for the separate areas, and updating
the code generation, we can split it into more manageable chunks -
this commit starts with the scheduling code.
In the long run, we'll probably want to split up the protobuf file into
multiple files as well.
Also dropped want_release_gil() from rsbridge, and the associated method
enum. While it allows us to skip the thread save/restore and mutex unlock/
lock, it looks to only be buying about 2.5% extra performance in the
best case (tested with timeit+format_timespan), and the majority of
the backend methods deal with I/O, and thus were already releasing the
GIL.
Work in progress - still to do:
- renames appear as 'Update Deck' - easiest way to solve it would
be to have a separate backend method for renames
- drag&drop of decks not yet undoable
- since the undo status is updated after the backend method ends,
the older checkpoint() calls need to be replaced with an
update_undo_status() at the end of the call - if we just remove the
checkpoint, then the menu doesn't get updated
- moved 'default to current deck when adding' into prefs
- move some profile options into the collection config, so they're
undoable and will sync. There is (currently) no automatic migration
from the old profile settings, meaning users will need to set the
options again if they've customized them.
- tidy up preferences.py
- drop the deleteMedia option that was not exposed in the UI
The existing code was really difficult to reason about:
- The default notetype depended on the selected deck, and vice versa,
and this logic was buried in the deck and notetype choosing screens,
and models.py.
- Changes to the notetype were not passed back directly, but were fired
via a hook, which changed any screen in the app that had a notetype
selector.
It also wasn't great for performance, as the most recent deck and tags
were embedded in the notetype, which can be expensive to save and sync
for large notetypes.
To address these points:
- The current deck for a notetype, and notetype for a deck, are now
stored in separate config variables, instead of directly in the deck
or notetype. These are cheap to read and write, and we'll be able to
sync them individually in the future once config syncing is updated in
the future. I seem to recall some users not wanting the tag saving
behaviour, so I've dropped that for now, but if people end up missing
it, it would be simple to add as an extra auxiliary config variable.
- The logic for getting the starting deck and notetype has been moved
into the backend. It should be the same as the older Python code, with
one exception: when "change deck depending on notetype" is enabled in
the preferences, it will start with the current notetype ("curModel"),
instead of first trying to get a deck-specific notetype.
- ModelChooser has been duplicated into notetypechooser.py, and it
has been updated to solely be concerned with keeping track of a selected
notetype - it no longer alters global state.
This splits update_card() into separate undoable/non-undoable ops
like the change to notes in b4396b94abdeba3347d30025c5c0240d991006c9
It means that actions get a blanket 'Update Card' description - in the
future we'll probably want to either add specific actions to the backend,
or allow an enum or string to be passed in to describe the op.
Other changes:
- card.flush() can no longer be used to add new cards. Card creation
is only supposed to be done in response to changes in a note's fields,
and this functionality was only exposed because the card generation
hadn't been migrated to the backend at that point. As far as I'm aware,
only Arthur's "copy notes" add-on used this functionality, and that should
be an easy fix - when the new note is added, the associated cards will
be generated, and they can then be retrieved with note.cards()
- tidy ups/PEP8
To coalesce successive note edits into a single undo op we'll need to
be able to get the original Undoable type, which is awkward to do with
a trait object.
- note.flush() behaves like before, as otherwise actions or add-ons
that perform bulk flushing would end up creating an undo entry for
each note
- added col.update_note() to opt in to the new behaviour
- tidy up the names of some related routines
- transact() now automatically clears card queues unless an op
opts-out (and currently only AnswerCard does). This means there's no
risk of forgetting to clear the queues in an operation, or when undoing/
redoing
- CollectionOp->UndoableOp
- clear queues when redoing "answer card", instead of clearing redo
when clearing queues
- use dataclasses for the review/checkpoint undo cases, instead of the
nasty ad-hoc list structure
- expose backend review undo to Python, and hook it into GUI
- redo is not currently exposed on the GUI, and the backend can only
cope with reviews done by the new scheduler at the moment
- the initial undo prototype code was bumping mtime/usn on undo, but
that was not ideal, as it was breaking the queue handling which expected
the mtime to match. The original rationale for bumping mtime/usn was
to avoid problems with syncing, but various operations like removing
a revlog can't be synced anyway - so we just need to ensure we clear the
undo queue prior to syncing
- fetch sfld and csum when fetching notes, to make it cheaper
to write them back out unmodified
- make `fields` private, and access it via accessors, so we can
still catch when fields have been mutated without calling
prepare_for_update()
- fix python importing code passing a string in as the checksum
- Currently we just use 1.5x and 2x the normal preview delay; we could
change this in the future.
- Don't try to capture the current state; just use a flag to denote
exit status.
- Show (end) when exiting
Bazel sets TZ to UTC when running tests, so the tests are reproducible.
But it seems like the Rust time crate was not honoring it, and using
the configured timezone instead. "Fix" by forcing UTC when testing, as
we already special-case a test run.
Notes:
- The fuzz seed is now derived from the card id and # of reps, so
if a card is undone and done again, the same fuzz will be used.
- The intervals shown on the answer buttons now include the fuzz, instead
of hiding it from the user. This will prevent questions about due dates
being different to what was shown on the buttons, but will create
questions about due dates being different for cards with the same
interval, and some people may find it distracting for learning cards.
The new approach is easier to reason about, but time will tell
whether it's a net gain or not.
- The env var we were using to shift the clock away from rollover for
unit tests has been repurposed to also disable fuzzing, which simplifies
the tests.
- Cards in filtered decks without scheduling now have the preview delay
fuzzed.
- Sub-day learning cards are mostly fuzzed like before, but will apply
the up-to-5-minutes of fuzz regardless of the time of day.
- The answer buttons now round minute values, as the fuzz on short
intervals is distracting.
This is not the way the code is intended to be used, but making it
conform to the existing API allows us to exercise the existing unit
tests and provides partial backwards compatibility.
- Leech handling is currently broken
- Fix answered_at in wrong units, and not being used
This reverts commit 8372931b9b.
I fear this will be too disruptive - let's give AnkiDroid a bit more
time to catch up. Reverting this will mean new users are presented with
an upgrade notice on first startup, which looks a bit silly, but it's
probably the lesser of two evils.
