Refactor/better error handling

Silkstring/Models/AliasEntry.cs

@@ -19,6 +19,8 @@ public class AliasEntry
 
     public string DisplayName = string.Empty;
     public string Name = string.Empty;
+
+    [JsonIgnore] public string[] triggers => Name.Split('|', StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries);
     [JsonIgnore] public string EffectiveName => string.IsNullOrWhiteSpace(DisplayName) ? Name : DisplayName;
     public bool Enabled = true;
     public List<CommandEntry> Output = new();
@@ -32,14 +34,13 @@ public AliasEntry()
 
     public bool IsValid()
     {
-        var names = Name.Split('|', StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries);
-        if (names.Length == 0) return false;
+        if (triggers.Length == 0) return false;
 
-        foreach (var name in names)
+        foreach (var trigger in triggers)
         {
-            if (Blacklist.Contains(name)) return false;
-            if (name.Contains(' ')) return false;
-            if (name.Contains('/')) return false;
+            if (Blacklist.Contains(trigger)) return false;
+            if (trigger.Contains(' ')) return false;
+            if (trigger.Contains('/')) return false;
         }
         return Output.Any(command => !string.IsNullOrWhiteSpace(command.Command));
     }

Silkstring/Models/ThemeColors.cs

@@ -8,6 +8,7 @@ public class ThemeColors
     public Vector4 Folder = new(0.7f, 0.5f, 1.0f, 1.0f);
     public Vector4 LineNumber = new(0.5f, 0.5f, 0.5f, 1.0f);
     public Vector4 Error = new(1.0f, 0.4f, 0.4f, 1.0f);
+    public Vector4 Warning = new(0.90f, 0.70f, 0.20f, 1.0f);
     public Vector4 Success = new(0.4f, 1.0f, 0.4f, 1.0f);
     public Vector4 Variable = new(0.9f, 0.8f, 0.4f, 1.0f);
     public Vector4 Parameter = new(1.0f, 0.6f, 0.3f, 1.0f);

Silkstring/Services/AliasValidator.cs

