From 67f07e05df80d481641a4a0a24d700e128fe8fd6 Mon Sep 17 00:00:00 2001 From: Ian Chamberlain Date: Sat, 17 Aug 2024 20:09:47 -0400 Subject: [PATCH] repl: tab-complete quoted attribute names Attribute names containing special characters like @ or . need to be quoted, so we need to do our own tokenization of the command line for completion, and quote the attribute names when we provide the completion. Fixes: https://git.lix.systems/lix-project/lix/issues/450 Change-Id: I55a30dd272880c89445d9ded49b3f2c90cb19326 --- doc/manual/change-authors.yml | 4 + .../rl-next/repl-complete-quoted-attrs.md | 10 ++ lix/libcmd/repl-interacter.cc | 141 ++++++++++++------ lix/libcmd/repl.cc | 24 ++- 4 files changed, 129 insertions(+), 50 deletions(-) create mode 100644 doc/manual/rl-next/repl-complete-quoted-attrs.md diff --git a/doc/manual/change-authors.yml b/doc/manual/change-authors.yml index 348dfd06f..52109f2d5 100644 --- a/doc/manual/change-authors.yml +++ b/doc/manual/change-authors.yml @@ -70,6 +70,10 @@ horrors: iFreilicht: github: iFreilicht +ian-h-chamberlain: + forgejo: ian-h-chamberlain + github: ian-h-chamberlain + isabelroses: forgejo: isabelroses github: isabelroses diff --git a/doc/manual/rl-next/repl-complete-quoted-attrs.md b/doc/manual/rl-next/repl-complete-quoted-attrs.md new file mode 100644 index 000000000..ff358ac0f --- /dev/null +++ b/doc/manual/rl-next/repl-complete-quoted-attrs.md @@ -0,0 +1,10 @@ +--- +synopsis: "`nix repl` correctly tab-completes attribute names that require quotes" +cls: [1783] +credits: [ian-h-chamberlain] +category: Improvements +--- + +The REPL (`nix repl`) now includes quotes as part of attribute names while completing with ``, +if necessary. For example, attribute names like `"hello@example.com"` or `"hello world"` would +be suggested without quotes, resulting in invalid syntax. diff --git a/lix/libcmd/repl-interacter.cc b/lix/libcmd/repl-interacter.cc index a29d89436..677229ff6 100644 --- a/lix/libcmd/repl-interacter.cc +++ b/lix/libcmd/repl-interacter.cc @@ -18,6 +18,7 @@ extern "C" { } #include "lix/libutil/finally.hh" +#include "lix/libutil/strings.hh" #include "lix/libcmd/repl-interacter.hh" namespace nix { @@ -30,52 +31,14 @@ void sigintHandler(int signo) { g_signal_received = signo; } -}; static detail::ReplCompleterMixin * curRepl; // ugly -static char * completionCallback(char * s, int * match) +/** + * @return a null-terminated list of completions as expected by `el_print_columns` + */ +char ** copyCompletions(const StringSet& possible) { - auto possible = curRepl->completePrefix(s); - if (possible.size() == 1) { - *match = 1; - auto * res = strdup(possible.begin()->c_str() + strlen(s)); - if (!res) - throw Error("allocation failure"); - return res; - } else if (possible.size() > 1) { - auto checkAllHaveSameAt = [&](size_t pos) { - auto & first = *possible.begin(); - for (auto & p : possible) { - if (p.size() <= pos || p[pos] != first[pos]) - return false; - } - return true; - }; - size_t start = strlen(s); - size_t len = 0; - while (checkAllHaveSameAt(start + len)) - ++len; - if (len > 0) { - *match = 1; - auto * res = strdup(std::string(*possible.begin(), start, len).c_str()); - if (!res) - throw Error("allocation failure"); - return res; - } - } - - *match = 0; - return nullptr; -} - -static int listPossibleCallback(char * s, char *** avp) -{ - auto possible = curRepl->completePrefix(s); - - if (possible.size() > (INT_MAX / sizeof(char *))) - throw Error("too many completions"); - int ac = 0; char ** vp = nullptr; @@ -90,15 +53,95 @@ static int listPossibleCallback(char * s, char *** avp) } return p; }; - vp = check(static_cast(malloc(possible.size() * sizeof(char *)))); - for (auto & p : possible) vp[ac++] = check(strdup(p.c_str())); - *avp = vp; + return vp; +} - return ac; +// Instead of using the readline-provided prefix, do our own tokenization +// to avoid the default behavior of treating dots/quotes as word boundaries. +// See the definition of SEPS for what it treats as a boundary: +// https://github.com/troglobit/editline/blob/caf4b3c0ce3b0785791198b11de6f3134e9f05d8/src/editline.c +std::string getLastTokenBeforeCursor() +{ + std::string_view line{rl_line_buffer, static_cast(rl_point)}; + + auto tokens = tokenizeString>( + line, + // Same as editline's SEPS, except for double and single quotes: + "#$&()*:;<=>?[\\]^`{|}~\n\t " + ); + + if (tokens.empty()) { + return ""; + } + + return tokens.back(); +} + +// Sometimes inserting text or listing possible completions has a side effect +// of hiding the text after the cursor (even though it remains in the buffer). +// This helper just refreshes the display while keeping the cursor in place. +// +// Inserting text also sometimes moves the whole buffer down one line, usually +// if the cursor is inside a quoted attr name. I'm not sure why (vs unquoted) +// but it still seems to work pretty well and is just a visual artifact. +el_status_t redisplay() +{ + int cursorPos = rl_point; + rl_refresh_line(0, 0); + rl_point = cursorPos; + return (rl_point == rl_end) ? CSstay : CSmove; +} + +}; + + +static el_status_t doCompletion() { + auto s = getLastTokenBeforeCursor(); + auto possible = curRepl->completePrefix(s); + + if (possible.empty()) { + return el_ring_bell(); + } + + if (possible.size() == 1) { + const auto completion = *possible.cbegin(); + if (completion.size() > s.size()) { + rl_insert_text(completion.c_str() + s.size()); + return redisplay(); + } + + return el_ring_bell(); + } + + auto checkAllHaveSameAt = [&](size_t pos) { + auto & first = *possible.begin(); + for (auto & p : possible) { + if (p.size() <= pos || p[pos] != first[pos]) { + return false; + } + } + return true; + }; + + size_t start = s.size(); + size_t len = 0; + while (checkAllHaveSameAt(start + len)) { + ++len; + } + if (len > 0) { + auto commonPrefix = possible.begin()->substr(start, len); + rl_insert_text(commonPrefix.c_str()); + el_ring_bell(); + return redisplay(); + } + + char** columns = copyCompletions(possible); + el_print_columns(possible.size(), columns); + return redisplay(); } ReadlineLikeInteracter::Guard ReadlineLikeInteracter::init(detail::ReplCompleterMixin * repl) @@ -115,8 +158,10 @@ ReadlineLikeInteracter::Guard ReadlineLikeInteracter::init(detail::ReplCompleter auto oldRepl = curRepl; curRepl = repl; Guard restoreRepl([oldRepl] { curRepl = oldRepl; }); - rl_set_complete_func(completionCallback); - rl_set_list_possib_func(listPossibleCallback); + // editline does its own escaping of completions, so we rebind tab + // to our own completion function to skip that and do nix escaping + // instead of shell escaping. + el_bind_key(CTL('I'), doCompletion); return restoreRepl; } diff --git a/lix/libcmd/repl.cc b/lix/libcmd/repl.cc index 56f9387ec..d1534a3d8 100644 --- a/lix/libcmd/repl.cc +++ b/lix/libcmd/repl.cc @@ -12,6 +12,7 @@ #include "lix/libutil/ansicolor.hh" #include "lix/libmain/shared.hh" +#include "lix/libutil/escape-string.hh" #include "lix/libexpr/eval.hh" #include "lix/libexpr/eval-cache.hh" #include "lix/libexpr/eval-inline.hh" @@ -350,7 +351,7 @@ ReplExitStatus NixRepl::mainLoop() } } -StringSet NixRepl::completePrefix(const std::string & prefix) +StringSet NixRepl::completePrefix(const std::string &prefix) { StringSet completions; @@ -415,6 +416,22 @@ StringSet NixRepl::completePrefix(const std::string & prefix) i++; } } else { + // To handle cases like `foo."bar.`, walk back the cursor + // to the previous dot if there are an odd number of quotes. + auto quoteCount = + std::count_if(cur.begin(), cur.begin() + dot, [](char c) { return c == '"'; }); + if (quoteCount % 2 != 0) { + // Find the last quote before the dot + auto prevQuote = cur.rfind('"', dot - 1); + if (prevQuote != std::string::npos) { + // And the previous dot prior to that quote + auto prevDot = cur.rfind('.', prevQuote); + if (prevDot != std::string::npos) { + dot = prevDot; + } + } + } + /* Temporarily disable the debugger, to avoid re-entering readline. */ auto debug = std::move(evaluator.debug); Finally restoreDebug([&]() { evaluator.debug = std::move(debug); }); @@ -431,7 +448,10 @@ StringSet NixRepl::completePrefix(const std::string & prefix) state.forceAttrs(v, noPos, "while evaluating an attrset for the purpose of completion (this error should not be displayed; file an issue?)"); for (auto & i : *v.attrs) { - std::string_view name = evaluator.symbols[i.name]; + std::ostringstream output; + printAttributeName(output, evaluator.symbols[i.name]); + std::string name = output.str(); + if (name.substr(0, cur2.size()) != cur2) continue; completions.insert(concatStrings(prev, expr, ".", name)); }