Read typstfmt config from files, if possible

[KillTheMule]

If we have a typstfmt-config.toml in the working directory, use that. Otherwise, look for it in the project root. Otherwise, use the default config. This seemed the most sensible thing to me.

[astrale-sharp]

If there is a need for typstfmt to be different/more flexible in it's config handling, ask and you might receive, I just want it to stay straightforward

[KillTheMule]

Note that typst-lsp does not call the typstfmt binary, but rather uses the library directly. So this would only benefit from the changes if you were to pull the config handling into the lib somehow.

[astrale-sharp]

The only part of config handling in main.rs is this :

    let config = {
        if let Ok(mut f) = File::options().read(true).open(CONFIG_PATH) {
            let mut buf = String::default();
            f.read_to_string(&mut buf).unwrap();
            Config::from_toml(&buf).unwrap_or_else(|e| panic!("Config file invalid: {e}.\nYou'll maybe have to delete it and use -C to create a default config file."))
        } else {
            Config::default()
        }
    };

Config also impl Deserialize and Serialize, from typst-lsp it could be quite easy to:

• provide file content and then Config::from_toml(content) and possibly unwrap_or_default with warnings
• Use the deserialize to create an interface where you can set the config parameters (I think this option is better)

Please let me know if I'm being unclear!

[KillTheMule]

Please let me know if I'm being unclear!

Maybe I'm misunderstanding, but 1) is what I've implemented here (and we're talking about configurability of the file name/path), and 2) is basically what I asked about (modulo the deserialization, thanks for that hint). Right? I guess I only need nvarner to chime in what exactly he wants to see here :)

[astrale-sharp]

Been thinking about this, what would be ideal:
if there is a config file, get your config there
otherwise get your config from the extension settings
the settings would be the same than config

The reasons for this:

• You can keep your config everywhere and override it for specific projects

[nvarner]

@KillTheMule sorry for such delay, but I've implemented the cache-based system I was thinking about:
KillTheMule#1
This prevents the config file from being read directly from the filesystem every time.

[KillTheMule]

Don't worry about the delay :)

Maybe I'm missing something, but where is the part where the config file gets re-read if it changes on disk? As far as I can see, this doesn't happen, but I'd pretty much deem this unacceptable. If people change a config value in the file, this needs to be picked up on next format imho.

[nvarner]

(Reviewing the PR to find the relevant lines, I realized I rebased, which is causing a lot of noise; sorry about that as well)

The config file ultimately comes from read_bytes_by_id, which calls into FsManager::read_bytes.

That will check if the LSP provides the config file (it won't; only Typst files should be provided), then checks the cache. If the read from cache fails, it loads the config file from the filesystem, and otherwise uses the cached file.

When the file is changed on disk, it triggers an event, which is handled by invalidate_local, which itself will remove the cache entry so that the config file will be read from disk next time it is requested.

[KillTheMule]

Oh wow awesome, I did not realize that! I'll try this out tonight, and bring the PR in a mergeable state.

[KillTheMule]

Ok, this works nicely, great! I rebased properly, all should check out now. I'd have written a test, but there doesn't seem to be any infra for that.

Question: Do we want to support a config file in the project root? That would complicate the caching, since we could work off a project root file when suddenly a (higher priority) config file at the document location appears, but it feels like the right piece of functionality for a language server.

[nvarner]

Testing infrastructure is certainly lacking. (It should be possible to write something basic, but the test wouldn't get much deeper than testing the FS/cache and typstfmt's parsing, so probably isn't worth much, anyway.)

I'm going to merge this PR now for sake of getting the feature in, but I'm interested in your idea. I'm not sure the exact config file structure you're imagining; could you write up a short example directory tree?

The implementation won't actually need to worry about caching. Just by using project.read_bytes_by_id with the appropriate paths, reads will always come from the cache when possible. A bit of abstraction on top of that can make it easy for config files to "fall through" if not found. My only concern is that the logic might be better off in typstfmt eventually, especially if it gets complex, or if the LSP implementation becomes incompatible with the command line implementation.

[KillTheMule]

Well, typstfmt by itself does not have a notion of a project, nor do I think it needs to/should. But typst-lsp has, via the root_dir config option. So it does make sense for typst-lsp to look for a config file in root_dir/typstfmt-config.toml, which probably should have lower priority than current_document_dir/typstfmt-config.toml. That's all I was thinking :)

Personally, I won't use that, since I can't really imagine making a project of some sorts out of a document, but maybe others do have complex papers/documents which would benefit.

[astrale-sharp]

Thanks @nvarner @KillTheMule , really appreciate it ;)