mirror of
https://github.com/sbrow/thor.git
synced 2026-08-26 11:23:32 -04:00
Compare commits
3 Commits
0307ab647f
...
e71c2a94e5
| Author | SHA1 | Date | |
|---|---|---|---|
| e71c2a94e5 | |||
| ef7327cc52 | |||
| 7b67201791 |
@@ -0,0 +1,61 @@
|
||||
# Diagnostics
|
||||
|
||||
Thor has two diagnostic tiers. This document explains why, and when to use each.
|
||||
|
||||
## Tier 1: Rust-style rich diagnostics (mustache engine)
|
||||
|
||||
`mustache/diagnostic.odin` implements multi-line source context, caret underlines, ANSI colors (gated on TTY detection), and Levenshtein suggestions. Used exclusively by the mustache engine for template errors:
|
||||
|
||||
- Unknown keys in `{{k}}`, `{{{k}}}`, `{{#k}}`, `{{^k}}`
|
||||
- Missing partials (`{{> name}}`)
|
||||
- Missing parent templates (`{{<name}}`)
|
||||
- Unmatched block overrides (`{{$name}}`)
|
||||
- Parse-time syntax errors
|
||||
|
||||
These benefit from rich diagnostics because **exact source location matters** — templates have complex syntax, and the user often doesn't know *where* the problem is. The diagnostic system operates on `Template.source` with byte offsets, producing output like:
|
||||
|
||||
```
|
||||
error: unknown key 'titel' in {{page.titel}}
|
||||
--> layouts/page.html:12:22
|
||||
|
|
||||
12 | <h1>{{page.titel}}</h1>
|
||||
| ^^^^^
|
||||
|
|
||||
= hint: did you mean 'title'?
|
||||
```
|
||||
|
||||
## Tier 2: Simple log warnings (content and runtime)
|
||||
|
||||
Everything outside the mustache engine uses `log.warnf` — flat one-line messages via `core:log`:
|
||||
|
||||
- Missing frontmatter dates (fallback to file mtime)
|
||||
- Duplicate menu weights
|
||||
- Non-numeric weight values in frontmatter
|
||||
- Tree-sitter highlight errors
|
||||
- Menu system issues (mixing config/frontmatter menus, etc.)
|
||||
|
||||
These are simple, actionable, and cross-file. The problem isn't *location* — it's that two files disagree, or a value is missing. A caret pointing at one file doesn't help; the message already communicates what to fix:
|
||||
|
||||
```
|
||||
[WARN] --- [menus.odin:290:warn_duplicate_weights()] menus('main'):'Ideas' and 'Stuff' share the same weight (11).
|
||||
```
|
||||
|
||||
## Why not use rich diagnostics everywhere?
|
||||
|
||||
Rust's diagnostic model is built for a single compilation unit with full AST/IR data. Three obstacles prevent reusing it for content warnings:
|
||||
|
||||
1. **Source tracking**: The mustache diagnostic system operates on `Template.source` (byte offsets into template strings). Content warnings come from frontmatter in markdown files — different source, different parser, no position tracking. Reusing the system would require building a parallel position-tracking infrastructure for frontmatter.
|
||||
|
||||
2. **Cross-file context**: Rust diagnostics point at one location. Weight duplicates are a relationship between two files. Rich diagnostics would need to show *both* file locations, which is more infrastructure for marginal value.
|
||||
|
||||
3. **Diminishing returns**: Rust diagnostics shine for syntax/type errors where the user doesn't understand the failure. Content warnings are already self-explanatory — "these two pages share weight 11" doesn't need a caret to be actionable.
|
||||
|
||||
## When to upgrade a Tier 2 warning to Tier 1
|
||||
|
||||
If a warning's usefulness would significantly improve from showing exact source location (e.g., a frontmatter syntax error where the user needs to see *which line* is malformed), consider extending the diagnostic system to frontmatter. This would require:
|
||||
|
||||
1. Position tracking in `frontmatter.odin` (store byte offsets for each parsed field)
|
||||
2. A `format_frontmatter_error` proc modeled on `format_render_error`
|
||||
3. File path propagation through the page loading pipeline
|
||||
|
||||
This is not currently planned — see `TODOS.md`.
|
||||
+23
@@ -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
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
- Polish existing features before moving on to new ones.
|
||||
- [ ] Improve diagnostics
|
||||
- [ ] keep track of every error and don't report them more than once.
|
||||
- [ ] All Diagnostics should show:
|
||||
- [ ] *What* went wrong
|
||||
- [ ] *where* (in the file)
|
||||
@@ -21,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
|
||||
@@ -35,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
|
||||
|
||||
|
||||
@@ -393,4 +393,3 @@ parse_config_menus :: proc(
|
||||
|
||||
return result
|
||||
}
|
||||
|
||||
|
||||
@@ -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 {
|
||||
@@ -309,14 +313,18 @@ write_gutter :: proc(sb: ^strings.Builder, width: int, faint: string, reset: str
|
||||
|
||||
// format_render_error produces a diagnostic for an Error value using the
|
||||
// template's path and source for context. Returns "" for nil errors.
|
||||
// If the error carries its own source/path (from tag_error), those are used
|
||||
// instead of the passed-in template — this ensures errors inside partials
|
||||
// point at the correct file.
|
||||
format_render_error :: proc(err: Error, tmpl: Template, colorize: bool = false) -> string {
|
||||
if err == nil {
|
||||
return ""
|
||||
}
|
||||
path := tmpl.path
|
||||
b := body(err)
|
||||
source := b.source != "" ? b.source : tmpl.source
|
||||
path := b.path != "" ? b.path : tmpl.path
|
||||
if path == "" {
|
||||
path = "<input>"
|
||||
}
|
||||
b := body(err)
|
||||
return format_error(path, tmpl.source, b.pos, b.msg, colorize = colorize)
|
||||
return format_error(path, source, b.pos, b.msg, colorize = colorize, span = b.span)
|
||||
}
|
||||
|
||||
+45
-11
@@ -26,9 +26,12 @@ Error_Kind :: enum {
|
||||
}
|
||||
|
||||
Error_Body :: struct {
|
||||
msg: string,
|
||||
pos: int,
|
||||
kind: Error_Kind,
|
||||
msg: string,
|
||||
pos: int,
|
||||
kind: Error_Kind,
|
||||
source: string,
|
||||
path: string,
|
||||
span: int,
|
||||
}
|
||||
|
||||
// Error is nil when no error occurred.
|
||||
@@ -47,6 +50,21 @@ body :: proc(err: Error) -> Error_Body {
|
||||
}
|
||||
}
|
||||
|
||||
// tag_error stamps an Error with the source/path of the template where it
|
||||
// originated, so diagnostics point at the correct file (e.g. a partial).
|
||||
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,
|
||||
span = b.span,
|
||||
kind = b.kind,
|
||||
source = tmpl.source,
|
||||
path = tmpl.path,
|
||||
}
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Node tree
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -209,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,
|
||||
@@ -229,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),
|
||||
@@ -243,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),
|
||||
@@ -263,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),
|
||||
@@ -284,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),
|
||||
@@ -585,7 +619,7 @@ render_nodes :: proc(
|
||||
if len(node.filters) > 0 {
|
||||
transformed, perr := apply_pipeline(val, node.filters[:], node.pos, ctx[:])
|
||||
if perr != nil {
|
||||
return perr
|
||||
return tag_error(perr, current)
|
||||
}
|
||||
val = transformed
|
||||
}
|
||||
@@ -627,7 +661,7 @@ render_nodes :: proc(
|
||||
if len(node.filters) > 0 {
|
||||
transformed, perr := apply_pipeline(val, node.filters[:], node.pos, ctx[:])
|
||||
if perr != nil {
|
||||
return perr
|
||||
return tag_error(perr, current)
|
||||
}
|
||||
val = transformed
|
||||
}
|
||||
@@ -665,7 +699,7 @@ render_nodes :: proc(
|
||||
if len(node.filters) > 0 {
|
||||
transformed, perr := apply_pipeline(val, node.filters[:], node.pos, ctx[:])
|
||||
if perr != nil {
|
||||
return perr
|
||||
return tag_error(perr, current)
|
||||
}
|
||||
val = transformed
|
||||
}
|
||||
@@ -729,7 +763,7 @@ render_nodes :: proc(
|
||||
if len(node.filters) > 0 {
|
||||
transformed, perr := apply_pipeline(val, node.filters[:], node.pos, ctx[:])
|
||||
if perr != nil {
|
||||
return perr
|
||||
return tag_error(perr, current)
|
||||
}
|
||||
val = transformed
|
||||
}
|
||||
|
||||
+39
-14
@@ -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,
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user