Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Empty cards become undoable #3386

Merged
merged 2 commits into from
Aug 29, 2024
Merged

Empty cards become undoable #3386

merged 2 commits into from
Aug 29, 2024

Conversation

Arthur-Milchior
Copy link
Contributor

@Arthur-Milchior Arthur-Milchior commented Aug 26, 2024

If there was a reason for this operation not to be undoable, I can't easily guess it. My main hyposhesis was that the number of deleted card may be too big. But I realized that deleting a deck is undoable and may delete as many note.

As you may know, I realized that only the undoable operations triggered notification in AnkiDroid that we may have to update the UI. And while I just wanted to trigger more notifications, some reviewers thought it would be nicer if the operation were returning a OpChanges. So here it's done. If you would please consider merging it.

I decided to introduce a new string because the closest strings I could find currently are "Empty cards..." and the trailing commas don't seem nice in "undo". And the title, which we may not be able to reuse in all language

@Arthur-Milchior
Copy link
Contributor Author

I guess the failures are:

Arthur, at the domain Milchior.fr
....
Author arthur, at the domain milchior.fr NOT found in list

I don't understand this failure. I guess it's a question of comparing upper/lower case. Except that in the contribtuors file, I'm listed as Arthur Milchior <[email protected]>, and I have no idea where the upper case in Milchior.fr comes from. I'd appreciate help here to resolve the contributor failure please.

@david-allison
Copy link
Contributor

david-allison commented Aug 28, 2024

@Arthur-Milchior This should be resolved if you make a whitespace change to CONTRIBUTORS in this commit

https://github.com/ankitects/anki/blob/main/CONTRIBUTORS

To prevent the automatic check from failing, you can edit this file again using GitHub's online editor, making a trivial edit like adding a space after your name, and then pull requests will work regardless of whether you create them using your computer or GitHub's online interface.


If there was a reason for this operation not to be undoable, I can't easily guess it. My main hyposhesis was that the number of deleted card may be too big. But I realized that deleting a deck is undoable and may delete as many note.

As you may know, I realized that only the undoable operations triggered notification in AnkiDroid that we may have to update the UI. And while I just wanted to trigger more notifications, some reviewers thought it would be nicer if the operation were returning a OpChanges. So here it's done. If you would please consider merging it.

I decided to introduce a new string because the closest strings I could find currently are "Empty cards..." and the trailing commas don't seem nice in "undo". And the title, which we may not be able to reuse in all language
@Arthur-Milchior
Copy link
Contributor Author

I changed the configuartion in git locally. Hope it'll solve it.

@david-allison
Copy link
Contributor

It was working previously, the build failure was due to a rustfmt error

@dae
Copy link
Member

dae commented Aug 29, 2024

Thanks Arthur! I can't recall any reason why this was deliberately omitted - I think it was probably just not a high priority when undo support was originally added to the codebase.

@dae dae merged commit ce2f413 into ankitects:main Aug 29, 2024
1 check passed
@Arthur-Milchior Arthur-Milchior deleted the Empty2 branch August 29, 2024 22:41
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.

3 participants