Reload language config with :config-reload

[willful759]

Just had a go at implementing this

Not quite sure where to make test or what tests to make, but I tried it and it seems to be able to reload auto formatting, at least

quick_edit.mp4

[dead10ck]

If the language config changes the LSP, will this handle sending the new config? Or if the command changes, will it shut down the current instance and start up the new one?

[the-mikedavis]

It looks like Document::detect_language calls Document::set_language under the hood which only changes syntax parts of the doc. :set-language/:lang has the same problem (#3359 (comment))

[dead10ck]

Oh interesting. Should this be a separate issue/PR?

[sudormrfbin]

If the language config changes the LSP, will this handle sending the new config?

We can send the didChangeConfiguration request to the LSP to signal a change, but I'm not sure how many server's support dynamic config registration. I suppose the most reliable way is to restart the server if the config or the lsp command changes.

[the-mikedavis]

Ah I didn't even think about changing config. For a first iteration, stopping the old server and starting a new one would be a good improvement. That can be done in a separate PR though - I'll make an issue 👍

[the-mikedavis]

I think the refresh_config, refresh_language_config and refresh_theme commands could use some refactoring to improve the error handling. With this change, if you have an error in your language config and a theme error, you only end up seeing the theme error since it overwrites the editor.set_error from refresh_language_config

refresh_language_config and refresh_theme can return Result<(), Error>s instead and then use the anyhow helpers like anyhow::anyhow! or anyhow::bail! to format the error messages and return early in cases of errors. Then refresh_config can set the editor error if there are any failures:

fn refresh_config(&mut self) {
    let mut refresh_config = || -> Result<(), Error> {
        let config = Config::load_default()
            .map_err(|err| anyhow::anyhow!("failed to load config: {}", err))?;
        self.refresh_language_config()?;
        self.refresh_theme(&config)?;

        // Store new config
        self.config.store(Arc::new(config));

        Ok(())
    };
    if let Err(err) = refresh_config() {
        self.editor.set_error(err.to_string());
    }
}

And the other functions can use ? to return errors early and remove the duplicated error handling code:

fn refresh_language_config(&mut self) -> Result<(), Error> {
    let syntax_config = helix_core::config::user_syntax_loader()?;
    self.syn_loader = // ...
    // ...
    Ok(())
}

[willful759]

Liked the suggestion, code looks a lot cleaner now.

I'm new to rust so recommendations like this are always welcomed

[dead10ck]

Haven't tested locally, but the code looks good to me 👍

[the-mikedavis]

Very minor thing but this can be written with anyhow::bail!:

anyhow::bail!("theme requires truecolor support, which is not available")