RMichelsen / Nvy

Nvy - A Neovim client in C++
MIT License
332 stars 28 forks source link

`--geometry` is based on default font ? #120

Closed bprb closed 1 month ago

bprb commented 4 months ago

Hi, nice client! Thanks for this project!

If I use --geometry=80x25 then the window's &lines and &columns will only match 80 and 25 when using the default font. It looks like start_rows is converted with RendererGridToPixelSize before set guifont takes effect?

I tried a few hacks, but couldn't find a nice solution. For example, move start_rows/cols into the renderer and resize the window from WM_RENDERER_FONT_UPDATE. But, you only want to do that at start up, not resize the window every time the user changes fonts. Sadly has_drawn is already set here because redraw/"flush" comes before the font update (using nvim 0.10.0). So you need an extra flag. And suppress the bogus zero length font string. And that means the window visibly flashes a resize... aaand I still seemed to be getting wrong [rows, cols] anyway.. So, a hack alright 🤔

Do you have some ideas for an elegant way to solve this? Can we force nvim to process gui options between NvimInitialize and NvimSendUIAttach or something? I know nothing of the nvim protocol, sorry.

Another option is to add a command line parameter for the default font, or at least the size. Also a bit hacky though since it duplicates what's in the init file 🤔

You can work-around by scaling the geometry params by "actual font size I'll use" divided by "hardcoded default size (14)".

Thanks! bert

bprb commented 4 months ago

Just adding to clarify, this "fixes" the problem but the challenge is how to make sure it only fires once, and ideally before ShowWindow:

case WM_RENDERER_FONT_UPDATE: {
    if (true) // "fix" the problem
    {
        int start_rows = 25; // pretend --geometry=80x25
        int start_cols = 80;
        UINT window_flags = SWP_NOSIZE | SWP_NOMOVE | SWP_NOZORDER | SWP_FRAMECHANGED;
        int window_w = 0, window_h = 0;

        if (start_rows != 0 && start_cols != 0) {
            PixelSize start_size = RendererGridToPixelSize(context->renderer,
                start_rows, start_cols);
            window_w = start_size.width;
            window_h = start_size.height;
            window_flags = window_flags & ~SWP_NOSIZE;
        }
        SetWindowPos(hwnd, HWND_TOP, 0, 0, window_w, window_h, window_flags);
    }
    auto [rows, cols] = RendererPixelsToGridSize(context->renderer,
        context->renderer->pixel_size.width, context->renderer->pixel_size.height);
    SendResizeIfNecessary(context, rows, cols);
} return 0;
bprb commented 4 months ago

This seems to work:

Before, we resize the window using default font metrics.
Instead, use VimEnter -> nvim_get_option_value &guifont -> response
as the trigger, so this runs after user config is parsed.

It also means we no longer have to find and parse the config file
manually.

The downside is that --geometry takes precedence over any `set lines`
or `set columns` in the user config file.  But if that's what the user
has set up then perhaps they don't need/use --geometry...

What do you think? Acceptable for a PR? :) Thanks.

RMichelsen commented 2 months ago

I would say if that works I would be more than happy, especially if it means we can get rid of the ugly config parsing code. If you make a PR I will review and merge it

RMichelsen commented 1 month ago

Fixed in #128