Implement issue 50: Use std::expected return types in skill module
- parse_skill_frontmatter() now returns std::expected<SkillMetadata, std::string> - scan_skills() now returns std::expected<std::vector<SkillMetadata>, std::string> - All throw statements replaced with return std::unexpected() - Filesystem scan errors are now fatal instead of warnings - Updated main.cpp to handle the new return types
This commit is contained in:
+5
-4
@@ -1,9 +1,9 @@
|
||||
#pragma once
|
||||
#include <string>
|
||||
#include <optional>
|
||||
#include <stdexcept>
|
||||
#include <vector>
|
||||
#include <unordered_map>
|
||||
#include <expected>
|
||||
#include "tool.hpp"
|
||||
|
||||
// Represents parsed YAML frontmatter metadata from a SKILL.md file
|
||||
@@ -16,11 +16,12 @@ struct SkillMetadata {
|
||||
};
|
||||
|
||||
// Parse YAML frontmatter from a file and extract content.
|
||||
// Throws std::runtime_error if the file is malformed or name is missing.
|
||||
SkillMetadata parse_skill_frontmatter(const std::string& filepath);
|
||||
// Returns std::unexpected with error message if the file is malformed or name is missing.
|
||||
std::expected<SkillMetadata, std::string> parse_skill_frontmatter(const std::string& filepath);
|
||||
|
||||
// Scans global and local skill directories and returns parsed SkillMetadata for each discovered skill.
|
||||
std::vector<SkillMetadata> scan_skills();
|
||||
// Returns std::unexpected with error message if any skill file is malformed or cannot be accessed.
|
||||
std::expected<std::vector<SkillMetadata>, std::string> scan_skills();
|
||||
|
||||
// Prints discovered paths to stdout
|
||||
void print_discovered_paths(const std::vector<std::string>& paths);
|
||||
|
||||
+13
-9
@@ -265,7 +265,12 @@ int main(int argc, char** argv) {
|
||||
};
|
||||
|
||||
// Scan for skills
|
||||
std::vector<SkillMetadata> all_skills = scan_skills();
|
||||
auto all_skills_result = scan_skills();
|
||||
if (!all_skills_result) {
|
||||
std::cerr << "Fatal: " << all_skills_result.error() << std::endl;
|
||||
return 1;
|
||||
}
|
||||
std::vector<SkillMetadata> all_skills = std::move(all_skills_result.value());
|
||||
|
||||
// Build list of discovered paths for debug output
|
||||
std::vector<std::string> discovered_paths;
|
||||
@@ -298,16 +303,15 @@ int main(int argc, char** argv) {
|
||||
}
|
||||
|
||||
if (debug_frontmatter) {
|
||||
try {
|
||||
auto metadata = parse_skill_frontmatter(debug_frontmatter);
|
||||
std::cout << "name: " << metadata.name << "\n";
|
||||
if (metadata.description.has_value()) {
|
||||
std::cout << "description: " << metadata.description.value() << "\n";
|
||||
}
|
||||
} catch (const std::runtime_error& e) {
|
||||
std::cerr << "Error parsing frontmatter: " << e.what() << "\n";
|
||||
auto metadata = parse_skill_frontmatter(debug_frontmatter);
|
||||
if (!metadata) {
|
||||
std::cerr << "Error parsing frontmatter: " << metadata.error() << "\n";
|
||||
return 1;
|
||||
}
|
||||
std::cout << "name: " << metadata->name << "\n";
|
||||
if (metadata->description.has_value()) {
|
||||
std::cout << "description: " << metadata->description.value() << "\n";
|
||||
}
|
||||
return 0;
|
||||
}
|
||||
|
||||
|
||||
+37
-30
@@ -12,10 +12,10 @@ namespace fs = std::filesystem;
|
||||
|
||||
// ---- Parsing ----
|
||||
|
||||
SkillMetadata parse_skill_frontmatter(const std::string& filepath) {
|
||||
std::expected<SkillMetadata, std::string> parse_skill_frontmatter(const std::string& filepath) {
|
||||
std::ifstream file(filepath);
|
||||
if (!file.is_open()) {
|
||||
throw std::runtime_error("Cannot open file: " + filepath);
|
||||
return std::unexpected("Cannot open file: " + filepath);
|
||||
}
|
||||
|
||||
std::string line;
|
||||
@@ -39,13 +39,13 @@ SkillMetadata parse_skill_frontmatter(const std::string& filepath) {
|
||||
}
|
||||
// If we hit a non-empty line before ---, it's not valid frontmatter
|
||||
if (!line.empty()) {
|
||||
throw std::runtime_error("File does not start with YAML frontmatter (---)");
|
||||
return std::unexpected("File does not start with YAML frontmatter (---)");
|
||||
}
|
||||
} else if (!reading_content) {
|
||||
// Look for closing ---
|
||||
if (line == "---") {
|
||||
if (name.empty()) {
|
||||
throw std::runtime_error("YAML frontmatter is missing required 'name' field");
|
||||
return std::unexpected("YAML frontmatter is missing required 'name' field");
|
||||
}
|
||||
reading_content = true;
|
||||
continue;
|
||||
@@ -88,7 +88,7 @@ SkillMetadata parse_skill_frontmatter(const std::string& filepath) {
|
||||
}
|
||||
|
||||
if (!reading_content) {
|
||||
throw std::runtime_error("YAML frontmatter is not properly closed (missing closing ---)");
|
||||
return std::unexpected("YAML frontmatter is not properly closed (missing closing ---)");
|
||||
}
|
||||
|
||||
// Strip leading newlines from content
|
||||
@@ -105,7 +105,7 @@ SkillMetadata parse_skill_frontmatter(const std::string& filepath) {
|
||||
|
||||
// ---- Scanning ----
|
||||
|
||||
std::vector<SkillMetadata> scan_skills() {
|
||||
std::expected<std::vector<SkillMetadata>, std::string> scan_skills() {
|
||||
std::map<std::string, SkillMetadata> skills_map;
|
||||
std::vector<std::string> discovered_paths;
|
||||
|
||||
@@ -123,9 +123,9 @@ std::vector<SkillMetadata> scan_skills() {
|
||||
};
|
||||
|
||||
// Helper to scan a directory and collect paths
|
||||
auto collect_paths = [&](const std::string& path) {
|
||||
auto collect_paths = [&](const std::string& path) -> std::expected<void, std::string> {
|
||||
if (!fs::exists(path) || !fs::is_directory(path)) {
|
||||
return;
|
||||
return {}; // Silently skip non-existent directories
|
||||
}
|
||||
try {
|
||||
for (const auto& entry : fs::recursive_directory_iterator(path)) {
|
||||
@@ -134,39 +134,46 @@ std::vector<SkillMetadata> scan_skills() {
|
||||
}
|
||||
}
|
||||
} catch (const fs::filesystem_error& e) {
|
||||
std::cerr << "Warning: Could not scan directory " << path << ": " << e.what() << "\n";
|
||||
return std::unexpected(std::string("Could not scan directory ") + path + ": " + e.what());
|
||||
}
|
||||
return {};
|
||||
};
|
||||
|
||||
// Global first
|
||||
for (const auto& p : global_paths) collect_paths(p);
|
||||
for (const auto& p : global_paths) {
|
||||
auto result = collect_paths(p);
|
||||
if (!result) return std::unexpected(result.error());
|
||||
}
|
||||
// Local
|
||||
for (const auto& p : local_paths) collect_paths(p);
|
||||
for (const auto& p : local_paths) {
|
||||
auto result = collect_paths(p);
|
||||
if (!result) return std::unexpected(result.error());
|
||||
}
|
||||
|
||||
// Now parse each discovered skill and enrich with content and files
|
||||
for (const auto& filepath : discovered_paths) {
|
||||
try {
|
||||
SkillMetadata metadata = parse_skill_frontmatter(filepath);
|
||||
|
||||
// Get directory and find other files
|
||||
std::string dir = fs::path(filepath).parent_path().string();
|
||||
metadata.directory = dir;
|
||||
|
||||
if (fs::exists(dir) && fs::is_directory(dir)) {
|
||||
for (const auto& entry : fs::directory_iterator(dir)) {
|
||||
if (entry.is_regular_file() && entry.path().filename() != "SKILL.md") {
|
||||
metadata.files.push_back(fs::absolute(entry.path()).string());
|
||||
}
|
||||
auto parse_result = parse_skill_frontmatter(filepath);
|
||||
if (!parse_result) {
|
||||
return std::unexpected(std::string("Malformed SKILL.md at ") + filepath + ": " + parse_result.error());
|
||||
}
|
||||
|
||||
SkillMetadata metadata = std::move(parse_result.value());
|
||||
|
||||
// Get directory and find other files
|
||||
std::string dir = fs::path(filepath).parent_path().string();
|
||||
metadata.directory = dir;
|
||||
|
||||
if (fs::exists(dir) && fs::is_directory(dir)) {
|
||||
for (const auto& entry : fs::directory_iterator(dir)) {
|
||||
if (entry.is_regular_file() && entry.path().filename() != "SKILL.md") {
|
||||
metadata.files.push_back(fs::absolute(entry.path()).string());
|
||||
}
|
||||
}
|
||||
std::sort(metadata.files.begin(), metadata.files.end());
|
||||
|
||||
// Use map to automatically override global skills with local ones of the same name
|
||||
skills_map[metadata.name] = std::move(metadata);
|
||||
} catch (const std::runtime_error& e) {
|
||||
std::cerr << "Fatal: Malformed SKILL.md at " << filepath << std::endl;
|
||||
exit(1);
|
||||
}
|
||||
std::sort(metadata.files.begin(), metadata.files.end());
|
||||
|
||||
// Use map to automatically override global skills with local ones of the same name
|
||||
skills_map[metadata.name] = std::move(metadata);
|
||||
}
|
||||
|
||||
// Convert map back to vector, preserving insertion order (global then local)
|
||||
|
||||
Reference in New Issue
Block a user