Switching to notes mode was throwing:
File "/Users/dae/Work/code/dtop/bazel-copy/runanki.runfiles/net_ankiweb_anki/qt/aqt/browser.py", line 449, in on_table_state_changed
self.table.toggle_state(checked, self._lastSearchTxt)
File "/Users/dae/Work/code/dtop/bazel-copy/runanki.runfiles/net_ankiweb_anki/qt/aqt/table.py", line 206, in toggle_state
self._state = self._model.toggle_state(
File "/Users/dae/Work/code/dtop/bazel-copy/runanki.runfiles/net_ankiweb_anki/qt/aqt/table.py", line 996, in toggle_state
self.search(context)
File "/Users/dae/Work/code/dtop/bazel-copy/runanki.runfiles/net_ankiweb_anki/qt/aqt/table.py", line 1005, in search
context.order = self.columns[self._state.sort_column]
* 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.
By passing back the builder to the calling code to run, we don't need
to plumb extra arguments like success= and handler= through each
operation, and the ability to override the default tooltip behaviour
comes free on all operations
By calling refresh() manually after performing an op, we were refreshing
twice, and the selection was being lost when changes were made outside
of the sidebar.
Also drop the after_hooks arg to perform_op(), since nothing is using
it now.
- 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
I think this may have accidentally been changed in the refactoring.
If we discard self._rows, it will result in the entire table flashing
"..." until the new data is available. Instead, we leave the cached
rows alone, and just update the cutoff point, so we can serve stale
content (avoiding any visible redraw) until the new data is available.
I've updated search() to reset the rows there, so we free up memory
on a new search.
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.
Instances can pass handled_by=self to more easily ignore events they
initiate.
Fixes ugly refresh when expanding/collapsing decks, but we're still
refreshing the card/notes area unnecessarily in that case.
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?
If the switch triggers heavy GUI action, like a lot of paint()s, the
concurrently running switch animation may look choppy.
A small timer makes these events execute first and the animation will
run smoothly afterwards when the event queue has been drained.
1) Check whether full row height is in viewport instead of just the
top left corner.
2) Add timer before scrolling to current row so editor will already
be set up.
subprocess.Popen emits ResourceWarning in the destructor if the status of the process was not read.
Fix by using subprocess.run() instead, which takes care of that.
Using run() is also recommended for simple cases like this in the docs.
I18n is not set up at init time, so the strings can't be generated
at import.
@kelciour you have a few importing add-ons, so wanted to give you a
heads-up. The importing code is likely to change more in
future months, but for now this should be the only change
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
- QueueConfig is only used by the scheduler
- DeckConfig was being used in places that Config should have been used
- Add "Dict" to the name so that the bare name is free for use with a
stronger type.
If it's some other error like the DB suddenly becoming accessible,
we don't want to scare the user into thinking their data was deleted,
and we want to know what the error was without popping up tens of
message boxes for each row.
- The previous commits moved the majority of the remaining global css
into components; move the remaining @emotion/css references into
ticks.scss and the styling of the Graph.svelte. This is not as elegant
as the emotion solution, but builds a whole lot faster, and most of
our styling can be scoped to a component anyway.
- Leave the .html files in ts/ for now. AnkiMobile uses them, and
AnkiDroid likely will in the future too. In the long run we'll likely
move to loading the JS into an existing page instead of loading a
separate page, but at that point we can just exclude the .html file from
copy_files_into_group() without affecting other clients.
Closes#1074
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.
Qt is pretty enthusiastic about redrawing the card list when any sort
of activity occurs, and by serving blank cells while the DB was busy,
we were getting ugly flashes, and cells getting stuck blank.
Resolve the issue by calculating a row up front and caching it, then
serving stale content when updates are blocked.
- 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
Setting it straight away causes the cursor to flash on quick operations,
like saving the current note. Delay it for 300ms, which should hopefully
be long enough to not get in the way, but short enough to give indication
that long-running requests are being processed.
- need to drop cardObjs cache when updating cells
- stop listening on editor_did_* hooks. unfocus_field and typing_timer
are covered by operation_did_execute on note save already, and the
user potentially has editors open in other windows as well
- distinguish between card queue refresh and note text redraw in review
screen again
- update preview window when note updated
- defer setUpdatesEnabled(True) until we receive focus again, as it
causes cells to redraw. We might want to use our own flag to prevent
updating in the model instead of using Qt for this
- 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.
- This avoids the need for a separate screen, though we may want to
slightly fade out the display when information is stale.
- Means the browser can delay updates just like the main window does.
'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?
Up until now, we've been forcing a new search whenever reset is called.
The primary reason was that the card list display routines did not expect
a card or note to have been removed. By updating the model to show
"(deleted)" when a card or note is missing, we no longer have to repeat
the search.
This has a few advantages:
- Searches, especially complex ones, can be slow to execute. When we
perform them after every operation like a delete, it can make Anki feel
sluggish.
- The fact that notes have been deleted becomes more obvious - some users
found it easy to miss the "deleted" pop-up in the past.
This change does not just affect deletions, as many other operations
trigger a reset as well. In the past, when using 'set due date' in the
review screen for example, it caused an ugly flicker in the browser screen,
and could be slow when the current search couldn't be quickly redone.
The disadvantage of this approach is that the displayed content may
not reflect the specified search, which has the potential to be confusing.
But if that turns out to be a problem, it could be (partly) alleviated by
displaying a refresh button next to the search bar when the search may
need to be refreshed.
Feedback welcome!
This reverts commit a0c47243b6, reversing
changes made to 0ab87b7339.
@RumoVZ this broke a bunch of operations like 'select notes' and
'set due date'. When the triggered signal is connected to a function,
PyQt looks at the function signature to decide what arguments to pass
it. The wrapper was using *args, so PyQt passes in an extra argument,
which the underlying function didn't expect.
I tried settting __signature__ on the wrapper, but PyQT seems to
ignore it, so we may either need to check all of the existing calls
and add the ignored extra arguments, or create a separate wrapper for
such cases.
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
- 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
Reviews and operations on the backend that support undoing can now be
committed immediately, so they will not be lost in the event of a crash.
This required tweaks to a few places:
- don't set collection mtime on save() unless changes were made in
Python, as otherwise we end up accidentally clearing the backend undo
queue
- autosave() is now run on every reset()
- garbage collection now runs in a timer, instead of relying on
autosave() to be run periodically
- 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
