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

[Feature request] fzf integration #2927

Open
2 tasks done
mattdkerr opened this issue Mar 11, 2024 · 16 comments
Open
2 tasks done

[Feature request] fzf integration #2927

mattdkerr opened this issue Mar 11, 2024 · 16 comments
Labels
Milestone

Comments

@mattdkerr
Copy link

Suggestion

fzf clink integration in cmder shell

Use case

No response

Extra info/examples/attachments

https://github.com/chrisant996/clink-fzf
or
https://github.com/chrisant996/clink-gizmos

Checklist

  • I have read the documentation and made sure this feature doesn't already exist.
  • I have searched for similar issues and found none that describe my feature request.
@DRSDavidSoft
Copy link
Contributor

@chrisant996 Can you please give a short list of requirements for implementing this in Cmder, if any? How much additional space would you estimate this will require?

@DRSDavidSoft
Copy link
Contributor

I also found this interesting gist for using fzf for history:
https://gist.github.com/rupeshtr78/ed42a23aec87b8463e4f04bec0308d63

@chrisant996
Copy link
Contributor

It's covered in detail in the docs here: https://github.com/chrisant996/clink-fzf/#fzf-integration-for-clink

Note the settings and key bindings.

Here is a quick summary to get you started:

@chrisant996 Can you please give a short list of requirements for implementing this in Cmder, if any?

