Repository: junegunn/fzf (language: go)

Issue:
feature request: host completion improvements
- [X] I have read through the manual page (`man fzf`)
- [X] I have the latest version of fzf
- [X] I have searched through the existing issues

## Info

- OS
    - [X] Linux
    - [ ] Mac OS X
    - [ ] Windows
    - [ ] Etc.
- Shell
    - [X] bash
    - [ ] zsh
    - [ ] fish

Hey.

I think the current host completion has a number of areas where it could be improved:
1. It's not (easily) configurable, e.g. it would be nice to have a user switch that disables v4 and/or v6 addresses, and perhaps options that do sorting
2. It lacks many names that are shown by bash-completion’s `_known_hosts_real()`, which is quite powerful and IIRC even checks `ssh_config` for other configured `GlobalKnownHostsFile` and `UserKnownHostsFile`. `fzf`’s current completion seems to ignore any host (which are more or less all of them) that I have in the `GlobalKnownHostsFile`. `_known_hosts_real()` also has many more features, I think it can optionally use Avahi and ruptime.


For (2), I think it would be best if `fzf` would simply use `_known_hosts_real()` - why re-inventing the wheel, if bash-completion guys already maintain that properly?

Based on the 0.42.0 `fzf`, I tried roughly the following:
```
_fzf_host_completion() {
  _fzf_complete +m -- "$@" < <(
        _known_hosts_real ''
        printf '%s\n' "${COMPREPLY[@]}" | sort -u -V
  ) 
} 
```
and at a first glance it seemed to work:
- The `sort-u` is necessary as `COMPREPLY` contained many duplicates (no idea why).
- `-V` is just for sorting ... ideally this would be configurable,... plus I'd like a sort mode that sort by domains[0].

Some open points:
- Is `_known_hosts_real()` available for sure? I mean bash-completion does load completions for commands dynamically (i.e. only when the command is actually completed), but I'd blindly guess that `_known_hosts_real()` is always available and we don't need to "import" it first.
- Do we have any side-effects? At least `COMPREPLY` is set. Haven't checked whether `_known_hosts_real()` uses other non-local variables. As long as the function is called within process substitution `<(…)` this shouldn't be a problem, but I've seen that in edfdcc8cee31e227a5f7eda9c1faa4893757645c you placed the printing code in a separate function, so if that would ever be called outside process substitution it might cause unwanted side effects.
- What about the config env vars recognised by `_known_hosts_real()`, like `COMP_KNOWN_HOSTS_WITH_AVAHI`? Should we just use them directly, or clean them and initialise them only by our own e.g. `FZF_KNOWN_HOSTS_WITH_AVAHI` counterparts (so that people could configure bash-completion and fzf differently)?


Discussion / hints:
[0] With sorting by domains I mean e.g.
```
9.aq
5.a.com
6.a.com
1.b.com
```
I.e. sorted reversely by each domain component, and within the same component one should again do alphabetical (or better natural) sort.

The advantage of such a sorting is, that host that (possibly) "belong together" are sorted next to each other
Thanks for the suggestion, but before we discuss the topic in detail I have to mention that I'm not particularly looking to improve/expand the shell features (key bindings and fuzzy completion) we provide here. They are intentionally kept simple (with minimal configuration knobs)

* to reduce the maintenance burden of keeping three implementations (bash, zsh, fish) in sync
* to serve as the basic reference implementation with no bells and whistles for those who are trying to learn how to write fzf-powered shell functionality on their own.

I believe extended versions of the scripts should be maintained as separate projects.

* https://github.com/junegunn/fzf/issues/2805#issuecomment-1110366399
* https://github.com/junegunn/fzf/issues/1761#issuecomment-555581153

https://github.com/Aloxaf/fzf-tab is a good example.

A separate project focused on a specific shell doesn't have the problem of maintaining the same functionality across three different shells, so it should be easier to add more advanced features. It's going to be used by advanced users who know what they're doing so the maintainer can experiment, break, and fix things with more confidence.
> Thanks for the suggestion, but before we discuss the topic in detail I have to mention that I'm not particularly looking to improve/expand the shell features (key bindings and fuzzy completion) we provide here. They are intentionally kept simple (with minimal configuration knobs)