@@ -7,6 +7,18 @@ namespace Silkstring.Services;
 
 public static class AliasValidator
 {
+    public static List<Diagnostic> Validate(AliasEntry alias, ISet<string> definedVariables, bool allowUnsafe, IEnumerable<AliasEntry> allAliases)
+    {
+        var diagnostics = new List<Diagnostic>();
+        diagnostics.AddRange(ValidateBlocks(alias));
+        diagnostics.AddRange(ValidateSets(alias, definedVariables));
+        diagnostics.AddRange(ValidateWaits(alias));
+        diagnostics.AddRange(ValidateUntils(alias, allowUnsafe));
+        var cycle = FindCycle(alias, allAliases);
+        if (cycle.Count > 0) diagnostics.Add(new($"Cycle detected: {string.Join(" → ", cycle)}"));
+        return diagnostics;
+    }
+
     public static List<string> FindCycle(AliasEntry target, IEnumerable<AliasEntry> allAliases)
     {
         var lookup = BuildTriggerLookup(allAliases);
@@ -15,76 +27,69 @@ public static List<string> FindCycle(AliasEntry target, IEnumerable<AliasEntry>
         return Dfs(target, lookup, visited, path);
     }
 
-    public static string? ValidateBlocks(AliasEntry Alias)
+    public static IEnumerable<Diagnostic> ValidateBlocks(AliasEntry alias)
     {
         var elseSeen = new Stack<bool>();
-
-        foreach (var command in Alias.Output)
+        for (var i = 0; i < alias.Output.Count; i++)
         {
-            var (kind, expression) = BlockInterpreter.Classify((command.Command.Trim()));
-
+            var (kind, expression) = BlockInterpreter.Classify(alias.Output[i].Command.Trim());
             switch (kind)
             {
                 case BlockKind.If:
-                    try { new Parser(Tokenizer.Tokenize(expression)).Parse(); }
-                    catch (ConditionException ex) { return $"Invalid condition: {ex.Message}"; }
+                    if (!Condition.TryParse(expression, out _, out var error)) { yield return new($"Invalid condition: {error}", i); yield break; }
                     elseSeen.Push(false);
                     break;
                 case BlockKind.Else:
-                    if (elseSeen.Count == 0) return ":else without a matching :if";
-                    if (elseSeen.Peek()) return "Duplicate :else in a block";
+                    if (elseSeen.Count == 0) { yield return new(":else without a matching :if", i); yield break; }
+                    if (elseSeen.Peek()) { yield return new("Duplicate :else in a block", i); yield break; }
                     elseSeen.Pop();
                     elseSeen.Push(true);
                     break;
                 case BlockKind.EndIf:
-                    if (elseSeen.Count == 0) return ":endif without a matching :if";
+                    if (elseSeen.Count == 0) { yield return new(":endif without a matching :if", i); yield break; }
                     elseSeen.Pop();
                     break;
             }
         }
-        return elseSeen.Count > 0 ? "Unclosed :if (missing :endif)" : null;
+        if (elseSeen.Count > 0) yield return new("Unclosed :if (missing :endif)");
     }
 
-    public static string? ValidateSets(AliasEntry alias, ISet<string> defined)
+    public static IEnumerable<Diagnostic> ValidateSets(AliasEntry alias, ISet<string> defined)
     {
-        foreach (var command in alias.Output)
+        for (var i = 0; i < alias.Output.Count; i++)
         {
-            var (kind, expression) = BlockInterpreter.Classify(command.Command);
+            var (kind, expression) = BlockInterpreter.Classify(alias.Output[i].Command);
             if (kind != BlockKind.Set) continue;
             var (name, _) = BlockInterpreter.ParseSet(expression);
-            if (string.IsNullOrEmpty(name)) return ":set needs a variable name";
-            if (!defined.Contains(name)) return $"Unknown variable in :set: {name}";
+            if (string.IsNullOrEmpty(name)) { yield return new(":set needs a variable name", i); continue; }
+            if (!defined.Contains(name)) yield return new($"Unknown variable in :set: {name}", i);
         }
-        return null;
     }
 
-    public static string? ValidateWaits(AliasEntry alias)
+    public static IEnumerable<Diagnostic> ValidateWaits(AliasEntry alias)
     {
-        foreach (var command in alias.Output)
+        for (var i = 0; i < alias.Output.Count; i++)
         {
-            var (kind, expression) = BlockInterpreter.Classify(command.Command);
+            var (kind, expression) = BlockInterpreter.Classify(alias.Output[i].Command);
             if (kind != BlockKind.Wait) continue;
             var (value, _) = BlockInterpreter.ParseSet(expression);
-            if (string.IsNullOrEmpty(value)) return ":wait needs a duration";
+            if (string.IsNullOrEmpty(value)) { yield return new(":wait needs a duration", i); continue; }
             if (expression.Contains('{')) continue;
-            if (!BlockInterpreter.TryParseDuration(expression, out _)) return $"Invalid :wait duration: {expression}";
+            if (!BlockInterpreter.TryParseDuration(expression, out _)) yield return new($"Invalid :wait duration: {expression}", i);
         }
-        return null;
     }
 
-    public static string? ValidateUntils(AliasEntry alias, bool allowUnsafe)
+    public static IEnumerable<Diagnostic> ValidateUntils(AliasEntry alias, bool allowUnsafe)
     {
-        foreach (var command in alias.Output)
+        for (var i = 0; i < alias.Output.Count; i++)
         {
-            var (kind, expression) = BlockInterpreter.Classify(command.Command);
+            var (kind, expression) = BlockInterpreter.Classify(alias.Output[i].Command);
             if (kind != BlockKind.Until) continue;
             var (isUnsafe, condition) = BlockInterpreter.ParseUntil(expression);
-            if (string.IsNullOrWhiteSpace(condition)) return ":until needs a condition";
-            try { new Parser(Tokenizer.Tokenize(condition)).Parse(); }
-            catch (ConditionException ex) { return $"Invalid :until condition: {ex.Message}"; }
-            if (isUnsafe && !allowUnsafe) return "This :until uses -unsafe, but unsafe waits are off in settings";
+            if (string.IsNullOrWhiteSpace(condition)) { yield return new(":until needs a condition", i); continue; }
+            if (!Condition.TryParse(condition, out _, out var error)) { yield return new($"Invalid :until condition: {error}", i); continue; }
+            if (isUnsafe && !allowUnsafe) yield return new("This :until uses -unsafe, but unsafe waits are off in settings", i, Severity.Warning);
         }
-        return null;
     }
 
     private static Dictionary<string, AliasEntry> BuildTriggerLookup(IEnumerable<AliasEntry> allAliases)

Silkstring/Services/CommandHandler.cs

@@ -100,18 +100,7 @@ await _framework.RunOnFrameworkThread(() =>
         }
     }
 
-    private bool EvaluateSafe(string expression, IReadOnlyList<string> args)
-    {
-        try
-        {
-            return _conditions.Evaluate(expression, args);
-        }
-        catch (ConditionException ex)
-        {
-            Log.Warning(ex, "Invalid condition: {Expression}", expression);
-            return false;
-        }
-    }
+    private bool EvaluateSafe(string expression, IReadOnlyList<string> args) => _conditions.Evaluate(expression, args);
 
     private async Task WaitUntilAsync(string condition, IReadOnlyList<string> args, bool isUnsafe, int capMs, CancellationToken token)
     {

Silkstring/Services/Conditions/Condition.cs

@@ -0,0 +1,11 @@
+namespace Silkstring.Services.Conditions;
+
+internal static class Condition
+{
+    public static bool TryParse(string expression, out ConditionNode? node, out string? error)
+    {
+        node = null;
+        if (!Tokenizer.TryTokenize(expression, out var tokens, out error)) return false;
+        return new Parser(tokens).TryParse(out node, out error);
+    }
+}

Silkstring/Services/Conditions/ConditionEvaluator.cs

@@ -9,11 +9,7 @@ public sealed class ConditionEvaluator
 
     public ConditionEvaluator(Func<string, IReadOnlyList<string>, string> resolve) => _resolve = resolve;
 
-    public bool Evaluate(string expression, IReadOnlyList<string> args)
-    {
-        var ast = new Parser(Tokenizer.Tokenize(expression)).Parse();
-        return Eval(ast, args);
-    }
+    public bool Evaluate(string expression, IReadOnlyList<string> args) => Condition.TryParse(expression, out var ast, out _) && Eval(ast!, args);
 
     private bool Eval(ConditionNode node, IReadOnlyList<string> args) => node switch
     {

Silkstring/Services/Conditions/Parser.cs

@@ -6,58 +6,60 @@ internal sealed class Parser
 {
     private readonly List<Token> _tokens;
     private int _pos;
+    private string? _error;
+    public Parser(List<Token> tokens) => _tokens = tokens;
 
-    public Parser(List<Token> tokens) =>  _tokens = tokens;
+    public bool TryParse(out ConditionNode? node, out string? error)
+    {
+        node = ParseOr();
+        if (_error == null && Peek is { } leftover) _error = $"Unexpected '{leftover.Text}'";
+        error = _error;
+        if (_error != null) node = null;
+        return _error == null;
+    }
 
     private Token? Peek => _pos < _tokens.Count ? _tokens[_pos] : null;
     private void Next() => _pos++;
     private bool IsOp(string s) => Peek is { Kind: TokenKind.Op } t && t.Text == s;
     private static bool IsComparison(string s) => s is "==" or "!=" or "<" or ">" or "<=" or ">=";
 
-    public ConditionNode Parse()
-    {
-        var n = ParseOr();
-        if (Peek is { } leftover) throw new ConditionException($"Unexpected '{leftover.Text}'");
-        return n;
-    }
-
-    private ConditionNode ParseOr()
+    private ConditionNode? ParseOr()
     {
         var n = ParseAnd();
-        while (IsOp("||")) { Next(); n = new OrNode(n, ParseAnd()); }
+        while (_error == null && IsOp("||")) { Next(); n = new OrNode(n!, ParseAnd()!); }
         return n;
     }
-
-    private ConditionNode ParseAnd()
+    private ConditionNode? ParseAnd()
     {
         var n = ParseCmp();
-        while (IsOp("&&")) { Next(); n = new AndNode(n, ParseCmp()); }
+        while (_error == null && IsOp("&&")) { Next(); n = new AndNode(n!, ParseCmp()!); }
         return n;
     }
-
-    private ConditionNode ParseCmp()
+    private ConditionNode? ParseCmp()
     {
         if (Peek is { Kind: TokenKind.LParen })
         {
             Next();
             var n = ParseOr();
-            if (Peek is not { Kind: TokenKind.RParen }) throw new ConditionException("Expected ')'");
+            if (_error != null) return null;
+            if (Peek is not { Kind: TokenKind.RParen }) { _error = "Expected ')'"; return null; }
             Next();
             return n;
         }
-
         var left = ExpectOperand();
+        if (_error != null) return null;
         if (Peek is { Kind: TokenKind.Op } op && IsComparison(op.Text))
         {
             Next();
-            return new CmpNode(left, op.Text, ExpectOperand());
+            var right = ExpectOperand();
+            if (_error != null) return null;
+            return new CmpNode(left!, op.Text, right!);
         }
-        return new BareNode(left);
+        return new BareNode(left!);
     }
-
-    private string ExpectOperand()
+    private string? ExpectOperand()
     {
-        if (Peek is not { Kind: TokenKind.Operand } t) throw new ConditionException("Expected a value");
+        if (Peek is not { Kind: TokenKind.Operand } t) { _error = "Expected a value"; return null; }
         Next();
         return t.Text;
     }

Silkstring/Services/Conditions/Tokenizer.cs

@@ -6,9 +6,10 @@ internal enum TokenKind { Operand, Op, LParen, RParen }
 internal readonly record struct Token(TokenKind Kind, string Text);
 internal static class Tokenizer
 {
-    public static List<Token> Tokenize(string expr)
+    public static bool TryTokenize(string expr, out List<Token> tokens, out string? error)
     {
-        var tokens = new List<Token>();
+        tokens = new List<Token>();
+        error = null;
         var i = 0;
         while (i < expr.Length)
         {
@@ -24,12 +25,12 @@ public static List<Token> Tokenize(string expr)
             }
 
             if (c is '<' or '>') { tokens.Add(new(TokenKind.Op, c.ToString())); i++; continue; }
-            if (c is '&' or '|' or '=' or '!') throw new ConditionException($"Unexpected '{c}'");
+            if (c is '&' or '|' or '=' or '!') { error = $"Unexpected '{c}'"; return false; }
 
             if (c == '"')
             {
                 var end = expr.IndexOf('"', i + 1);
-                if (end < 0) throw new ConditionException("Unterminated Quote");
+                if (end < 0) { error = "Unterminated quote"; return false; }
                 tokens.Add(new(TokenKind.Operand, expr[(i + 1)..end]));
                 i = end + 1;
                 continue;
@@ -39,7 +40,6 @@ public static List<Token> Tokenize(string expr)
             while (i < expr.Length && !char.IsWhiteSpace(expr[i]) && expr[i] is not ('(' or ')' or '"' or '&' or '|' or '=' or '!' or '<' or '>')) i++;
             tokens.Add(new(TokenKind.Operand, expr[start..i]));
         }
-
-        return tokens;
+        return true;
     }
 }

Silkstring/Services/Diagnostic.cs

@@ -0,0 +1,5 @@
+namespace Silkstring.Services;
+
+public enum Severity { Error, Warning }
+
+public readonly record struct Diagnostic(string Message, int? Line = null, Severity Severity = Severity.Error);

Silkstring/UI/Palette.cs

@@ -12,6 +12,7 @@ public static class Palette
     public static Vector4 Command;
     public static Vector4 Text;
     public static Vector4 Error;
+    public static Vector4 Warning;
     public static Vector4 Heading;
     public static Vector4 Folder;
     public static Vector4 Success;
@@ -27,6 +28,7 @@ public static void Apply(ThemeColors t)
         Command = t.Command;
         Text = t.Text;
         Error = t.Error;
+        Warning = t.Warning;
         Heading = t.Heading;
         Folder = t.Folder;
         Success = t.Success;