These things are needed:

  1. Cmder would need to include fzf.exe (available from fzf).
  2. Cmder would need to include fzf.lua (available from clink-fzf).
  3. But before Cmder can include fzf.lua, I will have to make some changes to the fzf.lua script. Otherwise when Cmder includes the script it will interfere with everyone who is already using clink-fzf or clink-gizmos, and it will cause errors (it wasn't designed to have multiple copies of it all loaded at the same time, and if Cmder includes it then that will start happening for all Cmder users who were already using clink-fzf).

How much additional space would you estimate this will require?

The size of fzf.exe, which is around 4 MB.

I also found this interesting gist for using fzf for history:
https://gist.github.com/rupeshtr78/ed42a23aec87b8463e4f04bec0308d63

No, don't use that:

Clink-fzf already has support for fzf history. The cited gist is missing all of the rest of the fzf support. The gist appears to be only trying to change clink-fzf's history implementation to print "Searching History" first, but it has bugs in how it tries to do that, and it won't work properly. (And the official fzf support in other shells doesn't print that.)

@DRSDavidSoft DRSDavidSoft added this to the 1.4 milestone May 31, 2024
@DRSDavidSoft DRSDavidSoft added the 👆 clink Upstream issue in clink. label May 31, 2024
@chrisant996
Copy link
Contributor

Also, if you make Cmder by default enable the fzf default key bindings, you'll receive complaints about Tab no longer working as expected. Anyone who has bound Tab to any of the completion commands other than complete (e.g. menu-complete or clink-select-complete, or setting clink.default_bindings to windows) will suddenly experience Tab no longer working how they wanted.

I'll try to make the script do its best to compensate for that. It'll probably be able to "magically" do what people expect for 99% of users, but in some fancy custom configurations it may still interference.

@DRSDavidSoft
Copy link
Contributor

@chrisant996 Thanks, in addition to that, I believe it would be best for the "conflicting" features to be opt-in and enabled through some sort of config, and instructing users to use a command line text editor to enable those that they'd like.

Clink already has an excellent mechanism to enable/disable configuration through its set subcommand, we need to put more emphasis on that in the Cmder's README as well, in my opinion. Many users are missing all the great Clink features that you have developed, it would be awesome to include most of them in Cmder since they will not really introduce a noticeable size difference in the final package as well.

@DRSDavidSoft
Copy link
Contributor

fzf should be included in the Cmder full package (which already includes git). The lua scripts in my opinion are just as important as the clink-completions package and very lightweight.

@chrisant996
Copy link
Contributor

@chrisant996 Thanks, in addition to that, I believe it would be best for the "conflicting" features to be opt-in and enabled through some sort of config, and instructing users to use a command line text editor to enable those that they'd like.

The fzf integration in fzf.lua is opt-in; it's off by default (see its readme.md for more info).

I had interpreted the original request as "enable fzf integration by default".
If the plan is to merely to add the ability to enable fzf integration without needing to manually install additional files, then that seems fine -- although in Cmder's documentation it should probably call out how to enable the integration and what the side effects can be. And it should include a link to both the clink-fzf readme.md and to the fzf repo, for more info about usage and configuration and capabilities.

Clink already has an excellent mechanism to enable/disable configuration through its set subcommand, we need to put more emphasis on that in the Cmder's README as well, in my opinion. Many users are missing all the great Clink features that you have developed, it would be awesome to include most of them in Cmder since they will not really introduce a noticeable size difference in the final package as well.

I noticed that a change was made to turn on match.expand_envvars.

But I think instead Cmder should be using default_settings (and maybe also default_inputrc) for defining defaults. That way defaults can be changed without requiring any fancy scripting or conditions or special commands.

See notes under Files.

Cmder could simply include a file default_settings in the vendor\clink directory, and any values listed in it would override the built-in defaults. And if a user runs clink set name=value to set their own value for a setting, then the user configuration supersedes whatever default was listed for that setting in the default_settings file. I.e. the precedence order for each setting is (1) a value set by user, (2) a value in default_settings, and lastly (3) the built-in default value.

@DRSDavidSoft
Copy link
Contributor

Good points, also I like the idea of the lua scripts being able to deal with multiple instances being loaded from multiple places, since there are users who unknowingly install both. The integration can be made after handling such cases are made within the code, in the meantime I'll read the documentation to better understand how each feature works and what we can offer to first time users, as well as "pro" users who have been using Cmder/clink for some time. Clink really is the best shell on Cmder! 😄

@chrisant996
Copy link
Contributor

I like the idea of the lua scripts being able to deal with multiple instances being loaded from multiple places, since there are users who unknowingly install both.

On the surface, it sounds great, right? If all the different copies are identical, then it's great, right? But in reality, the different copies will usually end up being different. And then things get confusing and weird and problems happen and people report bizarre issues that take many hours to track down.

After doing some work to prototype "multiple versions coexisting", it's a bit of a mess (as should be expected), and there's no way to make it fully reliable for all kinds of duplication situations.

I'm leaning towards adding some Lua code in Cmder that loads a built-in copy unless another copy is loaded. That would make it so Cmder comes with built-in fzf support, but if a user has installed clink-fzf or clink-gizmos then the user's version gets used instead. It's easy to make that reliable.

Does Cmder plan to include fzf support for powershell, git-bash, etc? Or will it be only for cmd/Clink?

@daxgames
Copy link
Member

daxgames commented Jun 1, 2024

Just mu $.02. Keep in mind I have no idea how to do it for one shell much less all of them, I just think if we do it for one, we should do it for all. The same fzf.exe should work for all I would think.

@chrisant996
Copy link
Contributor

The same fzf, yes.

But the integration is different for each shell.

If the goal was "just run xyz set fzf_integration on and viola", then no, that isn't realistic unless someone wants to write a special xyz program that knows all the shells and can configure (and un-configure!) fzf for all the shells at once.

And I think that's also a mark against configuring Clink in a way that's too different from how powershell/etc behave.

If Cmder's goal is to provide an environment where all the shells behave the same, then that has to cater to the lowest common denominator and also avoid things that involve a bunch of configuration.

@daxgames
Copy link
Member

daxgames commented Jun 1, 2024

I'm not the one asking for the feature. It is installed in my Linux and configured and I never use it. Maybe because I know nothing about it. I have seen it used but it is more different stuff for me to remember and that does not always work so well for me. Too much going in and out as it is so new stuff has a hard time getting retained. :-)

@DRSDavidSoft
Copy link
Contributor

I'm leaning towards adding some Lua code in Cmder that loads a built-in copy unless another copy is loaded. That would make it so Cmder comes with built-in fzf support, but if a user has installed clink-fzf or clink-gizmos then the user's version gets used instead. It's easy to make that reliable.

@chrisant996 It's an excellent idea, especially since if a copy is already loaded, it wouldn't make sense to unload that so that Cmder could load its own. (If for some reason the user prefers the copy that is included in Cmder, then the idealistic approach would be to somehow load the included version before the existing copy outside Cmder is loaded). I haven't looked at the lua scripts deeply, so that's just my intuition.

Does Cmder plan to include fzf support for powershell, git-bash, etc? Or will it be only for cmd/Clink?

I haven't had heard of fzf support for powershell before, but I found PSFzf and a link that describes setting it up:
https://sathyasays.com/2023/04/11/powershell-fzf-psfzf/

I don't believe we should focus on integrating everything on every shell for now

If Cmder's goal is to provide an environment where all the shells behave the same, then that has to cater to the lowest common denominator and also avoid things that involve a bunch of configuration.

I agree.

How about another approach, then? We don't exactly have to include clink-fzf and/or clink-gizmos directly in Cmder, and leave it to the user to enable it.

What we can do instead is to give the user the option to install these packages into their Cmder installation in a compatible way that wouldn't break Cmder and/or conflict with the existing configuration. In this way, we can modify the current lua scripts in both Cmder and the clink-* scripts that would cooperate in a way that clink-fzf can be integrated into Cmder in a seamless way.

As @daxgames have said before, one of the core points of Cmder is that it's portable, you can copy it around using a USB stick and access the same shell and tools. Would it be possible to install clink-fzf and/or clink-gizmos inside an existing Cmder package for this purpose?

Another method of using clink-* tools, to me, is that they would be accessible from Cmder in a non-portable way. In other words, if a machine has clink-* tools installed, and Cmder is also installed on the machine, one would be able to use fzf from Cmder, with Cmder's clink profile configuration. Same with any other useful clink-gizmos utils.

Lastly, if this works, maybe in the future we can provide a xyz add clink-* or xyz install clink-* tool that would facilitate "downloading" and "installing" the latest versions of these packages directly from GitHub. I know I would appreciate a way that would allow the users to update their clink-completions package the same way that they can just do clink update and enjoy the latest version of clink. (Great update, by the way, @chrisant996).

@daxgames Also on the topic of cli commands, I have been working on a project called "Cmder Tweaker" on and off for the past couple of years, it's a script that I have been experimenting with that would allow applying customizations and configurations directly from the cli to Cmder, things such as setting the font, theme, default task, prompt colors, as well as registering profiles, context menu, and adding/removing env vars. I haven't committed it to a repo, since I'm experimenting with different scripting languages (powershell, cmd, bash), but if it comes to fruition, it would be a nice tool to add to Cmder where the user would have the ability to do cmder-tweaks change font, cmder-tweaks add external-package, etc. directly from CLI. If it proves useful, I was even thinking of moving its code to C++ and adding it to the launcher in a PR, if it'd be helpful to the users.

@chrisant996
Copy link
Contributor

I'm leaning towards adding some Lua code in Cmder that loads a built-in copy unless another copy is loaded. That would make it so Cmder comes with built-in fzf support, but if a user has installed clink-fzf or clink-gizmos then the user's version gets used instead. It's easy to make that reliable.

@chrisant996 It's an excellent idea, especially since if a copy is already loaded, it wouldn't make sense to unload that so that Cmder could load its own. (If for some reason the user prefers the copy that is included in Cmder, then the idealistic approach would be to somehow load the included version before the existing copy outside Cmder is loaded). I haven't looked at the lua scripts deeply, so that's just my intuition.

I simplified it further: the last copy of the script loaded will win.

Which means Cmder will be able to simply include clink-fzf as a vendor directory like it does for clink-completions, and Cmder load the script using the same kind of approach it uses for clink-completions.

Does Cmder plan to include fzf support for powershell, git-bash, etc? Or will it be only for cmd/Clink?

I haven't had heard of fzf support for powershell before, but I found PSFzf and a link that describes setting it up: https://sathyasays.com/2023/04/11/powershell-fzf-psfzf/

I don't believe we should focus on integrating everything on every shell for now

It sounds like there are differences of opinions, so I think that's something for the Cmder maintainers to work out among yourselves.

How about another approach, then? We don't exactly have to include clink-fzf and/or clink-gizmos directly in Cmder, and leave it to the user to enable it.

What we can do instead is to give the user the option to install these packages into their Cmder installation in a compatible way that wouldn't break Cmder and/or conflict with the existing configuration. In this way, we can modify the current lua scripts in both Cmder and the clink-* scripts that would cooperate in a way that clink-fzf can be integrated into Cmder in a seamless way.

I'm not sure what you mean. Are you suggesting that clink-* scripts should be made compatible with Cmder so that users can install the clink-* scripts? If that's what you mean, then that has always worked, and there are already many Cmder users who install clink-gizmos or clink-fzf.

As @daxgames have said before, one of the core points of Cmder is that it's portable, you can copy it around using a USB stick and access the same shell and tools. Would it be possible to install clink-fzf and/or clink-gizmos inside an existing Cmder package for this purpose?

The question is ambiguous. Are you trying to figure out how to tell Clink what directories to load scripts from, so that users can manually add directories inside the Cmder directory hierarchy and Clink will find them and load them?

Cmder is already doing that to load clink-completions. Cmder can load scripts from wherever it wants.

Another method of using clink-* tools, to me, is that they would be accessible from Cmder in a non-portable way. In other words, if a machine has clink-* tools installed, and Cmder is also installed on the machine, one would be able to use fzf from Cmder, with Cmder's clink profile configuration. Same with any other useful clink-gizmos utils.

Yes, that's what people have been doing so far.

Lastly, if this works, maybe in the future we can provide a xyz add clink-* or xyz install clink-* tool that would facilitate "downloading" and "installing" the latest versions of these packages directly from GitHub. I know I would appreciate a way that would allow the users to update their clink-completions package the same way that they can just do clink update and enjoy the latest version of clink. (Great update, by the way, @chrisant996).

It's possible to make an auto-updater for clink-completions inside Cmder. If a script updater is on by default, then a side effect is that it will automatically silently discard any local edits that users have made to the scripts.

It's also possible to make a script that can update clink-completions. It's just a matter of extracting the contents from the latest .zip release file into the vendor/clink-completions directory.

@DRSDavidSoft
Copy link
Contributor

@chrisant996 Sorry for the ambiguity, you're quite right. Additionally, I wonder what approach could be used to include the scripts with Cmder yet leave it disabled. Do we need to put the scripts in a directory that won't be loaded by default, and either a) ask the user to move the files into another directory, or b) change the configuration to load scripts from the specified path? I'm under the assumption that there are no config files that are used to disable/enable scripts that are loaded by clink (sorry if that's the case).