Well, while on one side I can see your points (especially keeping `fzf` itself clean and simple and also not having to add functionality for numerous shells at the same time), in practise this poses IMO several problems.
- Generally it's very bad to distribute software outside some form of distribution or other package management system (no security support, no systematic way for regular updates, dependency mess, breaking any form up centralised management (checking for updates, automatic deplyoment, etc. pp.). While this seems to have become a new fashion, one should still try to avoid it as best as one can.
  If any such functionality is now put in 3rd party repos, there's in practise little chance that the respective code gets ever packaged, especially if there are many (like a separate `fzf-*` for every little thing). By keeping at least generic stuff in the main `fzf` project, platform that packages this, should at least sooner or later get the new functionality.
- In practise, such 3rd party repos are also rather prone to get outdated or have other issues that are never fixed. This is even more likely when the 3rd party project would expand features that are already in the main `fzf`. Consider there was now another project that extends host completion by overwriting `_fzf_host_completion()` it's all too likely that sooner or later you make a change that doesn't work with their state anymore.

I think bash-completion might serve as a good model:
- it tries to include base functionality of generic use
- it ships the "plugins" for major programs or such of generic use, if the respective project doesn't ship the completions by itself
- it still easily possible for some actual project (like `git` or) so to maintain and ship its own set of completions

Another possible way, that would help you to keep the `fzf` repo clean from all to many shell stuff, could be that you generally outsource all shell integration into separate `fzf-<shell>` repos.<br>
I guess these would have at least some chance to get packaged, and there would no longer be so many possible areas of conflict between shell integration code shipped by the main `fzf` package and such from 3rd party project, since, that would be again in one repo.

That would be again similar to how things are handled in the `bash` world:
- there's `bash` itself, just the shell - which would be `fzf` - just the fuzzy finder
- `bash-completion`, generic completion infrastructure and support for some major programs - which would be `fzf-bash`, `fzf-zsh`, etc.
- downstream projects like `git` shipping their own completions - which would be 3rd party `fzf-something` projects


What's your opinion on that?

Any beyond that, would you still accept code for the actual feature idea itself?
btw: (Your) current implementation of `__fzf_list_hosts()` also gives errors, if `~/.ssh/known_hosts` or `/etc/hosts` doesn't exist. But TBH, I'd very much prefer to go the "use `_known_hosts_real()` way" proposed here, rather than fixing this. ;-)

So would be happy if you'd agree to get something like that merged.

Fix patch:
diff --git a/shell/completion.bash b/shell/completion.bash
index 7532bb99b6b..3bc695c86c1 100644
--- a/shell/completion.bash
+++ b/shell/completion.bash
@@ -407,15 +407,34 @@ _fzf_proc_completion_post() {
   command awk '{print $2}'
 }
 
