Report error when setting a wrong guifont value

[Tmpod]

What kind of change does this PR introduce?

It addresses #2077 by fist implementing a new parallel UiCommand, ShowError { message: String } which gets translated to neovim's echomsg, and then using it in tandem with log::error! in CachingShaper::update_font (when self.font_loader.get_or_load fails).

Did this PR introduce a breaking change?

• Yes
  • Invalid guifont values will now error out instead of silently being accepted. To pick no hinting or aliased edging, you should now use #h-none and #e-alias, respectively.
    • Actually, just checked the docs, and they already specified those options, even though the code didn't check for them specifically, they were just being handled on the wildcard match. Still this is a breaking change because of the new errors.

Closes #2077

[Tmpod]

At the moment, this is not producing any message, though the log is going through just fine.

I'm not familiar with Neovim's API, so perhaps I'm not using echomsg right?

PS: I'm aware of the extra : there, my bad, will remove in later commit.

[Tmpod]

Alright, this is now working. I have two questions regarding the code though (sorry, first contribution!)

1. Since show_error_message is just a forward call to Neovim::err_writeln, is it really worth having? Perhaps it would just be best to use that neovim API call directly in the command match arm.
2. Should I write a test for this, and if so, where?

Thank you!

[fredizzimo]

@Tmpod.

1. We use the nvim api directly elsewhere in the file, so I think removing the function makes sense. It's clear what writeline does.
2. I think tests are quite hard for this, since they would rely on the fonts installed on the system or require some mocking. Additionally, the actual functionality is quite trivial, so I don't think we need any tests.

The only thing left to do would be to also verify the fallback fonts. But if you are unsure how to do that, we can leave it for later in another PR. Basically, you need to try loading all of the fonts in the guifont list.

BTW, I'm also on Discord so if you have further questions, you can ask there too.

[Tmpod]

1. We use the nvim api directly elsewhere in the file, so I think removing the function makes sense. It's clear what writeline does. 2. I think tests are quite hard for this, since they would rely on the fonts installed on the system or require some mocking. Additionally, the actual functionality is quite trivial, so I don't think we need any tests.

Makes sense!

The only thing left to do would be to also verify the fallback fonts. But if you are unsure how to do that, we can leave it for later in another PR. Basically, you need to try loading all of the fonts in the guifont list.

That doesn't sound too difficult, I can try do that as well. Essentially, walk the font list and try loading them until we get one that does work? Or try load them all and report which ones failed?

BTW, I'm also on Discord so if you have further questions, you can ask there too.

o7

[fredizzimo]

Thank you!

[Tmpod]

Glad to help! :3