From f6158ea53b90a60899ee8171c04dc1f978fb8723 Mon Sep 17 00:00:00 2001 From: Jade Lovelace Date: Mon, 26 Feb 2024 00:30:51 -0800 Subject: [PATCH 1/4] finally.hh: include works by itself; mark as nodiscard --- src/libutil/finally.hh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libutil/finally.hh b/src/libutil/finally.hh index db654301f..4cae20a36 100644 --- a/src/libutil/finally.hh +++ b/src/libutil/finally.hh @@ -1,11 +1,13 @@ #pragma once ///@file +#include + /** * A trivial class to run a function at the end of a scope. */ template -class Finally +class [[nodiscard("Finally values must be used")]] Finally { private: Fn fun; From 76aced691552e193e0225af40f8acf484cfeaefe Mon Sep 17 00:00:00 2001 From: Jade Lovelace Date: Mon, 26 Feb 2024 01:21:54 -0800 Subject: [PATCH 2/4] finally.hh: delete copy constructor which is a bad idea --- src/libutil/finally.hh | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/libutil/finally.hh b/src/libutil/finally.hh index 4cae20a36..f9f0195a1 100644 --- a/src/libutil/finally.hh +++ b/src/libutil/finally.hh @@ -11,8 +11,15 @@ class [[nodiscard("Finally values must be used")]] Finally { private: Fn fun; + bool movedFrom = false; public: Finally(Fn fun) : fun(std::move(fun)) { } - ~Finally() { fun(); } + // Copying Finallys is definitely not a good idea and will cause them to be + // called twice. + Finally(Finally &other) = delete; + Finally(Finally &&other) : fun(std::move(other.fun)) { + other.movedFrom = true; + } + ~Finally() { if (!movedFrom) fun(); } }; From 70a6ce139bd39f915a6c2c499d741e2c27557dc0 Mon Sep 17 00:00:00 2001 From: Jade Lovelace Date: Mon, 26 Feb 2024 00:34:11 -0800 Subject: [PATCH 3/4] refactor: move readline stuff into its own file This is in direct preparation for an automation mode of nix repl. --- src/libcmd/repl-interacter.cc | 175 ++++++++++++++++++++++++++++++++++ src/libcmd/repl-interacter.hh | 48 ++++++++++ src/libcmd/repl.cc | 175 ++-------------------------------- src/libcmd/repl.hh | 5 - 4 files changed, 232 insertions(+), 171 deletions(-) create mode 100644 src/libcmd/repl-interacter.cc create mode 100644 src/libcmd/repl-interacter.hh diff --git a/src/libcmd/repl-interacter.cc b/src/libcmd/repl-interacter.cc new file mode 100644 index 000000000..9aa1f7bb9 --- /dev/null +++ b/src/libcmd/repl-interacter.cc @@ -0,0 +1,175 @@ +#include "file-system.hh" +#include "libcmd/repl.hh" +#include + +#ifdef USE_READLINE +#include +#include +#else +// editline < 1.15.2 don't wrap their API for C++ usage +// (added in https://github.com/troglobit/editline/commit/91398ceb3427b730995357e9d120539fb9bb7461). +// This results in linker errors due to to name-mangling of editline C symbols. +// For compatibility with these versions, we wrap the API here +// (wrapping multiple times on newer versions is no problem). +extern "C" { +#include +} +#endif + +#include "signals.hh" +#include "finally.hh" +#include "repl-interacter.hh" + +namespace nix { + +namespace { +// Used to communicate to NixRepl::getLine whether a signal occurred in ::readline. +volatile sig_atomic_t g_signal_received = 0; + +void sigintHandler(int signo) +{ + g_signal_received = signo; +} +}; + +static detail::ReplCompleterMixin * curRepl; // ugly + +static char * completionCallback(char * s, int * match) +{ + 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; + + auto check = [&](auto * p) { + if (!p) { + if (vp) { + while (--ac >= 0) + free(vp[ac]); + free(vp); + } + throw Error("allocation failure"); + } + return p; + }; + + vp = check((char **) malloc(possible.size() * sizeof(char *))); + + for (auto & p : possible) + vp[ac++] = check(strdup(p.c_str())); + + *avp = vp; + + return ac; +} + +ReadlineLikeInteracter::Guard ReadlineLikeInteracter::init(detail::ReplCompleterMixin * repl) +{ + // Allow nix-repl specific settings in .inputrc + rl_readline_name = "nix-repl"; + try { + createDirs(dirOf(historyFile)); + } catch (SystemError & e) { + logWarning(e.info()); + } +#ifndef USE_READLINE + el_hist_size = 1000; +#endif + read_history(historyFile.c_str()); + auto oldRepl = curRepl; + curRepl = repl; + Guard restoreRepl([oldRepl] { curRepl = oldRepl; }); +#ifndef USE_READLINE + rl_set_complete_func(completionCallback); + rl_set_list_possib_func(listPossibleCallback); +#endif + return restoreRepl; +} + +bool ReadlineLikeInteracter::getLine(std::string & input, const std::string & prompt) +{ + struct sigaction act, old; + sigset_t savedSignalMask, set; + + auto setupSignals = [&]() { + act.sa_handler = sigintHandler; + sigfillset(&act.sa_mask); + act.sa_flags = 0; + if (sigaction(SIGINT, &act, &old)) + throw SysError("installing handler for SIGINT"); + + sigemptyset(&set); + sigaddset(&set, SIGINT); + if (sigprocmask(SIG_UNBLOCK, &set, &savedSignalMask)) + throw SysError("unblocking SIGINT"); + }; + auto restoreSignals = [&]() { + if (sigprocmask(SIG_SETMASK, &savedSignalMask, nullptr)) + throw SysError("restoring signals"); + + if (sigaction(SIGINT, &old, 0)) + throw SysError("restoring handler for SIGINT"); + }; + + setupSignals(); + char * s = readline(prompt.c_str()); + Finally doFree([&]() { free(s); }); + restoreSignals(); + + if (g_signal_received) { + g_signal_received = 0; + input.clear(); + return true; + } + + if (!s) + return false; + input += s; + input += '\n'; + return true; +} + +ReadlineLikeInteracter::~ReadlineLikeInteracter() +{ + write_history(historyFile.c_str()); +} + +}; diff --git a/src/libcmd/repl-interacter.hh b/src/libcmd/repl-interacter.hh new file mode 100644 index 000000000..e549bab36 --- /dev/null +++ b/src/libcmd/repl-interacter.hh @@ -0,0 +1,48 @@ +#pragma once +/// @file + +#include "finally.hh" +#include "types.hh" +#include +#include + +namespace nix { + +namespace detail { +/** Provides the completion hooks for the repl, without exposing its complete + * internals. */ +struct ReplCompleterMixin { + virtual StringSet completePrefix(const std::string & prefix) = 0; +}; +}; + +enum class ReplPromptType { + ReplPrompt, + ContinuationPrompt, +}; + +class ReplInteracter +{ +public: + using Guard = Finally>; + + virtual Guard init(detail::ReplCompleterMixin * repl) = 0; + /** Returns a boolean of whether the interacter got EOF */ + virtual bool getLine(std::string & input, const std::string & prompt) = 0; + virtual ~ReplInteracter(){}; +}; + +class ReadlineLikeInteracter : public virtual ReplInteracter +{ + std::string historyFile; +public: + ReadlineLikeInteracter(std::string historyFile) + : historyFile(historyFile) + { + } + virtual Guard init(detail::ReplCompleterMixin * repl) override; + virtual bool getLine(std::string & input, const std::string & prompt) override; + virtual ~ReadlineLikeInteracter() override; +}; + +}; diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc index 8b83608fa..8af3c5ff3 100644 --- a/src/libcmd/repl.cc +++ b/src/libcmd/repl.cc @@ -3,32 +3,17 @@ #include #include -#include - -#ifdef USE_READLINE -#include -#include -#else -// editline < 1.15.2 don't wrap their API for C++ usage -// (added in https://github.com/troglobit/editline/commit/91398ceb3427b730995357e9d120539fb9bb7461). -// This results in linker errors due to to name-mangling of editline C symbols. -// For compatibility with these versions, we wrap the API here -// (wrapping multiple times on newer versions is no problem). -extern "C" { -#include -} -#endif - +#include "libcmd/repl-interacter.hh" #include "repl.hh" #include "ansicolor.hh" -#include "signals.hh" #include "shared.hh" #include "eval.hh" #include "eval-cache.hh" #include "eval-inline.hh" #include "eval-settings.hh" #include "attr-path.hh" +#include "signals.hh" #include "store-api.hh" #include "log-store.hh" #include "common-eval-args.hh" @@ -38,7 +23,6 @@ extern "C" { #include "flake/flake.hh" #include "flake/lockfile.hh" #include "users.hh" -#include "terminal.hh" #include "editor-for.hh" #include "finally.hh" #include "markdown.hh" @@ -75,6 +59,7 @@ enum class ProcessLineResult { struct NixRepl : AbstractNixRepl + , detail::ReplCompleterMixin #if HAVE_BOEHMGC , gc #endif @@ -90,17 +75,16 @@ struct NixRepl int displ; StringSet varNames; - const Path historyFile; + std::unique_ptr interacter; NixRepl(const SearchPath & searchPath, nix::ref store,ref state, std::function getValues); - virtual ~NixRepl(); + virtual ~NixRepl() = default; ReplExitStatus mainLoop() override; void initEnv() override; - StringSet completePrefix(const std::string & prefix); - bool getLine(std::string & input, const std::string & prompt); + virtual StringSet completePrefix(const std::string & prefix) override; StorePath getDerivationPath(Value & v); ProcessLineResult processLine(std::string line); @@ -143,16 +127,10 @@ NixRepl::NixRepl(const SearchPath & searchPath, nix::ref store, refstaticBaseEnv.get())) - , historyFile(getDataDir() + "/nix/repl-history") + , interacter(make_unique(getDataDir() + "/nix/repl-history")) { } - -NixRepl::~NixRepl() -{ - write_history(historyFile.c_str()); -} - void runNix(Path program, const Strings & args, const std::optional & input = {}) { @@ -169,79 +147,6 @@ void runNix(Path program, const Strings & args, return; } -static NixRepl * curRepl; // ugly - -static char * completionCallback(char * s, int *match) { - 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; - - auto check = [&](auto *p) { - if (!p) { - if (vp) { - while (--ac >= 0) - free(vp[ac]); - free(vp); - } - throw Error("allocation failure"); - } - return p; - }; - - vp = check((char **)malloc(possible.size() * sizeof(char*))); - - for (auto & p : possible) - vp[ac++] = check(strdup(p.c_str())); - - *avp = vp; - - return ac; -} - -namespace { - // Used to communicate to NixRepl::getLine whether a signal occurred in ::readline. - volatile sig_atomic_t g_signal_received = 0; - - void sigintHandler(int signo) { - g_signal_received = signo; - } -} - static std::ostream & showDebugTrace(std::ostream & out, const PosTable & positions, const DebugTrace & dt) { if (dt.isError) @@ -281,24 +186,7 @@ ReplExitStatus NixRepl::mainLoop() loadFiles(); - // Allow nix-repl specific settings in .inputrc - rl_readline_name = "nix-repl"; - try { - createDirs(dirOf(historyFile)); - } catch (SystemError & e) { - logWarning(e.info()); - } -#ifndef USE_READLINE - el_hist_size = 1000; -#endif - read_history(historyFile.c_str()); - auto oldRepl = curRepl; - curRepl = this; - Finally restoreRepl([&] { curRepl = oldRepl; }); -#ifndef USE_READLINE - rl_set_complete_func(completionCallback); - rl_set_list_possib_func(listPossibleCallback); -#endif + auto _guard = interacter->init(static_cast(this)); std::string input; @@ -307,7 +195,7 @@ ReplExitStatus NixRepl::mainLoop() logger->pause(); // When continuing input from previous lines, don't print a prompt, just align to the same // number of chars as the prompt. - if (!getLine(input, input.empty() ? "nix-repl> " : " ")) { + if (!interacter->getLine(input, input.empty() ? "nix-repl> " : " ")) { // Ctrl-D should exit the debugger. state->debugStop = false; logger->cout(""); @@ -356,51 +244,6 @@ ReplExitStatus NixRepl::mainLoop() } } - -bool NixRepl::getLine(std::string & input, const std::string & prompt) -{ - struct sigaction act, old; - sigset_t savedSignalMask, set; - - auto setupSignals = [&]() { - act.sa_handler = sigintHandler; - sigfillset(&act.sa_mask); - act.sa_flags = 0; - if (sigaction(SIGINT, &act, &old)) - throw SysError("installing handler for SIGINT"); - - sigemptyset(&set); - sigaddset(&set, SIGINT); - if (sigprocmask(SIG_UNBLOCK, &set, &savedSignalMask)) - throw SysError("unblocking SIGINT"); - }; - auto restoreSignals = [&]() { - if (sigprocmask(SIG_SETMASK, &savedSignalMask, nullptr)) - throw SysError("restoring signals"); - - if (sigaction(SIGINT, &old, 0)) - throw SysError("restoring handler for SIGINT"); - }; - - setupSignals(); - char * s = readline(prompt.c_str()); - Finally doFree([&]() { free(s); }); - restoreSignals(); - - if (g_signal_received) { - g_signal_received = 0; - input.clear(); - return true; - } - - if (!s) - return false; - input += s; - input += '\n'; - return true; -} - - StringSet NixRepl::completePrefix(const std::string & prefix) { StringSet completions; diff --git a/src/libcmd/repl.hh b/src/libcmd/repl.hh index 21aa8bfc7..aac79ec74 100644 --- a/src/libcmd/repl.hh +++ b/src/libcmd/repl.hh @@ -3,11 +3,6 @@ #include "eval.hh" -#if HAVE_BOEHMGC -#define GC_INCLUDE_NEW -#include -#endif - namespace nix { struct AbstractNixRepl From ea31b8a117e0a2e18809fd3921209d106d9040c8 Mon Sep 17 00:00:00 2001 From: Jade Lovelace Date: Mon, 26 Feb 2024 00:43:44 -0800 Subject: [PATCH 4/4] refactor: repl prompts are now the job of the interacter --- src/libcmd/repl-interacter.cc | 19 +++++++++++++++---- src/libcmd/repl-interacter.hh | 4 ++-- src/libcmd/repl.cc | 2 +- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/libcmd/repl-interacter.cc b/src/libcmd/repl-interacter.cc index 9aa1f7bb9..3e34ecdb6 100644 --- a/src/libcmd/repl-interacter.cc +++ b/src/libcmd/repl-interacter.cc @@ -1,5 +1,3 @@ -#include "file-system.hh" -#include "libcmd/repl.hh" #include #ifdef USE_READLINE @@ -19,6 +17,8 @@ extern "C" { #include "signals.hh" #include "finally.hh" #include "repl-interacter.hh" +#include "file-system.hh" +#include "libcmd/repl.hh" namespace nix { @@ -124,7 +124,18 @@ ReadlineLikeInteracter::Guard ReadlineLikeInteracter::init(detail::ReplCompleter return restoreRepl; } -bool ReadlineLikeInteracter::getLine(std::string & input, const std::string & prompt) +static constexpr const char * promptForType(ReplPromptType promptType) +{ + switch (promptType) { + case ReplPromptType::ReplPrompt: + return "nix-repl> "; + case ReplPromptType::ContinuationPrompt: + return " "; + } + assert(false); +} + +bool ReadlineLikeInteracter::getLine(std::string & input, ReplPromptType promptType) { struct sigaction act, old; sigset_t savedSignalMask, set; @@ -150,7 +161,7 @@ bool ReadlineLikeInteracter::getLine(std::string & input, const std::string & pr }; setupSignals(); - char * s = readline(prompt.c_str()); + char * s = readline(promptForType(promptType)); Finally doFree([&]() { free(s); }); restoreSignals(); diff --git a/src/libcmd/repl-interacter.hh b/src/libcmd/repl-interacter.hh index e549bab36..cc70efd07 100644 --- a/src/libcmd/repl-interacter.hh +++ b/src/libcmd/repl-interacter.hh @@ -28,7 +28,7 @@ public: virtual Guard init(detail::ReplCompleterMixin * repl) = 0; /** Returns a boolean of whether the interacter got EOF */ - virtual bool getLine(std::string & input, const std::string & prompt) = 0; + virtual bool getLine(std::string & input, ReplPromptType promptType) = 0; virtual ~ReplInteracter(){}; }; @@ -41,7 +41,7 @@ public: { } virtual Guard init(detail::ReplCompleterMixin * repl) override; - virtual bool getLine(std::string & input, const std::string & prompt) override; + virtual bool getLine(std::string & input, ReplPromptType promptType) override; virtual ~ReadlineLikeInteracter() override; }; diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc index 8af3c5ff3..228d66f5e 100644 --- a/src/libcmd/repl.cc +++ b/src/libcmd/repl.cc @@ -195,7 +195,7 @@ ReplExitStatus NixRepl::mainLoop() logger->pause(); // When continuing input from previous lines, don't print a prompt, just align to the same // number of chars as the prompt. - if (!interacter->getLine(input, input.empty() ? "nix-repl> " : " ")) { + if (!interacter->getLine(input, input.empty() ? ReplPromptType::ReplPrompt : ReplPromptType::ContinuationPrompt)) { // Ctrl-D should exit the debugger. state->debugStop = false; logger->cout("");