Add Edit dialog #306
Add Edit dialog #306
Conversation
|
I looked into testing a bit. It seems that the best choice would be Not sure how to run existing tests, they seem to be failing due to various errors, such as
or
|
I think that's fine. I actually prefer snake case myself (as that follows the python coding standard), but the code all used to be camel case, which I have been matching. Agreed that it just makes sense to use snake case going forward.
Sounds good to me.
I think it's great that there is any sort of identifier at all :) That should work just fine.
Fine with either approach.
Yeah I wouldn't worry about it. It may be fixed in future releases of AnkiConnect for all we know. |
|
Aha, great! As for testing; I've been experimenting with testing a bit and if that's fine with you I can try to fix current tests. Most now fail due to changes in Anki. I want to:
|
|
Anyway, here's my attempt, with tests running on GitHub Actions passing. I hope the commit history is ok. This PR is mostly ready, but I'll keep it as a draft for a few days in case I remember anything. I removed one API method and fixed a few and converted all tests to pytest and made them run with tox against Anki versions 2.1.45 to 2.1.49. The tests might be slightly flaky. I fixed all of the flakiness I could find but I'm not sure what can be done about segfaults. If the tests prove too flaky, we can try run them forked with As for the Note that I added GitHub actions for PRs as well. This should be safe as due to the dangers inherent to automatic processing of PRs, GitHub’s standard |
|
This is all looking good so far 👍 |
|
Thanks! Of course right after pushing I discovered a major problem, that should be fixed now. A few issues should be adressed before this is ready to merge though.
Oh and I guess the documentation needs to get updated. P.S. I set the tests to run with |
|
I wouldn't worry too much about older versions; they can always use older releases if necessary (or just upgrade to a newer version of AnkiConnect). |
Like Edit Current, but: * has a Preview button to preview all cards for this note * has a Browse button to open the browser with these * has Previous/Back buttons to navigate the history of the dialog * has no Close button bar
The method was failing due to Anki API changes. Also, make it mandatory to call the method with `cardsToo=True`, since deleting decks on Anki >= 2.1.28 without cards is no longer supported, and the deprecated `decks.rem()` method on Anki >= 2.1.45 ignores keyword arguments.
I think this was a typo?
This method was broken and there are no issues regarding it on GitHub.
Don't start the web server if imported not from inside Anki Make sure Anki Connect instance is not garbage collected. This kills the timer that's responsible for the web server.
* make previewer use a more generic Adapter to flip through cards; * return Previewer from `show_preview()` for testing, * as well as Edit dialog from `open_dialog_and_show_note_with_id`; * disable/enable `Edit` editor buttons more reliably; * a few minor changes
So that it is importable by tests
Makes Pycharm happy
Previously, tests were run against Anki launched by user.
Now,
* most tests run against isolated Anki in current process;
* tests in `test_server.py` launch another Anki in a separate process
and run a few commands to test the server;
* nearly all tests were preserved in the sense that
what was being tested is tested still.
A few tests in `test_graphical.py` are skipped due to
a problem with the method tests, see the comments;
* tests can be run:
* In a single profile, using --no-tear-down-profile-after-each-test;
* In a single app instance, but with the profile being torn down
after each test--default;
* In separate processes, using --forked.
The tests can be run now using `tox` against multiple Anki versions; see instructions in `tox.ini`. The tests depend on `pytest-anki` that had to be slightly modified to remove the upper constraint on Anki version, as well as to remove a few dependencies that are not essential to using it.
The functionality was broken, creating a dialog that was not, in fact, closing after adding a card. See the deleted comment in `test_graphical.py`
It turns out that `pytest-anki` does what we are trying to do already. Note that `empty_anki_session_started` creates a temporary user too. We are “overwriting“ it in `profile_created_and_loaded` by calling `temporary_user`. It seems that doing this is safe.
Before, pressing the Browse button would only show browser with the cards or notes corresponding to the currently edited note. Now, it shows all cards or notes from the dialog history, in reverse order (last seen on top), with the currently edited note or its cards selected.
Waitress is a WSGI server that Anki starts to serve css etc to its web views. It seems to have a race condition issue; the main loop thread is trying to `select.select` the sockets which a worker thread is closing because of a dead connection. This makes waitress skip actually closing the sockets.
|
Okay I think this is nearly there (again). I also:
I'll give it a couple of days and then mark as ready. A few days ago Anki 2.1.50 was released and I want to fix the test to also pass for Anki 2.1.50. Shouldn't be too hard, although since they changed all things JavaScript, adding buttons will require some work. |
|
Awesome work! I'll put this on AnkiWeb this week. |
|
Thanks! Although it might have been better not to squash the commits. I specifically tried making the commits in such a way that they are useful for anyone who later looks at git history. Anyway, I think I will have a few more fixes for Anki 2.1.50 shortly 😛 |
As discussed in FooSoft/yomichan#2070, this dialog can be an alternative to opening Browser after adding a card in Yomichan. Some parts of the browser, such as the sidebar and the note list, take a lot of space and cannot be collapsed fully. This dialog looks like Edit Current dialog, except it:
This is a work in progress. At the time being the code works fine on Anki 2.1.49 (dc80804a) but has not been tested on other Anki versions. Remains to be done:
[ ] Make code compatible with earlier Anki versionsI made some decisions that I'm not completely confident in.
Ctrl-Ffor the Browser, andAlt-LeftandAlt-Rightfor the Previous/Next buttons.Ctrl-Fis easy to press and that's one of the hotkeys that focus the input field in the Browser.Alt-LeftandAlt-Rightare what browsers normally use so I hope these are a good fit."foosoft.ankiconnect."as a prefix for various identifiers, something else might be more appropriate?Also, what's the minimal version of Anki that the code is supposed to support? This should work on Anki 2.1.45+ without changes. Making it work with earlier versions would lead to significant ugliness.