Conversation
DHowett
left a comment
DHowett
left a comment
There was a problem hiding this comment.
How much does simple JSON settings grow our binary?
crates/edit/src/bin/edit/settings.rs
Outdated
|
|
||
| #[cfg(target_os = "windows")] | ||
| { | ||
| var_path("APPDATA").map(|p| push(p, "Microsoft/Edit")) |
There was a problem hiding this comment.
Technically we should use one of the Known Folder APIs. Should this type of thing live in the edit sys crate?
There was a problem hiding this comment.
I feel like it would be neat if the edit code (the code outside the bin directory) had as little to do with edit as possible. I've already extracted substantial parts into other crates out of necessity, and I expect this to continue. Additionally, it would indeed make a neat TUI library if it was properly cleaned up. And a TUI library probably wouldn't need any known-dir APIs.
Re: SHGetKnownFolderPath. I avoided it because of the [internal implementation details].
There was a problem hiding this comment.
BTW I did some measurements. I knew what the result would be, reading the implementation, but hard numbers are technically the right thing to do, eh?
GetEnvironmentVariableW = 187ns/op
SHGetKnownFolderPath = 569ns/op (3x)
This may lead you to believe that we did solid™ engineering™, but this was measured when running those APIs in a loop. If you measure the latency-to-first-result it changes drastically:
GetEnvironmentVariableW = ~6us/op
SHGetKnownFolderPath = ~1700us/op (283x)
You see, I'm not irrationally snarky, but fully-justified-snarky! The windows.storage.dll APIs are en...hanced and I'd prefer avoiding it.
|
272384 -> 297984, so 25.6kB. Roughly half of that (11kB) are Rust's f64 parsing tables, aka Switching to f32 doesn't change this, since edit didn't do any float parsing previously and f32 parsing is still complex. If anyone would be up for some fun time, we could implement our own float parsing. It is technically not that difficult if we don't care at all about theoretical last digit precision (we don't need that) or performance (definitely don't need that for settings parsing). |
| pub fn bootstrap(tb: &mut TextBuffer) { | ||
| tb.set_crlf(false); | ||
| tb.write_raw(b"{\n}\n"); | ||
| tb.cursor_move_to_logical(Default::default()); |
There was a problem hiding this comment.
hmm some way for the cursor to go inside the {} ? is that silly?
There was a problem hiding this comment.
I also considered that, but VS Code puts it at 0,0 and I'll keep that for consistency.
|
|
||
| #[cfg(target_os = "windows")] | ||
| { | ||
| var_path("APPDATA").map(|p| push(p, "Microsoft\\Edit")) |
There was a problem hiding this comment.
fwiw when i said "edit sys crate" i actually thought edit itself had its own system specific crate, different from the generic sys one in this repo. I ask mostly because if I were to add UEFI, I would want this to return None without having to put uefi-specific things all over the code
|
Also link to the issue number where people are asking for settings! |
2 participants
Adds settings.json loading and parsing.
For now, that's just for a
files.associationskey.Part of #22
Part of #624