📝 docs(plan): implement args parser startup errors
This commit is contained in:
@@ -0,0 +1,813 @@
|
||||
# Args Parser Startup Errors Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Refactor argument parsing into a stateless, strictly validated API and make all startup failures exit predictably without contaminating LSP stdout.
|
||||
|
||||
**Architecture:** `lsp.utils.args_parser` becomes a pure parser that returns `std::expected<ParseResult, std::string>` and owns only CLI syntax plus help rendering. `lsp.cli.launcher` owns logger initialization and maps argument, logger, and server failures to exit codes 2, 1, and 1. Repository callers are migrated away from the removed stdout/stderr selection flags.
|
||||
|
||||
**Tech Stack:** C++23 Modules, `std::expected`, `std::from_chars`, spdlog, CMake/CTest, Python 3 subprocess tests, TypeScript
|
||||
|
||||
---
|
||||
|
||||
## Plan Meta
|
||||
|
||||
- **Plan Group:** args-parser-startup-errors
|
||||
- **Parent Plan:** none
|
||||
- **Verification Scope:** parser unit tests, CLI startup tests, LSP JSON/provider tests, VSCode TypeScript compile
|
||||
- **Verification Gate:** all scoped CTest cases pass, VSCode compiles, and removed flags have no remaining runtime callers
|
||||
- **Execution Constraints:** `karpathy-guidelines`, `.agents`, `AGENT_RULES.md`, test-driven development
|
||||
|
||||
## File Map
|
||||
|
||||
- Modify `lsp-server/src/utils/args_parser.cppm`: stateless parsing types, validation, and help rendering.
|
||||
- Modify `lsp-server/src/cli/launcher.cppm`: logger initialization and staged startup errors.
|
||||
- Create `lsp-server/test/test_args_parser/CMakeLists.txt`: parser unit-test target.
|
||||
- Create `lsp-server/test/test_args_parser/main.cc`: parser unit-test entry point.
|
||||
- Create `lsp-server/test/test_args_parser/test_args_parser.cppm`: parser and help tests.
|
||||
- Create `lsp-server/test/test_cli_startup.py`: executable-level startup regression tests.
|
||||
- Modify `lsp-server/test/CMakeLists.txt`: register the new C++ and Python tests.
|
||||
- Modify `lsp-server/test/run_lsp_json_tests.py`: remove obsolete logger-selection argument.
|
||||
- Modify `lsp-server/test/test_provider/server_json_test.cppm`: remove obsolete stdio alias.
|
||||
- Modify `vscode/src/extension.ts`: stop injecting the removed stderr flag.
|
||||
- Modify `vscode/package.json`: update default server arguments.
|
||||
- Modify `vscode/README.md`: document current arguments.
|
||||
- Modify `vim/README.md`: remove obsolete logger-selection examples while leaving interpreter-path expansion out of scope.
|
||||
|
||||
### Task 1: Drive the parser and launcher refactor with regression tests
|
||||
|
||||
**Files:**
|
||||
|
||||
- Create: `lsp-server/test/test_args_parser/CMakeLists.txt`
|
||||
- Create: `lsp-server/test/test_args_parser/main.cc`
|
||||
- Create: `lsp-server/test/test_args_parser/test_args_parser.cppm`
|
||||
- Create: `lsp-server/test/test_cli_startup.py`
|
||||
- Modify: `lsp-server/test/CMakeLists.txt`
|
||||
- Modify: `lsp-server/src/utils/args_parser.cppm`
|
||||
- Modify: `lsp-server/src/cli/launcher.cppm`
|
||||
|
||||
- [ ] **Step 1: Add the parser unit-test target and failing tests**
|
||||
|
||||
Create `lsp-server/test/test_args_parser/CMakeLists.txt`:
|
||||
|
||||
```cmake
|
||||
cmake_minimum_required(VERSION 4.0)
|
||||
|
||||
project(test_args_parser LANGUAGES CXX)
|
||||
|
||||
set(CMAKE_CXX_STANDARD 23)
|
||||
set(CMAKE_CXX_STANDARD_REQUIRED ON)
|
||||
set(CMAKE_EXPERIMENTAL_CXX_MODULE_DYNDEP 1)
|
||||
|
||||
find_package(spdlog CONFIG REQUIRED)
|
||||
find_package(fmt CONFIG REQUIRED)
|
||||
|
||||
add_executable(test_args_parser main.cc test_args_parser.cppm)
|
||||
if(TARGET std_module)
|
||||
add_dependencies(test_args_parser std_module)
|
||||
endif()
|
||||
|
||||
target_sources(
|
||||
test_args_parser
|
||||
PRIVATE
|
||||
FILE_SET cxx_modules TYPE CXX_MODULES
|
||||
BASE_DIRS ${CMAKE_CURRENT_SOURCE_DIR}
|
||||
${CMAKE_CURRENT_SOURCE_DIR}/../../src
|
||||
FILES test_args_parser.cppm
|
||||
../../src/bridge/spdlog.cppm
|
||||
../../src/utils/args_parser.cppm)
|
||||
|
||||
target_compile_definitions(test_args_parser PRIVATE SPDLOG_HEADER_ONLY
|
||||
FMT_HEADER_ONLY)
|
||||
target_link_libraries(test_args_parser PRIVATE spdlog::spdlog_header_only
|
||||
fmt::fmt-header-only)
|
||||
target_compile_options(
|
||||
test_args_parser
|
||||
PRIVATE -Wall -Wextra -Wpedantic
|
||||
-Wno-import-implementation-partition-unit-in-interface-unit
|
||||
$<$<CONFIG:Debug>:-g -O0>
|
||||
$<$<CONFIG:Release>:-O3>)
|
||||
|
||||
if(BUILD_TESTS)
|
||||
add_test(NAME test_args_parser COMMAND $<TARGET_FILE:test_args_parser>)
|
||||
endif()
|
||||
```
|
||||
|
||||
Create `lsp-server/test/test_args_parser/main.cc`:
|
||||
|
||||
```cpp
|
||||
import lsp.test.args_parser;
|
||||
|
||||
int main()
|
||||
{
|
||||
return Run();
|
||||
}
|
||||
```
|
||||
|
||||
Create `lsp-server/test/test_args_parser/test_args_parser.cppm`:
|
||||
|
||||
```cpp
|
||||
module;
|
||||
|
||||
export module lsp.test.args_parser;
|
||||
|
||||
import spdlog;
|
||||
import std;
|
||||
|
||||
import lsp.utils.args_parser;
|
||||
|
||||
namespace
|
||||
{
|
||||
void Expect(bool condition, std::string_view message)
|
||||
{
|
||||
if (!condition)
|
||||
throw std::runtime_error(std::string(message));
|
||||
}
|
||||
|
||||
auto Parse(std::initializer_list<std::string_view> arguments)
|
||||
{
|
||||
std::vector<std::string> storage{ "tsl-server" };
|
||||
storage.reserve(arguments.size() + 1);
|
||||
for (auto argument : arguments)
|
||||
storage.emplace_back(argument);
|
||||
|
||||
std::vector<char*> argv;
|
||||
argv.reserve(storage.size());
|
||||
for (auto& argument : storage)
|
||||
argv.push_back(argument.data());
|
||||
|
||||
return lsp::utils::ParseArgs(static_cast<int>(argv.size()), argv.data());
|
||||
}
|
||||
|
||||
void ExpectError(std::initializer_list<std::string_view> arguments,
|
||||
std::string_view fragment)
|
||||
{
|
||||
auto result = Parse(arguments);
|
||||
Expect(!result.has_value(), "arguments should be rejected");
|
||||
Expect(result.error().find(fragment) != std::string::npos,
|
||||
"error should identify the invalid argument");
|
||||
}
|
||||
}
|
||||
|
||||
export int Run()
|
||||
{
|
||||
int failures = 0;
|
||||
auto RunCase = [&failures](std::string_view name, auto test) {
|
||||
try
|
||||
{
|
||||
test();
|
||||
std::cout << "[PASS] " << name << '\n';
|
||||
}
|
||||
catch (const std::exception& error)
|
||||
{
|
||||
++failures;
|
||||
std::cout << "[FAIL] " << name << ": " << error.what() << '\n';
|
||||
}
|
||||
};
|
||||
|
||||
RunCase("defaults", [] {
|
||||
auto result = Parse({});
|
||||
Expect(result.has_value(), "default arguments should parse");
|
||||
Expect(result->action == lsp::utils::ParseAction::kRun,
|
||||
"default action should run");
|
||||
Expect(result->config.thread_count == 4, "default thread count should be 4");
|
||||
Expect(result->config.log_level == spdlog::level::info,
|
||||
"default log level should be info");
|
||||
});
|
||||
|
||||
RunCase("valid values and last occurrence", [] {
|
||||
auto result = Parse({ "--threads=8", "--threads=16", "--log=debug",
|
||||
"--log-file=server.log", "--interpreter=/opt/tsl" });
|
||||
Expect(result.has_value(), "valid arguments should parse");
|
||||
Expect(result->config.thread_count == 16, "last thread count should win");
|
||||
Expect(result->config.log_level == spdlog::level::debug,
|
||||
"debug log level should parse");
|
||||
Expect(result->config.log_file == "server.log", "log file should parse");
|
||||
Expect(result->config.interpreter_path == "/opt/tsl",
|
||||
"interpreter path should parse");
|
||||
});
|
||||
|
||||
RunCase("all log levels", [] {
|
||||
const std::array levels{
|
||||
std::pair{ "trace", spdlog::level::trace },
|
||||
std::pair{ "debug", spdlog::level::debug },
|
||||
std::pair{ "info", spdlog::level::info },
|
||||
std::pair{ "warn", spdlog::level::warn },
|
||||
std::pair{ "error", spdlog::level::err },
|
||||
std::pair{ "off", spdlog::level::off },
|
||||
};
|
||||
for (const auto& [name, level] : levels)
|
||||
{
|
||||
const std::string argument = std::string("--log=") + name;
|
||||
auto result = Parse({ argument });
|
||||
Expect(result.has_value(), "known log level should parse");
|
||||
Expect(result->config.log_level == level, "log level should match");
|
||||
}
|
||||
});
|
||||
|
||||
RunCase("help takes precedence", [] {
|
||||
auto result = Parse({ "--unknown", "--help", "--threads=bad" });
|
||||
Expect(result.has_value(), "help should bypass other validation");
|
||||
Expect(result->action == lsp::utils::ParseAction::kShowHelp,
|
||||
"help action should be returned");
|
||||
});
|
||||
|
||||
RunCase("thread validation", [] {
|
||||
Expect(Parse({ "--threads=1" }).has_value(), "one thread should parse");
|
||||
Expect(Parse({ "--threads=256" }).has_value(), "256 threads should parse");
|
||||
for (auto argument : { "--threads=", "--threads=0", "--threads=-1",
|
||||
"--threads=257", "--threads=12junk",
|
||||
"--threads=18446744073709551616" })
|
||||
ExpectError({ argument }, "--threads");
|
||||
});
|
||||
|
||||
RunCase("other invalid values", [] {
|
||||
for (auto argument : { "--log=verbose", "--log-file=", "--interpreter=",
|
||||
"--log-stdout", "--log-stderr", "--use-stdio",
|
||||
"--unknown" })
|
||||
ExpectError({ argument }, "--");
|
||||
});
|
||||
|
||||
RunCase("help formatting", [] {
|
||||
std::ostringstream output;
|
||||
lsp::utils::PrintHelp(output, "tsl-server");
|
||||
const auto text = output.str();
|
||||
Expect(text.contains("Usage: tsl-server [options]\n\nOptions:\n"),
|
||||
"help should contain physical line breaks");
|
||||
Expect(!text.contains("\\n"), "help should not contain escaped newline text");
|
||||
Expect(!text.contains("--log-stdout"), "removed stdout flag should be absent");
|
||||
Expect(!text.contains("--log-stderr"), "removed stderr flag should be absent");
|
||||
Expect(!text.contains("--use-stdio"), "removed stdio alias should be absent");
|
||||
});
|
||||
|
||||
std::cout << "Failures: " << failures << '\n';
|
||||
return failures == 0 ? 0 : 1;
|
||||
}
|
||||
```
|
||||
|
||||
Register the directory in `lsp-server/test/CMakeLists.txt` after
|
||||
`test_scheduler`:
|
||||
|
||||
```cmake
|
||||
if(EXISTS ${CMAKE_CURRENT_LIST_DIR}/test_args_parser/CMakeLists.txt)
|
||||
add_subdirectory(test_args_parser)
|
||||
endif()
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Add failing executable-level startup tests**
|
||||
|
||||
Create `lsp-server/test/test_cli_startup.py`:
|
||||
|
||||
```python
|
||||
#!/usr/bin/env python3
|
||||
|
||||
import argparse
|
||||
import subprocess
|
||||
import tempfile
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
|
||||
|
||||
SERVER: Path
|
||||
|
||||
|
||||
def run_server(*arguments: str) -> subprocess.CompletedProcess[bytes]:
|
||||
return subprocess.run(
|
||||
[str(SERVER), *arguments],
|
||||
input=b"",
|
||||
capture_output=True,
|
||||
check=False,
|
||||
)
|
||||
|
||||
|
||||
class CliStartupTest(unittest.TestCase):
|
||||
def test_help_uses_real_newlines(self) -> None:
|
||||
result = run_server("--help")
|
||||
self.assertEqual(0, result.returncode)
|
||||
self.assertIn(b"\n\nOptions:\n", result.stdout)
|
||||
self.assertNotIn(b"\\n", result.stdout)
|
||||
|
||||
def test_invalid_threads_return_argument_error(self) -> None:
|
||||
for argument in ("--threads=", "--threads=-1", "--threads=12junk"):
|
||||
with self.subTest(argument=argument):
|
||||
result = run_server(argument)
|
||||
self.assertEqual(2, result.returncode)
|
||||
self.assertIn(b"--threads", result.stderr)
|
||||
self.assertNotIn(b"TSL-LSP server starting", result.stderr)
|
||||
|
||||
def test_removed_stdout_flag_is_rejected(self) -> None:
|
||||
result = run_server("--log-stdout")
|
||||
self.assertEqual(2, result.returncode)
|
||||
self.assertEqual(b"", result.stdout)
|
||||
|
||||
def test_default_logger_never_writes_stdout(self) -> None:
|
||||
result = run_server()
|
||||
self.assertEqual(0, result.returncode)
|
||||
self.assertEqual(b"", result.stdout)
|
||||
self.assertIn(b"TSL-LSP server starting", result.stderr)
|
||||
|
||||
def test_log_file_failure_is_controlled(self) -> None:
|
||||
with tempfile.TemporaryDirectory() as temp_dir:
|
||||
path = Path(temp_dir) / "missing" / "server.log"
|
||||
result = run_server(f"--log-file={path}")
|
||||
self.assertEqual(1, result.returncode)
|
||||
self.assertIn(b"Failed to initialize logger", result.stderr)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
parser = argparse.ArgumentParser()
|
||||
parser.add_argument("--server", type=Path, required=True)
|
||||
args, unittest_args = parser.parse_known_args()
|
||||
SERVER = args.server.resolve()
|
||||
unittest.main(argv=[__file__, *unittest_args])
|
||||
```
|
||||
|
||||
Register it next to `test_lsp_json` in `lsp-server/test/CMakeLists.txt`:
|
||||
|
||||
```cmake
|
||||
add_test(NAME test_cli_startup
|
||||
COMMAND ${PYTHON3_EXECUTABLE}
|
||||
${CMAKE_CURRENT_LIST_DIR}/test_cli_startup.py
|
||||
--server $<TARGET_FILE:tsl-server>)
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Run the new tests to verify the old implementation fails**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cmake -S lsp-server -B lsp-server/build/clang-linux/Release
|
||||
cmake --build lsp-server/build/clang-linux/Release --target tsl-server
|
||||
python lsp-server/test/test_cli_startup.py \
|
||||
--server lsp-server/build/clang-linux/Release/src/tsl-server
|
||||
cmake --build lsp-server/build/clang-linux/Release --target test_args_parser
|
||||
```
|
||||
|
||||
Expected: the Python suite fails the escaped-newline and controlled-error assertions.
|
||||
The subsequent parser-test build fails because `ParseArgs`, `ParseAction`, and the
|
||||
stream-based `PrintHelp` do not exist.
|
||||
|
||||
- [ ] **Step 4: Replace the stateful parser with the stateless implementation**
|
||||
|
||||
Replace `lsp-server/src/utils/args_parser.cppm` with:
|
||||
|
||||
```cpp
|
||||
module;
|
||||
|
||||
export module lsp.utils.args_parser;
|
||||
|
||||
import spdlog;
|
||||
import std;
|
||||
|
||||
export namespace lsp::utils
|
||||
{
|
||||
struct ServerConfig
|
||||
{
|
||||
std::size_t thread_count = 4;
|
||||
spdlog::level::level_enum log_level = spdlog::level::info;
|
||||
std::string log_file;
|
||||
std::string interpreter_path;
|
||||
};
|
||||
|
||||
enum class ParseAction
|
||||
{
|
||||
kRun,
|
||||
kShowHelp,
|
||||
};
|
||||
|
||||
struct ParseResult
|
||||
{
|
||||
ParseAction action = ParseAction::kRun;
|
||||
ServerConfig config;
|
||||
};
|
||||
|
||||
std::expected<ParseResult, std::string> ParseArgs(int argc, char* const argv[]);
|
||||
void PrintHelp(std::ostream& output, std::string_view program_name);
|
||||
}
|
||||
|
||||
namespace lsp::utils
|
||||
{
|
||||
namespace
|
||||
{
|
||||
constexpr std::size_t kMinThreadCount = 1;
|
||||
constexpr std::size_t kMaxThreadCount = 256;
|
||||
|
||||
std::optional<spdlog::level::level_enum> ParseLogLevel(std::string_view value)
|
||||
{
|
||||
if (value == "trace")
|
||||
return spdlog::level::trace;
|
||||
if (value == "debug")
|
||||
return spdlog::level::debug;
|
||||
if (value == "info")
|
||||
return spdlog::level::info;
|
||||
if (value == "warn")
|
||||
return spdlog::level::warn;
|
||||
if (value == "error")
|
||||
return spdlog::level::err;
|
||||
if (value == "off")
|
||||
return spdlog::level::off;
|
||||
return std::nullopt;
|
||||
}
|
||||
}
|
||||
|
||||
std::expected<ParseResult, std::string> ParseArgs(int argc, char* const argv[])
|
||||
{
|
||||
for (int i = 1; i < argc; ++i)
|
||||
{
|
||||
if (std::string_view(argv[i]) == "--help")
|
||||
return ParseResult{ .action = ParseAction::kShowHelp };
|
||||
}
|
||||
|
||||
ParseResult result;
|
||||
constexpr std::string_view kLogPrefix = "--log=";
|
||||
constexpr std::string_view kLogFilePrefix = "--log-file=";
|
||||
constexpr std::string_view kThreadsPrefix = "--threads=";
|
||||
constexpr std::string_view kInterpreterPrefix = "--interpreter=";
|
||||
|
||||
for (int i = 1; i < argc; ++i)
|
||||
{
|
||||
const std::string_view argument = argv[i];
|
||||
if (argument.starts_with(kLogPrefix))
|
||||
{
|
||||
const auto value = argument.substr(kLogPrefix.size());
|
||||
auto level = ParseLogLevel(value);
|
||||
if (!level)
|
||||
return std::unexpected("Invalid value for --log: '" +
|
||||
std::string(value) + "'");
|
||||
result.config.log_level = *level;
|
||||
}
|
||||
else if (argument.starts_with(kLogFilePrefix))
|
||||
{
|
||||
const auto value = argument.substr(kLogFilePrefix.size());
|
||||
if (value.empty())
|
||||
return std::unexpected("--log-file requires a non-empty path");
|
||||
result.config.log_file = value;
|
||||
}
|
||||
else if (argument.starts_with(kThreadsPrefix))
|
||||
{
|
||||
const auto value = argument.substr(kThreadsPrefix.size());
|
||||
std::size_t count = 0;
|
||||
const auto [end, error] =
|
||||
std::from_chars(value.data(), value.data() + value.size(), count);
|
||||
if (error != std::errc{} || end != value.data() + value.size() ||
|
||||
count < kMinThreadCount || count > kMaxThreadCount)
|
||||
{
|
||||
return std::unexpected("Invalid value for --threads: '" +
|
||||
std::string(value) + "' (expected 1..256)");
|
||||
}
|
||||
result.config.thread_count = count;
|
||||
}
|
||||
else if (argument.starts_with(kInterpreterPrefix))
|
||||
{
|
||||
const auto value = argument.substr(kInterpreterPrefix.size());
|
||||
if (value.empty())
|
||||
return std::unexpected("--interpreter requires a non-empty path");
|
||||
result.config.interpreter_path = value;
|
||||
}
|
||||
else
|
||||
{
|
||||
return std::unexpected("Unknown argument: " + std::string(argument));
|
||||
}
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
void PrintHelp(std::ostream& output, std::string_view program_name)
|
||||
{
|
||||
output << "Usage: " << program_name << " [options]\n\n"
|
||||
<< "Options:\n"
|
||||
<< " --help Show this help message\n"
|
||||
<< " --log=<level> Set log level (trace, debug, info, warn, error, off)\n"
|
||||
<< " --log-file=<path> Output logs to specified file\n"
|
||||
<< " --threads=<count> Number of worker threads (1-256)\n"
|
||||
<< " --interpreter=<path> Custom interpreter path\n";
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Move logger ownership and staged errors into the launcher**
|
||||
|
||||
Replace `lsp-server/src/cli/launcher.cppm` with:
|
||||
|
||||
```cpp
|
||||
module;
|
||||
|
||||
export module lsp.cli.launcher;
|
||||
|
||||
import spdlog;
|
||||
import std;
|
||||
|
||||
import lsp.core.server;
|
||||
import lsp.utils.args_parser;
|
||||
|
||||
namespace
|
||||
{
|
||||
void SetupLogger(const lsp::utils::ServerConfig& config)
|
||||
{
|
||||
spdlog::set_pattern("%Y-%m-%d %H:%M:%S.%e [%t] [%^%l%$] %v");
|
||||
auto logger = config.log_file.empty()
|
||||
? spdlog::stderr_logger_mt("console_logger")
|
||||
: spdlog::basic_logger_mt("file_logger", config.log_file);
|
||||
logger->set_level(config.log_level);
|
||||
spdlog::set_default_logger(logger);
|
||||
spdlog::set_level(config.log_level);
|
||||
}
|
||||
}
|
||||
|
||||
export int Run(int argc, char* argv[])
|
||||
{
|
||||
auto parsed = lsp::utils::ParseArgs(argc, argv);
|
||||
if (!parsed)
|
||||
{
|
||||
std::cerr << "[TSL-LSP] Argument error: " << parsed.error() << '\n';
|
||||
return 2;
|
||||
}
|
||||
|
||||
if (parsed->action == lsp::utils::ParseAction::kShowHelp)
|
||||
{
|
||||
lsp::utils::PrintHelp(std::cout, argv[0]);
|
||||
return 0;
|
||||
}
|
||||
|
||||
const auto& config = parsed->config;
|
||||
try
|
||||
{
|
||||
SetupLogger(config);
|
||||
}
|
||||
catch (const std::exception& error)
|
||||
{
|
||||
std::cerr << "[TSL-LSP] Failed to initialize logger: " << error.what() << '\n';
|
||||
return 1;
|
||||
}
|
||||
|
||||
try
|
||||
{
|
||||
spdlog::info("TSL-LSP server starting...");
|
||||
lsp::core::LspServer server(config.thread_count, config.interpreter_path);
|
||||
server.Run();
|
||||
}
|
||||
catch (const std::exception& error)
|
||||
{
|
||||
std::cerr << "[TSL-LSP] Server fatal error: " << error.what() << '\n';
|
||||
spdlog::error("Server fatal error: {}", error.what());
|
||||
spdlog::shutdown();
|
||||
return 1;
|
||||
}
|
||||
catch (...)
|
||||
{
|
||||
std::cerr << "[TSL-LSP] Server unknown fatal error\n";
|
||||
spdlog::error("Server unknown fatal error");
|
||||
spdlog::shutdown();
|
||||
return 1;
|
||||
}
|
||||
|
||||
spdlog::info("TSL-LSP server stopped normally");
|
||||
spdlog::shutdown();
|
||||
return 0;
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 6: Format, build, and run the new tests**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
clang-format -i \
|
||||
lsp-server/src/utils/args_parser.cppm \
|
||||
lsp-server/src/cli/launcher.cppm \
|
||||
lsp-server/test/test_args_parser/main.cc \
|
||||
lsp-server/test/test_args_parser/test_args_parser.cppm
|
||||
cmake -S lsp-server -B lsp-server/build/clang-linux/Release
|
||||
cmake --build lsp-server/build/clang-linux/Release \
|
||||
--target tsl-server test_args_parser
|
||||
ctest --test-dir lsp-server/build/clang-linux/Release \
|
||||
-R 'test_args_parser|test_cli_startup' --output-on-failure
|
||||
```
|
||||
|
||||
Expected: both tests pass; malformed arguments return 2, logger creation failure returns
|
||||
1, and default logs do not appear on stdout.
|
||||
|
||||
- [ ] **Step 7: Commit the parser and launcher refactor**
|
||||
|
||||
```bash
|
||||
git add lsp-server/src/utils/args_parser.cppm \
|
||||
lsp-server/src/cli/launcher.cppm \
|
||||
lsp-server/test/CMakeLists.txt \
|
||||
lsp-server/test/test_args_parser \
|
||||
lsp-server/test/test_cli_startup.py
|
||||
git commit -m "refactor(cli): validate startup arguments"
|
||||
```
|
||||
|
||||
### Task 2: Migrate LSP test callers from removed flags
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `lsp-server/test/run_lsp_json_tests.py`
|
||||
- Modify: `lsp-server/test/test_provider/server_json_test.cppm`
|
||||
|
||||
- [ ] **Step 1: Run affected tests to expose obsolete arguments**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
ctest --test-dir lsp-server/build/clang-linux/Release \
|
||||
-R 'test_lsp_json|test_provider' --output-on-failure
|
||||
```
|
||||
|
||||
Expected: `test_lsp_json` fails because `--log-stderr` is unknown, and the provider
|
||||
server subprocess case fails because `--use-stdio` is unknown.
|
||||
|
||||
- [ ] **Step 2: Remove the obsolete arguments from both launch sites**
|
||||
|
||||
Change `lsp-server/test/run_lsp_json_tests.py` to:
|
||||
|
||||
```python
|
||||
proc = subprocess.Popen(
|
||||
[str(server_path), "--log=off"],
|
||||
stdin=subprocess.PIPE,
|
||||
stdout=subprocess.PIPE,
|
||||
stderr=subprocess.PIPE,
|
||||
)
|
||||
```
|
||||
|
||||
Change the command construction in
|
||||
`lsp-server/test/test_provider/server_json_test.cppm` to:
|
||||
|
||||
```cpp
|
||||
std::string command = "\"" + server_path.string() + "\" --log=off < \"" +
|
||||
input_path.string() + "\" > \"" + output_path.string() + "\"";
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Format and rerun the affected tests**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
clang-format -i lsp-server/test/test_provider/server_json_test.cppm
|
||||
cmake --build lsp-server/build/clang-linux/Release --target test_provider
|
||||
ctest --test-dir lsp-server/build/clang-linux/Release \
|
||||
-R 'test_lsp_json|test_provider' --output-on-failure
|
||||
```
|
||||
|
||||
Expected: both tests pass.
|
||||
|
||||
- [ ] **Step 4: Commit the LSP test-call migration**
|
||||
|
||||
```bash
|
||||
git add lsp-server/test/run_lsp_json_tests.py \
|
||||
lsp-server/test/test_provider/server_json_test.cppm
|
||||
git commit -m "test(lsp): remove obsolete logger flags"
|
||||
```
|
||||
|
||||
### Task 3: Update editor integrations and user documentation
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `vscode/src/extension.ts`
|
||||
- Modify: `vscode/package.json`
|
||||
- Modify: `vscode/README.md`
|
||||
- Modify: `vim/README.md`
|
||||
|
||||
- [ ] **Step 1: Confirm editor integrations still reference removed flags**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
rg -n --glob '!node_modules/**' -- \
|
||||
'--log-stdout|--log-stderr|--use-stdio|--log=stderr' vscode vim
|
||||
```
|
||||
|
||||
Expected: matches appear in `vscode/src/extension.ts`, `vscode/package.json`, both
|
||||
editor READMEs, and nowhere else under these directories.
|
||||
|
||||
- [ ] **Step 2: Stop VSCode from injecting the removed flag**
|
||||
|
||||
Change the argument setup in `vscode/src/extension.ts` to:
|
||||
|
||||
```typescript
|
||||
const runArgs = [...serverArguments]
|
||||
const debugArgs = serverArguments.map(arg => arg.startsWith('--log=') ? '--log=trace' : arg)
|
||||
if (!debugArgs.some(arg => arg.startsWith('--log='))) debugArgs.push('--log=trace')
|
||||
```
|
||||
|
||||
Change the `tsl.server.arguments` default in `vscode/package.json` to:
|
||||
|
||||
```json
|
||||
"default": [
|
||||
"--log=info"
|
||||
]
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Update documented examples without changing interpreter-path behavior**
|
||||
|
||||
In `vscode/README.md`, use `["--log=info"]` for both the documented default and the
|
||||
configuration example.
|
||||
|
||||
In `vim/README.md`, use this server argument list and remove the `--log-stderr` table
|
||||
row:
|
||||
|
||||
```json
|
||||
"args": ["--log=info", "--interpreter=~/tsl64"]
|
||||
```
|
||||
|
||||
Do not change `~/tsl64` in this task; home-directory expansion is explicitly deferred
|
||||
by the design.
|
||||
|
||||
- [ ] **Step 4: Compile VSCode and confirm removed flags are gone from runtime callers**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
npm --prefix vscode run compile
|
||||
rg -n --glob '!node_modules/**' -- \
|
||||
'--log-stdout|--log-stderr|--use-stdio|--log=stderr' \
|
||||
vscode/src vscode/package.json vscode/README.md vim/README.md
|
||||
```
|
||||
|
||||
Expected: TypeScript compilation succeeds and the search returns no matches.
|
||||
|
||||
- [ ] **Step 5: Commit editor integration updates**
|
||||
|
||||
```bash
|
||||
git add vscode/src/extension.ts vscode/package.json vscode/README.md vim/README.md
|
||||
git commit -m "refactor(editors): use fixed stderr logging"
|
||||
```
|
||||
|
||||
### Task 4: Run the complete scoped verification gate
|
||||
|
||||
**Files:**
|
||||
|
||||
- Verify only; no planned source changes.
|
||||
|
||||
- [ ] **Step 1: Verify formatting and whitespace**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
clang-format --dry-run --Werror \
|
||||
lsp-server/src/utils/args_parser.cppm \
|
||||
lsp-server/src/cli/launcher.cppm \
|
||||
lsp-server/test/test_args_parser/main.cc \
|
||||
lsp-server/test/test_args_parser/test_args_parser.cppm \
|
||||
lsp-server/test/test_provider/server_json_test.cppm
|
||||
git diff --check
|
||||
```
|
||||
|
||||
Expected: both commands exit 0 with no diagnostics.
|
||||
|
||||
- [ ] **Step 2: Build all affected C++ targets**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
cmake --build lsp-server/build/clang-linux/Release \
|
||||
--target tsl-server test_args_parser test_provider
|
||||
```
|
||||
|
||||
Expected: all three targets build successfully.
|
||||
|
||||
- [ ] **Step 3: Run all scoped CTest cases**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
ctest --test-dir lsp-server/build/clang-linux/Release \
|
||||
-R 'test_args_parser|test_cli_startup|test_lsp_json|test_provider' \
|
||||
--output-on-failure
|
||||
```
|
||||
|
||||
Expected: four tests run and all pass.
|
||||
|
||||
- [ ] **Step 4: Verify the editor build and removed runtime arguments**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
npm --prefix vscode run compile
|
||||
rg -n --glob '!node_modules/**' --glob '!docs/superpowers/**' -- \
|
||||
'--log-stdout|--log-stderr|--use-stdio|--log=stderr' \
|
||||
lsp-server/src lsp-server/test/run_lsp_json_tests.py \
|
||||
lsp-server/test/test_provider vscode/src vscode/package.json \
|
||||
vscode/README.md vim/README.md
|
||||
```
|
||||
|
||||
Expected: TypeScript compilation succeeds. The search may match only negative assertions
|
||||
inside `test_args_parser.cppm`; it must not match a server invocation, editor argument
|
||||
list, help string, or documentation example.
|
||||
|
||||
- [ ] **Step 5: Inspect final changes against the design**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
git status --short
|
||||
git log --oneline -4
|
||||
git diff HEAD~3 -- \
|
||||
lsp-server/src/utils/args_parser.cppm \
|
||||
lsp-server/src/cli/launcher.cppm \
|
||||
lsp-server/test vscode/src/extension.ts vscode/package.json \
|
||||
vscode/README.md vim/README.md
|
||||
```
|
||||
|
||||
Expected: the diff contains only the planned parser, launcher, tests, caller migrations,
|
||||
and documentation updates; unrelated pre-existing worktree changes remain untouched.
|
||||
Reference in New Issue
Block a user