From 94bcee63222c682dc759003a2a8165e03e2f19e6 Mon Sep 17 00:00:00 2001 From: Navid EMAD Date: Wed, 6 May 2026 18:19:44 +0200 Subject: [PATCH] xpath: apply review style/convention feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Rename Result.zig / Ast.zig / Functions.zig to snake_case (no top-level fields per Zig style guide) - Restructure imports across xpath module: lib (std/lp) → relative (further → nearer) → aliases - Move `frame` to last parameter on Evaluator.evaluate, searchAll, Functions.call, idFn (matches js bridge convention); call sites updated in webapi/XPath{Result,Expression}.zig and cdp/domains/dom.zig - Local-pos style in XPathResult.iterateNext --- src/browser/webapi/XPathExpression.zig | 4 ++-- src/browser/webapi/XPathResult.zig | 11 +++++---- src/browser/xpath/Evaluator.zig | 24 ++++++++++--------- src/browser/xpath/Parser.zig | 5 ++-- src/browser/xpath/{Ast.zig => ast.zig} | 0 .../xpath/{Functions.zig => functions.zig} | 16 +++++++------ src/browser/xpath/{Result.zig => result.zig} | 3 ++- src/cdp/domains/dom.zig | 2 +- 8 files changed, 36 insertions(+), 29 deletions(-) rename src/browser/xpath/{Ast.zig => ast.zig} (100%) rename src/browser/xpath/{Functions.zig => functions.zig} (98%) rename src/browser/xpath/{Result.zig => result.zig} (99%) diff --git a/src/browser/webapi/XPathExpression.zig b/src/browser/webapi/XPathExpression.zig index 8c771d2e..b24b6268 100644 --- a/src/browser/webapi/XPathExpression.zig +++ b/src/browser/webapi/XPathExpression.zig @@ -33,7 +33,7 @@ const Node = @import("Node.zig"); const XPathResult = @import("XPathResult.zig"); const xpath = struct { - const Ast = @import("../xpath/Ast.zig"); + const Ast = @import("../xpath/ast.zig"); const Parser = @import("../xpath/Parser.zig"); const Evaluator = @import("../xpath/Evaluator.zig"); }; @@ -76,7 +76,7 @@ pub fn evaluate( const arena = try frame.getArena(.medium, "XPathResult"); errdefer frame.releaseArena(arena); - const eval_result = try xpath.Evaluator.evaluate(arena, frame, self._expr, context_node); + const eval_result = try xpath.Evaluator.evaluate(arena, self._expr, context_node, frame); return XPathResult.fromResult(arena, requested_type, eval_result); } diff --git a/src/browser/webapi/XPathResult.zig b/src/browser/webapi/XPathResult.zig index 1da520ec..44c29b44 100644 --- a/src/browser/webapi/XPathResult.zig +++ b/src/browser/webapi/XPathResult.zig @@ -46,7 +46,7 @@ const Node = @import("Node.zig"); const xpath = struct { const Parser = @import("../xpath/Parser.zig"); const Evaluator = @import("../xpath/Evaluator.zig"); - const Result = @import("../xpath/Result.zig"); + const Result = @import("../xpath/result.zig"); }; const Allocator = std.mem.Allocator; @@ -101,7 +101,7 @@ pub fn fromExpression( // dupe into our long-lived arena before parsing. const owned = try arena.dupe(u8, expression); const expr = try xpath.Parser.parse(arena, owned); - const result = try xpath.Evaluator.evaluate(arena, frame, expr, context_node); + const result = try xpath.Evaluator.evaluate(arena, expr, context_node, frame); return fromResult(arena, requested_type, result); } @@ -220,9 +220,10 @@ pub fn iterateNext(self: *XPathResult) !?*Node { if (self._type != UNORDERED_NODE_ITERATOR_TYPE and self._type != ORDERED_NODE_ITERATOR_TYPE) { return error.InvalidStateError; } - if (self._iter_pos >= self._value.nodes.len) return null; - const node = self._value.nodes[self._iter_pos]; - self._iter_pos += 1; + const pos = self._iter_pos; + if (pos >= self._value.nodes.len) return null; + const node = self._value.nodes[pos]; + self._iter_pos = pos + 1; return node; } diff --git a/src/browser/xpath/Evaluator.zig b/src/browser/xpath/Evaluator.zig index 99202cbd..d654ed8f 100644 --- a/src/browser/xpath/Evaluator.zig +++ b/src/browser/xpath/Evaluator.zig @@ -30,17 +30,19 @@ //! reverse-axis positional predicates evaluate against proximity. const std = @import("std"); -const Allocator = std.mem.Allocator; const lp = @import("lightpanda"); -const Ast = @import("Ast.zig"); -const Parser = @import("Parser.zig"); -const Result = @import("Result.zig"); -const Functions = @import("Functions.zig"); const Node = @import("../webapi/Node.zig"); + +const Ast = @import("ast.zig"); +const Parser = @import("Parser.zig"); +const Result = @import("result.zig"); +const Functions = @import("functions.zig"); + +const Frame = lp.Frame; const Element = Node.Element; const Document = Node.Document; -const Frame = lp.Frame; +const Allocator = std.mem.Allocator; const Evaluator = @This(); @@ -62,7 +64,7 @@ frame: *Frame, /// Public entry. Returns the AST's value; node-sets are sorted into /// document order before return per XPath spec §3.3. -pub fn evaluate(arena: Allocator, frame: *Frame, expr: *const Ast.Expr, context_node: *Node) Error!Result.Result { +pub fn evaluate(arena: Allocator, expr: *const Ast.Expr, context_node: *Node, frame: *Frame) Error!Result.Result { var ev = Evaluator{ .arena = arena, .frame = frame }; const result = try ev.evalExpr(expr, context_node, 1, 1); if (result == .node_set) { @@ -77,9 +79,9 @@ pub const SearchError = Error || Parser.Error; /// evaluate and unwrap the node-set. Top-level scalar expressions yield /// an empty slice (decision #3 — these APIs are for finding nodes, not /// arbitrary computation). -pub fn searchAll(arena: Allocator, frame: *Frame, root: *Node, expression: []const u8) SearchError![]const *Node { +pub fn searchAll(arena: Allocator, root: *Node, expression: []const u8, frame: *Frame) SearchError![]const *Node { const expr = try Parser.parse(arena, expression); - return switch (try evaluate(arena, frame, expr, root)) { + return switch (try evaluate(arena, expr, root, frame)) { .node_set => |ns| ns, else => &.{}, }; @@ -506,7 +508,7 @@ fn evalFnCall(self: *Evaluator, fc: Ast.FnCall, ctx: *Node, pos: usize, size: us const eval_args = try self.arena.alloc(Result.Result, fc.args.len); for (fc.args, 0..) |a, i| eval_args[i] = try self.evalExpr(a, ctx, pos, size); - return Functions.call(self.arena, self.frame, fc.name, eval_args, ctx); + return Functions.call(self.arena, fc.name, eval_args, ctx, self.frame); } // ----- helpers ----- @@ -726,7 +728,7 @@ test "Evaluator: searchAll on scalar expression returns empty (decision #3)" { // the Frame or the context node. Adding a DOM-touching expression // (e.g. `id('x')`) to this list would crash on dereference. inline for (.{ "1 + 2", "'hello'", "true()", "1 = 1" }) |expr| { - const nodes = try searchAll(a, @ptrFromInt(0x1000), @ptrFromInt(0x2000), expr); + const nodes = try searchAll(a, @ptrFromInt(0x2000), expr, @ptrFromInt(0x1000)); try testing.expectEqual(@as(usize, 0), nodes.len); } } diff --git a/src/browser/xpath/Parser.zig b/src/browser/xpath/Parser.zig index 88d25b26..aa969e3f 100644 --- a/src/browser/xpath/Parser.zig +++ b/src/browser/xpath/Parser.zig @@ -25,11 +25,12 @@ //! and is valid for as long as the arena and input outlive it. const std = @import("std"); -const Allocator = std.mem.Allocator; const Tokenizer = @import("Tokenizer.zig"); +const Ast = @import("ast.zig"); + const Token = Tokenizer.Token; -const Ast = @import("Ast.zig"); +const Allocator = std.mem.Allocator; const Parser = @This(); diff --git a/src/browser/xpath/Ast.zig b/src/browser/xpath/ast.zig similarity index 100% rename from src/browser/xpath/Ast.zig rename to src/browser/xpath/ast.zig diff --git a/src/browser/xpath/Functions.zig b/src/browser/xpath/functions.zig similarity index 98% rename from src/browser/xpath/Functions.zig rename to src/browser/xpath/functions.zig index d0ae7eac..52cb4d14 100644 --- a/src/browser/xpath/Functions.zig +++ b/src/browser/xpath/functions.zig @@ -37,14 +37,16 @@ //! Allocations land in the caller's per-evaluation arena. const std = @import("std"); -const Allocator = std.mem.Allocator; const lp = @import("lightpanda"); -const Result = @import("Result.zig"); const Node = @import("../webapi/Node.zig"); + +const Result = @import("result.zig"); + +const Frame = lp.Frame; const Element = Node.Element; const Document = Node.Document; -const Frame = lp.Frame; +const Allocator = std.mem.Allocator; pub const Error = error{ OutOfMemory, @@ -59,14 +61,14 @@ pub const Error = error{ /// last lookup stop. pub fn call( arena: Allocator, - frame: *Frame, name: []const u8, args: []const Result.Result, ctx: *Node, + frame: *Frame, ) Error!Result.Result { // -- Node-set -- if (eql(name, "count")) return .{ .number = countFn(args) }; - if (eql(name, "id")) return idFn(arena, frame, args, ctx); + if (eql(name, "id")) return idFn(arena, args, ctx, frame); if (eql(name, "local-name")) return .{ .string = try localNameFn(arena, args, ctx) }; if (eql(name, "name")) return .{ .string = try nameFn(arena, args, ctx) }; if (eql(name, "namespace-uri")) return .{ .string = "" }; @@ -111,7 +113,7 @@ fn countFn(args: []const Result.Result) f64 { return @floatFromInt(args[0].node_set.len); } -fn idFn(arena: Allocator, frame: *Frame, args: []const Result.Result, ctx: *Node) Error!Result.Result { +fn idFn(arena: Allocator, args: []const Result.Result, ctx: *Node, frame: *Frame) Error!Result.Result { if (args.len == 0) return .{ .node_set = &.{} }; // Polyfill: node-set arg → join `stringVal(n)` of each by ' '. Scalar @@ -345,7 +347,7 @@ fn evalScalar(a: Allocator, src: []const u8) !Result.Result { // Synthetic Frame/Node pointers — the public `evaluate` entry only // touches the Frame for path/axis evaluation. Pure-scalar expressions // (arithmetic, function calls returning scalars) never deref it. - return Evaluator.evaluate(a, @ptrFromInt(0x1000), expr, @ptrFromInt(0x2000)); + return Evaluator.evaluate(a, expr, @ptrFromInt(0x2000), @ptrFromInt(0x1000)); } test "Functions: count() of non-node-set returns 0" { diff --git a/src/browser/xpath/Result.zig b/src/browser/xpath/result.zig similarity index 99% rename from src/browser/xpath/Result.zig rename to src/browser/xpath/result.zig index c0822054..e71efe83 100644 --- a/src/browser/xpath/Result.zig +++ b/src/browser/xpath/result.zig @@ -26,10 +26,11 @@ //! shortcuts inherited from the polyfill (decision #2). const std = @import("std"); -const Allocator = std.mem.Allocator; const Node = @import("../webapi/Node.zig"); + const CData = Node.CData; +const Allocator = std.mem.Allocator; pub const Result = union(enum) { /// Owned by the evaluator's arena. Order is significant only at the diff --git a/src/cdp/domains/dom.zig b/src/cdp/domains/dom.zig index 843cc20e..32af266c 100644 --- a/src/cdp/domains/dom.zig +++ b/src/cdp/domains/dom.zig @@ -130,7 +130,7 @@ fn performSearch(cmd: *CDP.Command) !void { if (isXPathQuery(params.query)) { const arena = try frame.getArena(.medium, "DOM.performSearch"); defer frame.releaseArena(arena); - const nodes = try xpath.searchAll(arena, frame, root, params.query); + const nodes = try xpath.searchAll(arena, root, params.query, frame); return finishSearch(cmd, bc, nodes); }