Add a way to set the CommandCollector

[idanarye]

Prior to 0.11, SkimOptions had a cmd_collector field which could be used to generate SkimItems based on the user-editable command query.

(In practice, because CommandCollector was not exported, this was not possible. I had to make a PR to the two_percent fork to expose it before I could use it)

In 0.11 I see that it's removed - I believe as part of making SkimOptions comform to Clap? - which means that it is no longer possible to register Rust functions that generate SkimItems from a user-editable command - at most one can register a shell command template. It also means that interactive mode can no longer work with custom SkimItems - only with strings.

Is it possible to restore this functionality? I don't suggest it should go back SkimOptions, but maybe some other way to inject it? Maybe the second argument of skim_run could be an enum with wither SkimItemReceiver or a CommandCollector? (because these two are mutually exclusive. Or at least they should be)

Other missing similar options are selector and engine_factory. That enum approach will not work for them though...

[LoricAndre]

These options were not actually options, and were added to SkimContext instead: https://docs.rs/skim/0.11.12/skim/context/struct.SkimContext.html.
CommandCollector was not, afaik, used in run_with, but directly in the binary through real_main (or sk_main now), so I'm not sure how you are trying to use it exactly.
The context module is public now, so you whould be able to use it without any issues, and it should not make any difference in how you use the rest of the library. If it does, please tell me as it means that the latest releases have introduced a regression in the library.

[idanarye]

Is it possible to create a SkimContext on my own and then run the fuzzy matcher UI with it? I don't see any such method in the docs...

[idanarye]

Also - the cmd_collector of SkimContext is a SkimItemReader rather than something that generates a new SkimItemReader each time the command changes in interactive mode.

[LoricAndre]

I'm really not sure what you are trying to achieve here, could you share a snippet ?

[LoricAndre]

There is a regression in the lib, you are right that cmd_collector is now a Rc<RefCell<SkimItemReader>> instead of a Rc<RefCell<dyn CommandCollector>>

[LoricAndre]

We'll fix that ASAP, but I'd like to understand your usage better first

[idanarye]

It's not really a regression because it never worked in Skim since CommandCollector was hidden. I had to switch to two_percent in order to use it.

I'll need some time to write a small example snippet, but my actual usecase is https://github.com/idanarye/nu_plugin_skim. Basically it's a plugin for using Skim in Nushell. Nushell is a rich shell that supports structured objects, and what my plugin does is run Skim with these objects instead of strings. This means that:

1. At the end of the pipe you get the same structured object you fed into Skim.
2. Inside the pipe you can format and preview the items using Nu closures that receive structured objects
3. Interactive mode is done with a Nu closure rather than a regular Bash command, meaning it can generate Nu object itself.

Here is example of interactive mode with my plugin:

Peek.2024-11-24.18-27.mp4

[LoricAndre]

From what I understand, this is for use in interactive mode. Is there any other use case you're missing that requires cmd_collector ?

[idanarye]

Not really - AFAIK the purpose of the command collector is to augment interactive mode. If interactive mode is not used and the cmd does not change, there is little difference between using it and just piping everything from outside.

[LoricAndre]

This should be enough for us to work with, I think this means we need to bring the options moved to the context back where they were, then make the options module public.

[idanarye]

Not sure what you mean by "options moved to the context back where they were". Weren't the options passed directly to Skim::run_with? Or is there some other hidden options struct?

[LoricAndre]

The missing options that were moved to a new SkimContext struct will be moved back

[LoricAndre]

This should not have any impact on you, other than making your use case work

[LoricAndre]

@idanarye could you please try it with #613 ?

[idanarye]

@LoricAndre Thanks - it seems to work.

I still need two more things before I can migrate it back from two_percent though:

• Make RankCriteria publicly accessible (currently it's pub, but hidden within a non-exported module)
• Add selector back to SkimOptions.

The former is easy and I can make a PR myself, but I'm not familiar enough with the codebase to do the latter. Is it something that should belong with #613 (since it's another non-CLI thing to add back to SkimOptions) or is it a completely separate issue (since it's a different mechanism)?

[LoricAndre]

I'll try to add both to this PR and rename it later this week

[idanarye]

Thanks!

[LoricAndre]

I had some time and motivation, so I added them

[LoricAndre]

You should be able to set selector in options, and RankCriteria is public at crate level now

[idanarye]

Thanks! tiebreak works now, but selector doesn't - even if I pass something that always returns true it does not select anything.

Should I make this a separate ticket?

[LoricAndre]

No, but I think we should move this discussion to the PR, I'll take a look at selector when I can