Skip to content

Conversation

@MartianGreed
Copy link

No description provided.

@3rd
Copy link
Owner

3rd commented Aug 16, 2024

The issue is that the JSON storage doesn't scale, save_data won't work if the JSON gets too large (>500kb).
Didn't the sql script work for you? #1 (comment)

@MartianGreed
Copy link
Author

It worked ! I liked the idea to provide user the possibility to between both !
And also to give user the possibility to choose to implement their own storage

@3rd
Copy link
Owner

3rd commented Aug 16, 2024

I like the idea and I appreciate the code and tests, but I think the underlying primitives (session) shouldn't depend on the storage. Things like start_session being duplicated and storage-dependent would be weird and hard to maintain, a storage-only interface would be better.

The issue I have with JSON built-in is that vim.fn.json_encode() fails when the payload gets >~500KB, which is only a few days of activity for some (me), and even if it didn't have a limit the load/save steps will take longer and longer to run.

Although the current SQLite usage mimics the previous implementation (load all entries to compute the stats, etc) it will be changed soon such that we'll let SQLite compute everything instead of doing it in Neovim.

I think we should definitely create the storage abstraction and let users plug their own storage layer code, but we shouldn't have a built-in JSON mode if it starts failing to save data after a few days. Maybe using JSONL would be a decent tradeoff?

@MartianGreed
Copy link
Author

Ok I see ! Let me some time to make this abstraction.
I'll for sure remove the json :)

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.

2 participants