Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: take into account prompt size in nui and builtin selects #135

Merged
merged 1 commit into from
Jan 14, 2024

Conversation

AlexJF
Copy link
Contributor

@AlexJF AlexJF commented Jan 12, 2024

Context

Width calculation logic in nui and builtin selects ignores prompt size. This makes it very easy to lose crucial information required to make a decision.

For example, here's what I see when running LspStart with the vtsls language server with master of stevearc/dressing.nvim:

image image

And here's what I see with this PR:

image image

Description

Initialize max_width (builtin) or line_width (nui) to the prompt length if it exists. Otherwise fallback to previous value of 1.

Test Plan

Used the manual select test with the following configuration:

-- Run this test with :source %

local function run_test(backend)
  local config = require("dressing.config")
  local prev_backend = config.select.backend
  config.select.backend = backend
  vim.ui.select({
    "first",
    "second",
    "third",
  }, {
    prompt = "Make a very informed selection after having collected all data and considered all viewpoints and opinions: ",
    kind = "test",
  }, function(item, lnum)
    if item and lnum then
      vim.notify(string.format("selected '%s' (idx %d)", item, lnum), vim.log.levels.INFO)
    else
      vim.notify("Selection canceled", vim.log.levels.INFO)
    end
    config.select.backend = prev_backend
  end)
end

-- Replace this with the desired backend to test
run_test("builtin")
--run_test("nui")

Before

image image

After

image image

@AlexJF AlexJF marked this pull request as ready for review January 12, 2024 18:51
@stevearc stevearc merged commit d7dde6a into stevearc:master Jan 14, 2024
7 checks passed
@stevearc
Copy link
Owner

Thanks for the PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants