One thing I forgot to implement is relative paths for the Include directive. I will do a rebase by the end of the weekend.

Done. If anyone has time, a code review will be greatly appreciated. I am still learning fish, and I generally dislike shell scripting for any tasks which are not trivial.

I adhered to the style guide with the exception to using fish_indent since it didn't do a very good job at indentation. Please see the before and after using fish_indent:

    for file in $ssh_configs
        if test -r $file
            # Print hosts from system wide ssh configuration file
            string match --regex --ignore-case '^\s*Host\s+\S+' <$file \
                # We only want the value(s)
                | string replace --regex --ignore-case '^\s*Host\s+' '' \
                # Print one per line
                | string trim | string replace --regex '\s+' ' ' | string split ' ' \
                # Ignore hosts with a glob
                | string match --invert '*\**'
            # Extract known_host paths
            set known_hosts $known_hosts (string match -ri '^\s*UserKnownHostsFile|^\s*GlobalKnownHostsFile' <$file \
                | string replace -ri '.*KnownHostsFile\s*' '')
        end
    end

    for file in $ssh_configs
        if test -r $file
            # Print hosts from system wide ssh configuration file
            string match --regex --ignore-case '^\s*Host\s+\S+' <$file # We only want the value(s)
 | string replace --regex --ignore-case '^\s*Host\s+' '' # Print one per line
 | string trim | string replace --regex '\s+' ' ' | string split ' ' # Ignore hosts with a glob
 | string match --invert '*\**'
            # Extract known_host paths
            set known_hosts $known_hosts (string match -ri '^\s*UserKnownHostsFile|^\s*GlobalKnownHostsFile' <$file \
                | string replace -ri '.*KnownHostsFile\s*' '')
        end
    end

I adhered to the style guide with the exception to using fish_indent since it didn't do a very good job at indentation.

I understand why you deviated from the output of fish_indent but that doesn't matter. Just as the C++ code has to conform to what clang-format dictates so does the fish script code have to conform to fish_indent. The solution is to improve fish_indent and/or rewrite the code into discrete steps so that it doesn't need special-casing.

If you separate your style changes into a separate PR from the functional changes it will be much easier to approve each of them.

However, there is an established pattern of using short flags in the completion scripts.

It's not universal (duply, launchctl, lxc, obname) and I think long flags are fine.

It's not universal (duply, launchctl, lxc, obname) and I think long flags are fine.

Sure, but long options are rare. And I can't find a single place where --regex is used rather than -r or --invert rather than -v.

But the main reason for my observation is that the existing scp/ssh complete commands uses the short forms; e.g., -c rather than --command. And being consistent with the surrounding code is a best practice. If someone wants to open an issue to make the case that we should standardize on long options and do the work to restyle the existing scripts I probably wouldn't object since, as I said earlier, I generally prefer long options in scripts for the increased clarity they provide.

@daleeidd: We want to see you contribute more improvements like this one. However, please keep in mind that mixing style and functional changes is always a bad idea since it can delay the inclusion of useful functional changes. Changes to the existing style of the code is almost always controversial. It is almost always better to separate the two types of change into separate pull-requests.

I'll modify your pull-request to address the gratuitous style changes and merge it tomorrow. I would be happy to see you open a new issue that talks about why you felt the need to deviate from the existing fish script style.

Thank you. I was planning on doing the changes myself, but I have fallen on hard times as of late, and I could not muster the energy to fix it. Cleaning that code was a must needed distraction so I couldn't help myself. My apologies.

No need to apologize, @daleeidd. Until roughly ten years ago I too greatly underestimated the value of having a consistent style to the code in any given project. And I've been earning a living as a programmer since 1978. Appreciating the value of such guidelines can take a long time to really sink in to the point where you start objecting when you see a best practice violated. 😄

As I said earlier the non-style aspects of your change are great. I hope I haven't scared you away from making contributions in the future.

Squash merged as commit eff6b98 plus another commit by myself to address the style problems. Hopefully I didn't introduce any bugs not present in @daleeidd's original change.