If a script updater is on by default, then a side effect is that it will automatically silently discard any local edits that users have made to the scripts.

That's a non-issue in my opinion since we have instructed users for many versions now (since a couple of years, I believe) NOT TO modify scripts in the vendor directory as they'll be overwritten in newer versions. @daxgames even took great care to move any files with potential configuration to the config directory to avoid losing modifications on each update. In any case, I believe the standard and recommended way that is suggested to update Cmder is to simply extract the ZIP file directly over the previous version which essentially has the same effect, which means it would be a good idea if the auto updater script would do this as well.

This is off topic, but may I ask that you'd add this functionality directly in either clink or clink-completions, since it's already a working feature of clink? That way, we could include and use it in Cmder, as well.

@chrisant996
Copy link
Contributor

chrisant996 commented Jun 2, 2024

Do we need to put the scripts in a directory that won't be loaded by default, and either a) ask the user to move the files into another directory, or b) change the configuration to load scripts from the specified path? I'm under the assumption that there are no config files that are used to disable/enable scripts that are loaded by clink (sorry if that's the case).

Cmder needs to do what Cmder does for clink-completions (please follow the link to see what Cmder already does).

The clink-completions directory is a custom directory, and Cmder has Lua code that specifically loads each of the scripts in that directory. Cmder can continue to use the same approach for any other Lua script collections that Cmder wants to include.

may I ask that you'd add this functionality directly in either clink or clink-completions, since it's already a working feature of clink? That way, we could include and use it in Cmder, as well.

I welcome the question, but the answer has to be "no" about building it into Clink, because:

  1. Neither clink-completions nor clink-gizmos are part of Clink. They aren't features of Clink, they're separate things which depend on Clink and add enhancements to Clink. (Like how a user's personal .bat scripts aren't a feature of CMD, they're a feature of the user's personal configuration.)
  2. Clink's updater is for Clink. It shouldn't update things other than Clink; that would be a recipe for trouble.
  3. The clink-completions and clink-gizmos repos already have an update mechanism. Their readme files recommend to install using git, and git pull is the update command (or other standard git workflows such as upstreams and branches).
  4. Cmder has a custom installer for the scripts, in effect. An updater for Cmder's scripts would need to be part of Cmder.

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

No branches or pull requests

4 participants