Additions to the InputSelector (resolving #4226)

[Danielkonge]

These changes should resolve my requests in #4226 . As mentioned in the issue this is my first time working in Rust, so please let me know if I am doing something weird in any of the submitted code. I have tested this by changing stuff around in my config, and everything seems to work as I would expect, but there might still be tiny bugs.

I don't know if we might want to move some of the logic a bit? E.g., do we want global config options for the InputSelector?

Also, I was thinking about adding colors in the selection when you press the first key in a two key key-sequence, but I haven't gotten around to that yet (and I am not entirely sure where to start).

[wez]

Thanks for this!

re: global config, no, I think having the options be part of the key assignment is good.

re: colors, I'm unsure about that. We don't have a similar concept in quickselect and we haven't had anyone raise the desire or need to add it there, so I suspect that it probably isn't necessary.

I've made a few suggestions!

[wez]

This seems like a smart approach. Should / also have similar treatment? Are there other unmodified Char patterns below that might also need similar treatment?

FWIW, I think it would be ok to inline this check where the event is being matched; I'm less concerned about the runtime cost of alphabet.contains than I am about the readability of this function, and I don't think that having a separate local variable that is many lines apart from the logic helps with readability.

[Danielkonge]

I have inlined it as self.args.alphabet.contains("j") now.

[Danielkonge]

To do something similar for /, we would need to add another way to search, I think. I am not sure it makes sense to add that.

[Danielkonge]

I have tried fixing (most of) the suggested stuff now. I probably won't have a lot of time to look at this the next few days.

Also I have found a weird bug, but I don't think it's in the InputSelector, but I will post it here for now so I can get back to it later if I can figure out what courses it. To reproduce it, the following config is enough (make sure to use directories that exist):

local wezterm = require('wezterm')
local act = wezterm.action
local config = wezterm.config_builder()

config.leader = { key = 'Space', mods = 'CTRL', timeout_milliseconds = 3000 }

config.keys = {
  {
    key = 'w',
    mods = 'LEADER',
    action = wezterm.action_callback(function(window, pane)
      -- Here you can dynamically construct a longer list if needed

      local home = wezterm.home_dir
      local workspaces = {
        { id = home .. '/work', label = 'Work' },
        { id = home .. '/personal', label = 'Personal' },
        { id = home .. '/.config', label = 'Config' },
      }

      window:perform_action(
          act.InputSelector {
              action = wezterm.action_callback(function(inner_window, inner_pane, id, label)
                  if not id and not label then
                      wezterm.log_info 'cancelled'
                  else
                      wezterm.log_info('id = ' .. id)
                      wezterm.log_info('label = ' .. label)
                      inner_window:perform_action(
                          act.SwitchToWorkspace {
                              name = label,
                              spawn = {
                                  label = 'Workspace: ' .. label,
                                  cwd = id,
                              }
                          },
                          inner_pane
                      )
                  end
              end),
              title = 'Choose Workspace',
              choices = workspaces,
          },
          pane
      )
    end),
  },
}

wezterm.on('update-status', function(_, pane)
    local cwd_uri = pane:get_current_working_dir()
end)

return config

When picking in the InputSelector either by pressing Enter (both with labels and when fuzzy searching) or clicking with the mouse, everything works as expected. When picking by clicking a number (one of the labels before this pull request), everything still looks like expected, but you get something like the following Error if you check the log:

ERROR  wezterm_gui::termwindow > while processing update-status event: pane id 19 not found in mux
stack traceback:
        [C]: in method 'get_current_working_dir'
        [string "/Users/daniel/.config/wezterm/wezterm.lua"]:153: in function <[string "/Users/daniel/.config/wezterm/wezterm.lua"]:152>

[Danielkonge]

I changed a few things so that you can use upper case letters in the InputSelector "labels" for picking items and I added a fuzzy_description. I don't really plan to add more stuff now, but I am willing to edit anything you think should be changed.

Also, as discussed in the Matrix room the error mentioned above seems to be a race condition problem, but it doesn't affect the use of the InputSelector since it "fixes" itself in about 15ms.

[wez]

Thanks! Almost there: a couple more suggestions!

[wez]

I tend to avoid using examples that use a leader key because they are a bit less copy-and-pastable; often the choice of leader and parameters tends to be very much a personal preference or opinion, and folks that are new to wezterm will tend to copy and paste without trying to understand what they are doing.

I think using a leader is sort of OK here, but I'd feel happier if it omitted the timeout_milliseconds parameter so that this example doesn't feel like having that particular parameter is necessary to achieve the effect described in the title for this example.

[Danielkonge]

I can just make another key binding. I will change it to Ctrl-Shift-S for now if that is better.

(I think the standard timeout_milliseconds are a bit too low for newcomers.)

[wez]

if you take my suggestion above, then these test changes no longer need to be part of this diff. However, it would be good to include a test case that shows the expected output of the new case-preserving mode

[Danielkonge]

I have taken the suggestions and added a few tests now.

[wez]

Thank you!

[bew]

Hello, very nice PR 👌
I think the word description isn't a great here, and prompt would better fit

Wdyt?

[Danielkonge]

I could go through the code and docs and change it, but I am not sure prompt would be a good idea. I would like to keep the naming similar to what you see in PromptInputLine (see https://wezfurlong.org/wezterm/config/lua/keyassignment/PromptInputLine.html ) and at the same time avoid confusion with this overlay.

Of course, we could change description to prompt in PromptInputLine too. What do you think @wez?

[wez]

I think description is fine; prompt doesn't feel like quite the right word anyway, and this is consistent with promptinputline.

[bew]

prompt doesn't feel like quite the right word anyway

🤔 huh? My understanding of these words:

• description for me is a text that describes something, can be long, and doesn't ask for any action
• prompt for me is the text just before a text field input, it's a 'call-to-action' (e.g. ends with a : )

And from where I see description & fuzzy_description being used in the code, prompt matches more than description.

Or maybe I'm missing some meaning of the english language? (I'm not english native)

[Danielkonge]

I think description can be used to describe pretty much anything; also something asking for an action. In this case description can be used to describe either what your current InputSelector does, what different keys do or really anything else that you think helps you when using the InputSelector, which I think is better than prompt (it is better to be a bit more general, since it has different use cases).

The way description is currently used in the code is not the only way to use it, it is just what I had in mind for myself.

[bew]

Sure I see how description is more generic here, but it definitely feels weird to me to have what I see as the prompt in a description field.

For the different use cases I'd use a separate options: one for the prompt and one for the description, if any.
👉 This is what fzf does for example, where both the prompt (named prompt) and the description (named header) exist.
And since they are separate the layout can be changed so that for example the header is displayed before the prompt line or after (but before the items to select)

[wez]

prompt is a compatible word, it just feels a bit more "active" or stronger than I think is necessary here. If I hadn't used description elsewhere I'd be more into it here, but as it stands: I don't have sufficiently strong feelings about it to actively want to change it