From e71c2a94e5e30a298a8bb402daba5a76fb2b5124 Mon Sep 17 00:00:00 2001 From: Spencer Brower <6729162+sbrow@users.noreply.github.com> Date: Thu, 30 Jul 2026 18:22:20 -0400 Subject: [PATCH] fix: pipe Typos diagnostic now highlights only the broken pipe name. --- PROBLEMS.md | 23 +++++++++++++++++ TODOS.md | 6 +++++ mustache/diagnostic.odin | 10 +++++--- mustache/mustache.odin | 34 ++++++++++++++++++++------ mustache/pipes.odin | 53 +++++++++++++++++++++++++++++----------- 5 files changed, 101 insertions(+), 25 deletions(-) diff --git a/PROBLEMS.md b/PROBLEMS.md index d9f81b3..de6b508 100644 --- a/PROBLEMS.md +++ b/PROBLEMS.md @@ -264,6 +264,29 @@ User's custom layout silently ignored, defaults used. (Same as B1 — restated here for the template-authoring perspective.) One bad tag → whole page `""`. The most impactful silent failure in the system. +### G3. Pipe errors lack "did you mean?" suggestions — High +`mustache/pipes.odin:241` + +Unknown key errors (`{{tittle}}`), missing partials, and unmatched block +overrides all get Levenshtein "did you mean?" hints via `suggest_correction`. +But unknown pipe operations (`{{date | formats}}`) get only `"unknown pipe op +'formats'"` with no suggestion. The known filter names (`"format"`, +`"group_by"`) are a small fixed set — perfect for suggestions. + +Structural gap: `Error_Body` has no `hint` field, and +`format_render_error` doesn't pass `hint` to `format_error` (defaults to +`""`). So even if a suggestion were computed, there's nowhere to put it +without either appending to `msg` or adding `hint` to `Error_Body`. + +### G4. Triple-mustache `{{{` mishandled by `tag_content_base` — Medium +`mustache/mustache.odin:230` + +`tag_content_base` skips `{{` and sigils (`#^/&><$!`) to find where tag +content begins. But triple-mustache `{{{key}}}` is common — after `{{`, +the next char is `{`, which is not in the sigil list, so `base` points at +`{` instead of the actual key content. Any pipe position calculation for +`{{{key | format}}}` will be off by one byte. + --- ## Cross-cutting themes diff --git a/TODOS.md b/TODOS.md index 68dd36d..71dad23 100644 --- a/TODOS.md +++ b/TODOS.md @@ -22,6 +22,11 @@ - [ ] Need to be careful about diagnostics across module boundaries. - we don't necessarily want to warn users about theme designers mistakes. (though perhaps we do) - [ ] consider reporting duplicate weights outside of menus + - [ ] Extend `tag_error` to all render-time errors, not just pipe errors. + Currently only pipe errors (4 sites in `render_nodes`) get stamped with + the correct template source/path. Other render errors still use the + content template's source/path, which can point at the wrong file. + - [ ] try to make file paths clickable links. - [ ] Load grammars dynamically - [ ] starred must be a param. - [ ] Documentation @@ -36,6 +41,7 @@ - [ ] cleanup `#partial switch`es. - [ ] improve json diagnostics. - i.e. "Missing quotes around string", etc. +- [ ] don't use bullshit "sub-tokens", add filters and pipes as proper tokens. ## Performance diff --git a/mustache/diagnostic.odin b/mustache/diagnostic.odin index 43e629d..88ad778 100644 --- a/mustache/diagnostic.odin +++ b/mustache/diagnostic.odin @@ -185,6 +185,7 @@ format_error :: proc( context_before: int = 2, context_after: int = 2, colorize: bool = false, + span: int = 0, ) -> string { line, col := line_col(source, pos) total_lines := count_lines(source) @@ -232,8 +233,11 @@ format_error :: proc( write_gutter(&sb, width, faint, reset) // Caret extent for the error line. - _, token_start, token_end := context_extent(source, pos) - line_start, _, _ := context_extent(source, pos) + line_start, token_start, token_end := context_extent(source, pos) + if span > 0 { + token_start = pos + token_end = pos + span + } caret_start_col := token_start - line_start + 1 caret_end_col := token_end - line_start + 1 if caret_end_col <= caret_start_col { @@ -322,5 +326,5 @@ format_render_error :: proc(err: Error, tmpl: Template, colorize: bool = false) if path == "" { path = "" } - return format_error(path, source, b.pos, b.msg, colorize = colorize) + return format_error(path, source, b.pos, b.msg, colorize = colorize, span = b.span) } diff --git a/mustache/mustache.odin b/mustache/mustache.odin index bf3783a..1d5c59c 100644 --- a/mustache/mustache.odin +++ b/mustache/mustache.odin @@ -31,6 +31,7 @@ Error_Body :: struct { kind: Error_Kind, source: string, path: string, + span: int, } // Error is nil when no error occurred. @@ -55,11 +56,12 @@ tag_error :: proc(err: Error, tmpl: Template) -> Error { if err == nil do return nil b := body(err) return Error_Body { - msg = b.msg, - pos = b.pos, - kind = b.kind, + msg = b.msg, + pos = b.pos, + span = b.span, + kind = b.kind, source = tmpl.source, - path = tmpl.path, + path = tmpl.path, } } @@ -225,6 +227,22 @@ parse_tokens :: proc( return } +// tag_content_base returns the absolute byte offset in source where the +// trimmed tag content begins (after {{, optional sigil, and whitespace). +tag_content_base :: proc(source: string, tag_pos: int) -> int { + base := tag_pos + 2 // skip {{ + if base < len(source) { + switch source[base] { + case '#', '^', '/', '&', '>', '<', '$', '!': + base += 1 + } + } + for base < len(source) && (source[base] == ' ' || source[base] == '\t') { + base += 1 + } + return base +} + parse_section :: proc( tokens: []Token, pos: ^int, @@ -245,7 +263,7 @@ parse_section :: proc( case .Variable: idx := len(nodes) append(nodes, Node{kind = .Variable}) - pipe_key, perr := parse_pipeline(tok.value, &nodes[idx].filters, tok.pos) + pipe_key, perr := parse_pipeline(tok.value, &nodes[idx].filters, tok.pos, tag_content_base(source, tok.pos)) if perr != nil { return Error_Body { msg = fmt.tprintf("pipe parse error in '{{{{%s}}}}': %v", tok.value, perr), @@ -259,7 +277,7 @@ parse_section :: proc( case .Unescaped: idx := len(nodes) append(nodes, Node{kind = .Unescaped}) - pipe_key, perr := parse_pipeline(tok.value, &nodes[idx].filters, tok.pos) + pipe_key, perr := parse_pipeline(tok.value, &nodes[idx].filters, tok.pos, tag_content_base(source, tok.pos)) if perr != nil { return Error_Body { msg = fmt.tprintf("pipe parse error in '{{{{&%s}}}}': %v", tok.value, perr), @@ -279,7 +297,7 @@ parse_section :: proc( content_start := 0 if pos^ < len(tokens) {content_start = tokens[pos^].pos} append(nodes, Node{kind = .Section, pos = tok.pos}) - pipe_key, perr := parse_pipeline(tok.value, &nodes[idx].filters, tok.pos) + pipe_key, perr := parse_pipeline(tok.value, &nodes[idx].filters, tok.pos, tag_content_base(source, tok.pos)) if perr != nil { return Error_Body { msg = fmt.tprintf("pipe parse error in '{{{{#%s}}}}': %v", tok.value, perr), @@ -300,7 +318,7 @@ parse_section :: proc( content_start := 0 if pos^ < len(tokens) {content_start = tokens[pos^].pos} append(nodes, Node{kind = .Inverted, pos = tok.pos}) - pipe_key, perr := parse_pipeline(tok.value, &nodes[idx].filters, tok.pos) + pipe_key, perr := parse_pipeline(tok.value, &nodes[idx].filters, tok.pos, tag_content_base(source, tok.pos)) if perr != nil { return Error_Body { msg = fmt.tprintf("pipe parse error in '{{{{^%s}}}}': %v", tok.value, perr), diff --git a/mustache/pipes.odin b/mustache/pipes.odin index 29699be..c6d222b 100644 --- a/mustache/pipes.odin +++ b/mustache/pipes.odin @@ -17,8 +17,9 @@ MAX_PIPE_ARGS :: 2 DEFAULT_DATE_FORMAT :: "2 Jan 2006" Pipe_Filter :: struct { - op: string, - args: [dynamic; MAX_PIPE_ARGS]string, + op: string, + args: [dynamic; MAX_PIPE_ARGS]string, + op_pos: int, } Group :: struct { @@ -77,6 +78,7 @@ parse_pipeline :: proc( content: string, filters_out: ^[dynamic; MAX_PIPES]Pipe_Filter, pos: int, + content_base: int, ) -> ( key: string, err: Error, @@ -86,14 +88,13 @@ parse_pipeline :: proc( return key, nil } - segments := strings.split(content, "|", allocator = context.temp_allocator) - - filter_count := len(segments) - 1 - if filter_count > MAX_PIPES { + // Count pipes to validate against MAX_PIPES. + pipe_count := strings.count(content, "|") + if pipe_count > MAX_PIPES { return "", Error_Body { msg = fmt.tprintf( "pipe expression has %d filters, max is %d", - filter_count, + pipe_count, MAX_PIPES, ), pos = pos, @@ -101,21 +102,38 @@ parse_pipeline :: proc( } } - key = strings.trim_space(segments[0]) + // Key is everything before the first |. + first_pipe := strings.index(content, "|") + key = strings.trim_space(content[:first_pipe]) if len(key) == 0 { return "", Error_Body{msg = "pipe expression missing key", pos = pos, kind = .Syntax} } - if filter_count == 0 { + if pipe_count == 0 { return key, nil } - for i in 0 ..< filter_count { - seg := strings.trim_space(segments[i + 1]) + // Walk pipe-delimited segments, tracking byte offsets within content. + seg_start := first_pipe + 1 // offset in content, just after | + for seg_start <= len(content) { + next_pipe := strings.index(content[seg_start:], "|") + + // Raw segment text (may have leading/trailing whitespace). + seg_end := seg_start + next_pipe if next_pipe >= 0 else len(content) + raw_seg := content[seg_start:seg_end] + + // Count leading whitespace to find op offset within content. + ws := 0 + for ws < len(raw_seg) && is_pipe_space(raw_seg[ws]) { + ws += 1 + } + seg := strings.trim_space(raw_seg) if len(seg) == 0 { return "", Error_Body{msg = "empty filter", pos = pos, kind = .Syntax} } + op_offset_in_content := seg_start + ws + tokens, terr := tokenize_fields(seg, pos) if terr != nil { delete(tokens) @@ -140,13 +158,19 @@ parse_pipeline :: proc( } filter := Pipe_Filter { - op = tokens[0], + op = tokens[0], + op_pos = content_base + op_offset_in_content, } for j in 1 ..< len(tokens) { append(&filter.args, tokens[j]) } append(filters_out, filter) delete(tokens) + + if next_pipe < 0 { + break + } + seg_start = seg_end + 1 } return key, nil @@ -239,8 +263,9 @@ apply_filter :: proc(value: any, filter: ^Pipe_Filter, pos: int, ctx: []any) -> case: return nil, Error_Body { - msg = fmt.tprintf("unknown pipe op '%s'", filter.op), - pos = pos, + msg = fmt.tprintf("unknown pipe op '%s'", filter.op), + pos = filter.op_pos, + span = len(filter.op), kind = .Data, } }