Minor changes #5
Conversation
There was a problem hiding this comment.
Thanks for the PR. This looks promising. I guess the goal is to read Anki collections straight from disk, without the intermediate Export step?
Do you have plans to read files (sounds, images, etc) from disk as well?
Let me know your plans, and if I should consider merging this soon, or after some additional changes.
Aside from that, I've left a couple of inline comments.
Thanks for contributing.
sqlite-std.go
Outdated
| } | ||
| conf, ok := collection.DeckConfigs[deck.ConfigID] | ||
| if !ok { | ||
| //return nil, fmt.Errorf("Deck %d references non-existent config %d", deck.ID, deck.ConfigID) |
There was a problem hiding this comment.
This line got commented out during the code move. Was this intentional?
There was a problem hiding this comment.
Uops. Yes, you're right. The problem is that in one of my local Anki profiles I had a situation where there was no dek for that deck.ConfigID. I don't know how big a problem that incosistency is, but both the desktop Anki client and Ankidroid ignore this feature.
I tried now and it it's not the case any more. I suppose it's probably something left after I removed a deck and it was probably not cleaned immediately but later, after I run the "check database" desktop client.
I'm removing it for now, but if it happens again, I'll investigate more.
| @@ -20,6 +20,97 @@ type DB struct { | |||
| tmpFile string | |||
| } | |||
|
|
|||
| func (db *DB) Collection() (*Collection, error) { | |||
There was a problem hiding this comment.
I would suggest putting these new *DB methods in a new file, sqlite.go, with no build constraints. By putting them here, they are not available for GopherJS builds (thus the Travis-CI failure).
There was a problem hiding this comment.
Ah, ok, thanks. I'll update my pull request later.
There was a problem hiding this comment.
Update, I found the situation again.
Deck 1446489859874 references non-existent config 0
Both the AnkiDroid and desktop app work without problems with this collection, so this isn't an error. Maybe just write a warning in stderr instead of returning an error here?
PS. Commented on a wrong code line, this was for the place where I commented the error when a deck isn't found by id.
There was a problem hiding this comment.
I guess that means that a default configuration is used in such a case?
Warning to sterr seems reasonable to me.
|
Hey, thanks for your feedback. So, I have two small side project which both need to read one mf my anki decks. But don't have the Ideally, the library would work regardless if you open an For files. No, I don't have plans to read files, but I do add audio files to my decks. An API for doing that would be cool, but I got it to work even without that. BTW, You can see here how it works. Remember, this is just a quick weekend project and the code is not really high quality, but it works :) |
|
Ah, and one more thing. I have a plan to create another small open source tool which will allow me to:
So, I'll probably need a better way to edit notes. As you can see in the code I already succeeded in doing it (although the code is still very ugly), but a simpler API for doing it would be a nice addition. What do you think? PS. Thanks for your great work on the library! |
That sounds like a very interesting project... good luck! I'd love to see the finished product.
No problem; I'm glad someone else found it useful. |
This pull request contains 3 minor changes.
Collections(),Notes()andCards()are now originally on DB, and the original functions atApkgare just calling them. The desktop client keeps everything in the database, not in.apkgfiles and this allows for reading (and eventually manipulating) data directly."\x1f"delimiterOpenOriginalDB()).