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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overhaul completions, redo #6693 #8131

Merged
merged 1 commit into from
Oct 23, 2023

Commits on Oct 16, 2023

  1. Overhaul completions, redo NixOS#6693

    As I complained in
    NixOS#6784 (comment) (a
    comment on the wrong PR, sorry again!), NixOS#6693 introduced a second
    completions mechanism to fix a bug. Having two completion mechanisms
    isn't so nice.
    
    As @thufschmitt also pointed out, it was a bummer to go from `FlakeRef`
    to `std::string` when collecting flake refs. Now it is `FlakeRefs`
    again.
    
    The underlying issue that sought to work around was that completion of
    arguments not at the end can still benefit from the information from
    latter arguments.
    
    To fix this better, we rip out that change and simply defer all
    completion processing until after all the (regular, already-complete)
    arguments have been passed.
    
    In addition, I noticed the original completion logic used some global
    variables. I do not like global variables, because even if they save
    lines of code, they also obfuscate the architecture of the code.
    
    I got rid of them  moved them to a new `RootArgs` class, which now has
    `parseCmdline` instead of `Args`. The idea is that we have many argument
    parsers from subcommands and what-not, but only one root args that owns
    the other per actual parsing invocation. The state that was global is
    now part of the root args instead.
    
    This did, admittedly, add a bunch of new code. And I do feel bad about
    that. So I went and added a lot of API docs to try to at least make the
    current state of things clear to the next person.
    
    --
    
    This is needed for RFC 134 (tracking issue NixOS#7868). It was very hard to
    modularize `Installable` parsing when there were two completion
    arguments. I wouldn't go as far as to say it is *easy* now, but at least
    it is less hard (and the completions test finally passed).
    
    Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
    Ericson2314 and fricklerhandwerk committed Oct 16, 2023
    Configuration menu
    Copy the full SHA
    d121326 View commit details
    Browse the repository at this point in the history