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

Re-enable and improve nixops completions #13

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

Avaq
Copy link

@Avaq Avaq commented Mar 29, 2020

Motivation

To close #12

Changes

For now I simply re-enabled the nixops completions, and I'm sourcing the updated file in my bashrc. I must say, it appears to represent a quality of life improvement already. I will continue using these completions for the coming weeks as I'm working with NixOps, and whenever I run into an issue I will fix it and push it to this branch. Hopefully this will lead to an eventually acceptable state for nixops support to be merged back upstream. :)

@Avaq Avaq changed the title Re-enable and fix nixops completions Re-enable and improve nixops completions Mar 29, 2020
Note that this completion does not (yet) respect the
state option passed that might've been passed already.
The possible deployments are taken from a database.
The database file is also passed on the command line.
If the user passes a database file, we'd like to list
the deployments from that file, and not the default one.
Machine names were being completed from _known_hosts. These commonly
don't align with actual machine names in a nixops deployment.

This commit introduces a new completion type for nixops machines based
on output from 'nixops info'. Furthermore, if the user had already
supplied a --state or --deployment argument, then these are respected.
Comment on lines +998 to +999
while read -r _ id _ name _; do deployments+=("$id" "$name")
done < <(nixops list $statecmd | tail -n +4 | head -n -1)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like having to parse command output like this. It's needed because nixops returns things in pretty tables. As far as I could tell, there's no "plumbing" versions of their commands.

Currently, some commands are explicitly handled by a case match that
does nothing, and other commands simply have no corresponding match.

This commit should improve the consistency of the code when it comes to
how commands that don't have any special arguments are handled.
See 'man nixops' > "COMMON OPTIONS PASSED ALONG TO NIX".

I could not reuse them from the 'nix_common_nixos_rebuild' variable,
because some options are missing, and some options are missing the
short flag alias.

I double-checked that every option in this list is in fact a valid
option for each of the main commands.
@Avaq
Copy link
Author

Avaq commented May 14, 2020

I've been working with these completions for a while now. I encounter one problem with it related to completion of the device names/ids. Namely that nixops list has side-effects which write to an output stream, like when it needs to download a channel update. So sometimes it outputs more than just the table of machines, and it messes with the completions. I now believe more strongly that, before this work can be considered stable, I should find a better way to complete machine names than to rely on nixops list.

@Avaq
Copy link
Author

Avaq commented Jun 9, 2021

I've been using these completions for over a year now. They're really just fine and absolutely better than not having them. I suggest just merging this despite the completions for names/ids having the occasional problem. Maybe once merged someone might be prompted to fix the problem at the source.

@Avaq Avaq marked this pull request as ready for review June 9, 2021 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

It doesn't seem like completions for nixops are actually working
1 participant