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.

Sign up for GitHub

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

Jump to bottom

Improve shell completion of flake inputs #6693

Merged
merged 2 commits into from
Jul 12, 2022
Merged

Improve shell completion of flake inputs #6693

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d6d0e78
Complete flake inputs for all given flakes
ncfavier Jun 19, 2022
711b2e1
Fix flake input completion for `InstallablesCommand`s
ncfavier Jun 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions src/libcmd/command.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,16 @@ struct MixFlakeOptions : virtual Args, EvalCommand
{
flake::LockFlags lockFlags;

std::optional<std::string> needsFlakeInputCompletion = {};

MixFlakeOptions();

virtual std::optional<FlakeRef> getFlakeRefForCompletion()
virtual std::vector<std::string> getFlakesForCompletion()
Copy link
Member

Choose a reason for hiding this comment

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

Any reason why this now return std::strings rather than FlakeRefs? Unless there's a good reason for it, it would be nicer to parse the refs eagerly just to avoid propagating opaque strings in the codebase

Copy link
Member Author

Choose a reason for hiding this comment

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

The reason is to do tilde expansion in a single place, rather than ask each implementation of getFlakesForCompletion to do it.

Copy link
Member

Choose a reason for hiding this comment

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

I see… I'm not entirely convinced that's the best long-term solution, but that'll definitively do it for the time being

Copy link
Member

Choose a reason for hiding this comment

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

We only complete one argument, right? So I think doing it in getFlakesForCompletion is fine?

I changed this in #8131

Copy link
Member

Choose a reason for hiding this comment

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

We only complete one argument, right? So I think doing it in getFlakesForCompletion is fine?

I changed this in #8131

{ return {}; }

void completeFlakeInput(std::string_view prefix);

void completionHook() override;
};

struct SourceExprCommand : virtual Args, MixFlakeOptions
Expand Down Expand Up @@ -119,7 +125,7 @@ struct InstallablesCommand : virtual Args, SourceExprCommand

virtual bool useDefaultInstallables() { return true; }

std::optional<FlakeRef> getFlakeRefForCompletion() override;
std::vector<std::string> getFlakesForCompletion() override;

private:

Expand All @@ -135,9 +141,9 @@ struct InstallableCommand : virtual Args, SourceExprCommand

void prepare() override;

std::optional<FlakeRef> getFlakeRefForCompletion() override
std::vector<std::string> getFlakesForCompletion() override
{
return parseFlakeRefWithFragment(_installable, absPath(".")).first;
return {_installable};
}

private:
Expand Down
46 changes: 25 additions & 21 deletions src/libcmd/installables.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,6 @@

namespace nix {

void completeFlakeInputPath(
ref<EvalState> evalState,
const FlakeRef & flakeRef,
std::string_view prefix)
{
auto flake = flake::getFlake(*evalState, flakeRef, true);
for (auto & input : flake.inputs)
if (hasPrefix(input.first, prefix))
completions->add(input.first);
}

MixFlakeOptions::MixFlakeOptions()
{
auto category = "Common flake-related options";
Expand Down Expand Up @@ -86,8 +75,7 @@ MixFlakeOptions::MixFlakeOptions()
lockFlags.inputUpdates.insert(flake::parseInputPath(s));
}},
.completer = {[&](size_t, std::string_view prefix) {
if (auto flakeRef = getFlakeRefForCompletion())
completeFlakeInputPath(getEvalState(), *flakeRef, prefix);
needsFlakeInputCompletion = {std::string(prefix)};
}}
});

Expand All @@ -103,12 +91,10 @@ MixFlakeOptions::MixFlakeOptions()
parseFlakeRef(flakeRef, absPath("."), true));
}},
.completer = {[&](size_t n, std::string_view prefix) {
if (n == 0) {
if (auto flakeRef = getFlakeRefForCompletion())
completeFlakeInputPath(getEvalState(), *flakeRef, prefix);
} else if (n == 1) {
if (n == 0)
needsFlakeInputCompletion = {std::string(prefix)};
else if (n == 1)
completeFlakeRef(getEvalState()->store, prefix);
}
}}
});

