jasonwilliams / anki

Anki VSCode Plugin
MIT License
278 stars 32 forks source link

A doozie #38

Closed fletchermoore closed 3 years ago

fletchermoore commented 3 years ago

So there is a lot in here, some of which doesn't fit what we discussed, for instance option names.

I don't think I changed anything fundamentally from what you originally had unless options are enabled.

Hope its helpful.

jasonwilliams commented 3 years ago

Yeah its helpful thanks, looks good overall

So what's outstanding here? Are you planning to implement everything in todo before merging?

Also I've left some comments

jasonwilliams commented 3 years ago
fletchermoore commented 3 years ago
* I think allowUpdates should be called "allowOverwrite" or something as its not actually updating the existing card. It also separates it from the "keepSync" feature which is very different.

* "keepSync" doesn't make it clear that it updates on save. I think "updateAnkiOnSave" would make more sense here

Sounds much better.

* Setting keepSync to true, sending, updating the front then sending again causes Anki to crash. Im not sure if you know about this.

Sadly I didn't. I probably didn't test your original card building strategy against my changes well enough since I was using nested headers. So going back and making sure that all works would be my priority after work.

* If I have multiple cards in a file it doesn't seem to pick up the changes I make to one of them.

Probably related to the above.

* Editing the front of a card deletes the card and creates a new one right? (when keepSync is true)

Yes. But it does this somewhat indirectly. It compares the IDs of the send that just completed to the IDs written in the meta from the previous send.

The idea being you might want to delete cards from the markdown or change the fronts (which would make new cards).

Changing card fronts essentially will always create a new card because the comparison check is against the front, the same way Anki checks for duplicates.

fletchermoore commented 3 years ago

Yeah its helpful thanks, looks good overall

So what's outstanding here? Are you planning to implement everything in todo before merging?

Also I've left some comments

I wasn't. The todo was mostly a wish list for my own purposes. I thought it was in a bug free state since I was using it for a few days now. That's why I sent the pull.

fletchermoore commented 3 years ago

So I am figured out how to do more complicated things in git right now than I've done before.

Making a new branch from your current live state, will then add probably:

1) Note Id fix 2) MarkdownFile object 3) Transformer changes to allow media to come from a path relative to source 4) the handful of ankiservice additions

No new options/commands

Then I'll send that as a pull first. Hopefully today.

Then I'll try to break the other additions into separate pulls, going forward to keep things organized.

jasonwilliams commented 3 years ago

That sounds good to me @fletchermoore, thanks