Skip to content

Commit

Permalink
nixd/Server/configuration: use .nixd.json for `workspace/configurat…
Browse files Browse the repository at this point in the history 
…ion` defaults (#261)

When `nixd` in `workspace/configuration` is unset in VSCode, we get
`[null]`. This causes the JSON configuration to be used (not
overwritten). However, in Emacs, we get `[{}]` instead, so the JSON
configuration is overwritten with the default one. By using the JSON
configuration as the defaults for `workspace/configuration`, we get the
same behaviour when `workspace/configuration` is unset in VSCode and
Emacs. The JSON configuration is no longer ignored when
`workspace/configuration` is set, but it never takes priority.

An additional advantage of this change is that when
`workspace/configuration` is unset (or set to `null`) while the language
server is running, we fall back to the JSON configuration instead of
keeping the old deleted `workspace/configuration`.
  • Loading branch information
@jonathanjameswatson
jonathanjameswatson authored Oct 7, 2023
2 parents e8f144c + 122d2c2 commit c7cd061
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 19 deletions.
45 changes: 42 additions & 3 deletions lspserver/include/lspserver/LSPBinder.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,33 @@
#include <llvm/ADT/StringMap.h>
#include <llvm/Support/JSON.h>

#include <type_traits>

namespace lspserver {

namespace detail {

template <typename T>
llvm::Expected<T> parseParam(const llvm::json::Value &Raw,
llvm::StringRef PayloadName,
llvm::StringRef PayloadKind) {
typename std::enable_if_t<std::is_default_constructible_v<T>, T>
valueOrUninitialized(const std::optional<T> &OptionalDefault) {
T Result;
if (OptionalDefault) {
Result = OptionalDefault.value();
}
return Result;
}

template <typename T>
typename std::enable_if_t<!std::is_default_constructible_v<T>, T>
valueOrUninitialized(const std::optional<T> &OptionalDefault) {
return OptionalDefault.value();
}

template <typename T>
llvm::Expected<T> parseParamWithOptionalDefault(
const llvm::json::Value &Raw, llvm::StringRef PayloadName,
llvm::StringRef PayloadKind, std::optional<T> OptionalDefault = {}) {
T Result = valueOrUninitialized(OptionalDefault);
llvm::json::Path::Root Root;
if (!fromJSON(Raw, Result, Root)) {
elog("Failed to decode {0} {1}: {2}", PayloadName, PayloadKind,
Expand All @@ -31,6 +51,25 @@ llvm::Expected<T> parseParam(const llvm::json::Value &Raw,
}
return Result;
}

} // namespace detail

template <typename T>
typename std::enable_if_t<std::is_default_constructible_v<T>, llvm::Expected<T>>
parseParam(const llvm::json::Value &Raw, llvm::StringRef PayloadName,
llvm::StringRef PayloadKind) {
return detail::parseParamWithOptionalDefault<T>(Raw, PayloadName,
PayloadKind);
}

template <typename T>
llvm::Expected<T>
parseParamWithDefault(const llvm::json::Value &Raw, llvm::StringRef PayloadName,
llvm::StringRef PayloadKind, T Default) {
return detail::parseParamWithOptionalDefault<T>(Raw, PayloadName, PayloadKind,
Default);
}

struct HandlerRegistry {
using JSON = llvm::json::Value;
template <typename HandlerT>
Expand Down
19 changes: 15 additions & 4 deletions nixd/include/nixd/Server/Controller.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@

namespace nixd {

struct OptionalValue {
std::optional<llvm::json::Value> Value;
};

bool fromJSON(const llvm::json::Value &E, OptionalValue &R, llvm::json::Path P);

/// The server instance, nix-related language features goes here
class Controller : public lspserver::LSPServer {
public:
Expand Down Expand Up @@ -122,6 +128,10 @@ class Controller : public lspserver::LSPServer {

std::mutex ConfigLock;

// When the server starts, DefaultConfig is set to the
// parsed contents of .nixd.json if it exists. This is used
// as the default when parsing workspace/configuration.
configuration::TopLevel DefaultConfig;
configuration::TopLevel Config; // GUARDED_BY(ConfigLock)

std::shared_ptr<const std::string> getDraft(lspserver::PathRef File) const;
Expand All @@ -135,7 +145,7 @@ class Controller : public lspserver::LSPServer {
PublishDiagnostic;

llvm::unique_function<void(const lspserver::ConfigurationParams &,
lspserver::Callback<configuration::TopLevel>)>
lspserver::Callback<OptionalValue>)>
WorkspaceConfiguration;

std::mutex DiagStatusLock;
Expand Down Expand Up @@ -255,9 +265,10 @@ class Controller : public lspserver::LSPServer {
static llvm::Expected<configuration::TopLevel>
parseConfig(llvm::StringRef JSON);

/// Try to update the server config from json encoded file \p File
/// Won't touch config field if exceptions encountered
void readJSONConfig(lspserver::PathRef File = ".nixd.json") noexcept;
/// Try to update the default config from json encoded file \p File
/// Won't touch default config field if exceptions encountered
/// Returns true if the default config is set and false otherwise.
bool readJSONConfigToDefault(lspserver::PathRef File = ".nixd.json") noexcept;

void onWorkspaceDidChangeConfiguration(
const lspserver::DidChangeConfigurationParams &) {
Expand Down
46 changes: 34 additions & 12 deletions nixd/lib/Server/Controller.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

#include "lspserver/Connection.h"
#include "lspserver/DraftStore.h"
#include "lspserver/LSPBinder.h"
#include "lspserver/Logger.h"
#include "lspserver/Path.h"
#include "lspserver/Protocol.h"
Expand Down Expand Up @@ -204,9 +205,15 @@ void Controller::fetchConfig() {
lspserver::ConfigurationParams{
std::vector<lspserver::ConfigurationItem>{
lspserver::ConfigurationItem{.section = "nixd"}}},
[this](llvm::Expected<configuration::TopLevel> Response) {
[this](llvm::Expected<OptionalValue> Response) {
if (Response) {
updateConfig(std::move(Response.get()));
llvm::Expected<configuration::TopLevel> ResponseConfig =
lspserver::parseParamWithDefault<configuration::TopLevel>(
Response.get().Value.value(), "workspace/configuration",
"reply", DefaultConfig);
if (ResponseConfig) {
updateConfig(std::move(ResponseConfig.get()));
}
}
});
}
Expand All @@ -226,20 +233,23 @@ Controller::parseConfig(llvm::StringRef JSON) {
return lspserver::error("value cannot be converted to internal config type");
}

void Controller::readJSONConfig(lspserver::PathRef File) noexcept {
bool Controller::readJSONConfigToDefault(lspserver::PathRef File) noexcept {
try {
std::string ConfigStr;
std::ostringstream SS;
std::ifstream In(File.str(), std::ios::in);
SS << In.rdbuf();

if (auto NewConfig = parseConfig(SS.str()))
updateConfig(std::move(NewConfig.get()));
else {
throw nix::Error("configuration cannot be parsed");
if (auto NewConfig = parseConfig(SS.str())) {
DefaultConfig = std::move(NewConfig.get());
return true;
}

throw nix::Error(".nixd.json configuration cannot be parsed");
} catch (std::exception &E) {
return false;
} catch (...) {
return false;
}
}

Expand All @@ -257,6 +267,16 @@ Controller::Controller(std::unique_ptr<lspserver::InboundPort> In,
: LSPServer(std::move(In), std::move(Out)), WaitWorker(WaitWorker),
ASTMgr(Pool) {

// JSON Config, run before initialize
if (readJSONConfigToDefault()) {
configuration::TopLevel JSONConfigCopy = DefaultConfig;
updateConfig(std::move(JSONConfigCopy));
};

WorkspaceConfiguration =
mkOutMethod<lspserver::ConfigurationParams, OptionalValue>(
"workspace/configuration", nullptr);

// Life Cycle
Registry.addMethod("initialize", this, &Controller::onInitialize);
Registry.addNotification("initialized", this, &Controller::onInitialized);
Expand Down Expand Up @@ -293,17 +313,12 @@ Controller::Controller(std::unique_ptr<lspserver::InboundPort> In,
// Workspace
Registry.addNotification("workspace/didChangeConfiguration", this,
&Controller::onWorkspaceDidChangeConfiguration);
WorkspaceConfiguration =
mkOutMethod<lspserver::ConfigurationParams, configuration::TopLevel>(
"workspace/configuration");

/// IPC
Registry.addNotification("nixd/ipc/diagnostic", this,
&Controller::onEvalDiagnostic);

Registry.addNotification("nixd/ipc/finished", this, &Controller::onFinished);

readJSONConfig();
}

//-----------------------------------------------------------------------------/
Expand Down Expand Up @@ -790,4 +805,11 @@ void Controller::onFormat(
};
boost::asio::post(Pool, std::move(Task));
}

bool fromJSON(const llvm::json::Value &E, OptionalValue &R,
llvm::json::Path P) {
R.Value = E;
return true;
}

} // namespace nixd

0 comments on commit c7cd061

Please sign in to comment.