Expand Down Expand Up @@ -139,6 +125,24 @@ MixFlakeOptions::MixFlakeOptions()
});
}

void MixFlakeOptions::completeFlakeInput(std::string_view prefix)
{
auto evalState = getEvalState();
for (auto & flakeRefS : getFlakesForCompletion()) {
auto flakeRef = parseFlakeRefWithFragment(expandTilde(flakeRefS), absPath(".")).first;
auto flake = flake::getFlake(*evalState, flakeRef, true);
for (auto & input : flake.inputs)
if (hasPrefix(input.first, prefix))
completions->add(input.first);
}
}

void MixFlakeOptions::completionHook()
{
if (auto & prefix = needsFlakeInputCompletion)
completeFlakeInput(*prefix);
}

SourceExprCommand::SourceExprCommand(bool supportReadOnlyMode)
{
addFlag({
Expand Down Expand Up @@ -1043,14 +1047,14 @@ void InstallablesCommand::prepare()
installables = parseInstallables(getStore(), _installables);
}

std::optional<FlakeRef> InstallablesCommand::getFlakeRefForCompletion()
std::vector<std::string> InstallablesCommand::getFlakesForCompletion()
{
if (_installables.empty()) {
if (useDefaultInstallables())
return parseFlakeRefWithFragment(".", absPath(".")).first;
return {"."};
return {};
}
return parseFlakeRefWithFragment(_installables.front(), absPath(".")).first;
return _installables;
}

InstallableCommand::InstallableCommand(bool supportReadOnlyMode)
Expand Down
10 changes: 9 additions & 1 deletion src/libutil/args.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ bool Args::processFlag(Strings::iterator & pos, Strings::iterator end)
bool anyCompleted = false;
for (size_t n = 0 ; n < flag.handler.arity; ++n) {
if (pos == end) {
if (flag.handler.arity == ArityAny) break;
if (flag.handler.arity == ArityAny || anyCompleted) break;
throw UsageError("flag '%s' requires %d argument(s)", name, flag.handler.arity);
}
if (auto prefix = needsCompletion(*pos)) {
Expand Down Expand Up @@ -362,6 +362,14 @@ bool MultiCommand::processArgs(const Strings & args, bool finish)
return Args::processArgs(args, finish);
}

void MultiCommand::completionHook()
{
if (command)
return command->second->completionHook();
else
return Args::completionHook();
}

nlohmann::json MultiCommand::toJSON()
{
auto cmds = nlohmann::json::object();
Expand Down
7 changes: 7 additions & 0 deletions src/libutil/args.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,11 @@ protected:
argument (if any) have been processed. */
virtual void initialFlagsProcessed() {}

/* Called after the command line has been processed if we need to generate
completions. Useful for commands that need to know the whole command line
in order to know what completions to generate. */
virtual void completionHook() { }

public:

void addFlag(Flag && flag);
Expand Down Expand Up @@ -223,6 +228,8 @@ public:

bool processArgs(const Strings & args, bool finish) override;

void completionHook() override;

nlohmann::json toJSON() override;
};

Expand Down
4 changes: 2 additions & 2 deletions src/nix/flake.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,9 @@ class FlakeCommand : virtual Args, public MixFlakeOptions
return flake::lockFlake(*getEvalState(), getFlakeRef(), lockFlags);
}

std::optional<FlakeRef> getFlakeRefForCompletion() override
std::vector<std::string> getFlakesForCompletion() override
{
return getFlakeRef();
return {flakeUrl};
}
};

Expand Down
5 changes: 4 additions & 1 deletion src/nix/main.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,10 @@ void mainWrapped(int argc, char * * argv)
if (!completions) throw;
}

if (completions) return;
if (completions) {
args.completionHook();
return;
}

if (args.showVersion) {
printVersion(programName);
Expand Down