-__fzf_list_hosts() {
-  command cat <(command tail -n +1 ~/.ssh/config ~/.ssh/config.d/* /etc/ssh/ssh_config 2> /dev/null | command grep -i '^\s*host\(name\)\? ' | command awk '{for (i = 2; i <= NF; i++) print $1 " " $i}' | command grep -v '[*?%]') \
-    <(command grep -oE '^[[a-z0-9.,:-]+' ~/.ssh/known_hosts | command tr ',' '\n' | command tr -d '[' | command awk '{ print $1 " " $1 }') \
-    <(command grep -v '^\s*\(#\|$\)' /etc/hosts | command grep -Fv '0.0.0.0') |
-    command awk -v "user=$1" '{if (length($2) > 0) {print user $2}}' | command sort -u
-}
+# To use custom hostname lists, override __fzf_list_hosts.
+# The function is expected to print hostnames, one per line as well as in the
+# desired sorting and with any duplicates removed, to standard output.
+if ! declare -F __fzf_list_hosts > /dev/null; then
+  if declare -F _known_hosts_real > /dev/null; then
+    # if available, use bash-completions’s _known_hosts_real() for getting the list of hosts
+
+    __fzf_list_hosts() {
+      # set the local attribute for any non-local variable that is set by _known_hosts_real()
+      local COMPREPLY=()
+
+      _known_hosts_real ''
+      printf '%s\n' "${COMPREPLY[@]}" | command sort -u --version-sort
+    }
+  else
+    # otherwise, get a list of hosts on our own
+
+    __fzf_list_hosts() {
+      command cat <(command tail -n +1 ~/.ssh/config ~/.ssh/config.d/* /etc/ssh/ssh_config 2> /dev/null | command grep -i '^\s*host\(name\)\? ' | command awk '{for (i = 2; i <= NF; i++) print $1 " " $i}' | command grep -v '[*?%]') \
+        <(command grep -oE '^[[a-z0-9.,:-]+' ~/.ssh/known_hosts 2> /dev/null | command tr ',' '\n' | command tr -d '[' | command awk '{ print $1 " " $1 }') \
+        <(command grep -v '^\s*\(#\|$\)' /etc/hosts 2> /dev/null | command grep -Fv '0.0.0.0') |
+        command awk '{if (length($2) > 0) {print $2}}' | command sort -u
+    }
+  fi
+fi
 
 _fzf_host_completion() {
-  _fzf_complete +m -- "$@" < <(__fzf_list_hosts "")
+  _fzf_complete +m -- "$@" < <(__fzf_list_hosts)
 }
 
 # Values for $1 $2 $3 are described here
@@ -431,7 +450,7 @@ _fzf_complete_ssh() {
     *)
       local user=
       [[ "$2" =~ '@' ]] && user="${2%%@*}@"
-      _fzf_complete +m -- "$@" < <(__fzf_list_hosts "$user")
+      _fzf_complete +m -- "$@" < <(__fzf_list_hosts | command awk -v user="$user" '{print user $0}')
       ;;
   esac
 }
diff --git a/shell/completion.zsh b/shell/completion.zsh
index a0406cb428c..d5ef40bc99c 100644
--- a/shell/completion.zsh
+++ b/shell/completion.zsh
@@ -218,16 +218,21 @@ _fzf_complete() {
   command rm -f "$fifo"
 }
 
-__fzf_list_hosts() {
-  setopt localoptions nonomatch
-  command cat <(command tail -n +1 ~/.ssh/config ~/.ssh/config.d/* /etc/ssh/ssh_config 2> /dev/null | command grep -i '^\s*host\(name\)\? ' | awk '{for (i = 2; i <= NF; i++) print $1 " " $i}' | command grep -v '[*?%]') \
-    <(command grep -oE '^[[a-z0-9.,:-]+' ~/.ssh/known_hosts | tr ',' '\n' | tr -d '[' | awk '{ print $1 " " $1 }') \
-    <(command grep -v '^\s*\(#\|$\)' /etc/hosts | command grep -Fv '0.0.0.0') |
-    awk -v "user=$1" '{if (length($2) > 0) {print user $2}}' | sort -u
-}
+# To use custom hostname lists, override __fzf_list_hosts.
+# The function is expected to print hostnames, one per line as well as in the
+# desired sorting and with any duplicates removed, to standard output.
+if ! declare -f __fzf_list_hosts > /dev/null; then
+  __fzf_list_hosts() {
+    setopt localoptions nonomatch
+    command cat <(command tail -n +1 ~/.ssh/config ~/.ssh/config.d/* /etc/ssh/ssh_config 2> /dev/null | command grep -i '^\s*host\(name\)\? ' | awk '{for (i = 2; i <= NF; i++) print $1 " " $i}' | command grep -v '[*?%]') \
+      <(command grep -oE '^[[a-z0-9.,:-]+' ~/.ssh/known_hosts 2> /dev/null | tr ',' '\n' | tr -d '[' | awk '{ print $1 " " $1 }') \
+      <(command grep -v '^\s*\(#\|$\)' /etc/hosts 2> /dev/null | command grep -Fv '0.0.0.0') |
+      awk '{if (length($2) > 0) {print $2}}' | sort -u
+  }
+fi
 
 _fzf_complete_telnet() {
-  _fzf_complete +m -- "$@" < <(__fzf_list_hosts "")
+  _fzf_complete +m -- "$@" < <(__fzf_list_hosts)
 }
 
 # The first and the only argument is the LBUFFER without the current word that contains the trigger.
@@ -241,7 +246,7 @@ _fzf_complete_ssh() {
     *)
       local user=
       [[ $prefix =~ @ ]] && user="${prefix%%@*}@"
-      _fzf_complete +m -- "$@" < <(__fzf_list_hosts "$user")
+      _fzf_complete +m -- "$@" < <(__fzf_list_hosts | awk -v user="$user" '{print user $0}')
       ;;
   esac
 }


Test patch: