Keep options in a fixed order, and say which block a diff hunk is in

Options of a block (server options, a view, a zone) are listed and
written grouped by topic, alphabetical within a group, so the same
option is in the same place everywhere. Top-level statements keep the
order they were added in.

named.conf diffs add the enclosing blocks to each hunk header, like
git's function context: @@ -50,6 +49,8 @@ view "internal" {.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

CHANGELOG.md

@@ -48,6 +48,16 @@ All notable changes to Jarl are documented here. The format follows
 
 ### Changed
 
+- Options are kept in one fixed order, in named.conf, the web UI and the
+  CLI: grouped by topic (files, listening, access control, recursion and
+  forwarding, DNSSEC, transfers and notify, then the rest), alphabetical
+  within a group. The same option is now in the same place in every view
+  and zone. The first apply after upgrading reorders named.conf (a diff
+  that only moves lines). Top-level statements keep their order.
+- Diffs of named.conf say which block each change is in, after the `@@`
+  line numbers (`@@ -50,6 +49,8 @@ view "internal" {`), like git does
+  with function names. Moving an option from the server's options into a
+  view no longer looks like two unrelated edits.
 - Web UI options (server, top-level, view and zone): values with blocks,
   such as logging, are shown and edited laid out as in named.conf, over
   several lines with indentation. The editor grows with its content, Tab

docs/api.md

@@ -444,7 +444,9 @@ listing with `?changeset=` marks them `pending`.
 | GET / PATCH | `/settings` | `{"nameservers": [...], "rname": "...", "default_ttl": 3600, "refresh": ..., "retry": ..., "expire": ..., "negative_ttl": ..., "serial_policy": "date"}` |
 
 Option values are named.conf syntax without the final semicolon. They are
-checked against the BIND 9.20 grammar for that scope.
+checked against the BIND 9.20 grammar for that scope. `position` only
+orders top-level statements; the options of a block are always listed and
+written in Jarl's fixed order (docs/concepts.md).
 
 ### Catalogs and search
 

docs/concepts.md

@@ -68,7 +68,9 @@ both, using a single mechanism:
 * `POST /api/v1/apply` (CLI: `jarl apply -f changes.yaml`) applies a list of
   operations as one changeset in a single request.
 * `?dry_run=1` (CLI: `--dry-run`) runs the whole pipeline except installing
-  files and reloading named, and shows the problems and the diff.
+  files and reloading named, and shows the problems and the diff. Each
+  part of a named.conf diff says which block it is in, after the `@@`
+  line numbers: `@@ -50,6 +49,8 @@ view "internal" {`.
 
 **Why bundling matters.** Some changes only make sense together. Turning
 `www` from an A record into a CNAME means deleting the A record and adding
@@ -159,6 +161,12 @@ RFC 1982 arithmetic.
   Jarl checks them before named does. Statements that Jarl manages itself
   (`zone`, `view`, `acl`, `key`, a zone's `type`/`file`, a view's
   `match-clients`, ...) can't be set as raw options.
+  Options of a block are kept in one fixed order, the same in named.conf,
+  the web UI and the CLI: grouped (files, listening, access control,
+  recursion and forwarding, DNSSEC, transfers and notify, then the rest),
+  alphabetical within a group. So the same option is in the same place in
+  every view and zone. Top-level statements stay in the order they were
+  added.
 * **Settings** are Jarl's defaults for new zones: name servers, SOA contact
   and timers, default TTL, serial policy.
 

internal/engine/deploy.go

@@ -335,5 +335,8 @@ func fileDiff(path string, old []byte, existed bool, new []byte, removed, secret
 	if d != "" && !strings.HasSuffix(d, "\n") {
 		d += "\n"
 	}
+	if strings.HasSuffix(path, ".conf") {
+		d = labelHunks(d, string(old), string(new))
+	}
 	return d
 }

internal/engine/diffcontext.go

@@ -0,0 +1,136 @@
+package engine
+
+import (
+	"regexp"
+	"strconv"
+	"strings"
+)
+
+// hunkHeader matches a unified diff hunk header; the line counts are left
+// out when they are 1.
+var hunkHeader = regexp.MustCompile(`^@@ -(\d+)(?:,\d+)? \+(\d+)(?:,\d+)? @@$`)
+
+// labelHunks adds the blocks a hunk's first change is in to its header,
+// like git does with function names, so a change to named.conf says where
+// it is:
+//
+//	@@ -50,6 +49,8 @@ view "internal" {
+//
+// Removed lines are looked up in the old file, added ones in the new.
+func labelHunks(diff, old, new string) string {
+	if !strings.Contains(diff, "\n@@ ") {
+		return diff
+	}
+	oldCtx, newCtx := blockContext(old), blockContext(new)
+	lines := strings.SplitAfter(diff, "\n")
+	for i, line := range lines {
+		m := hunkHeader.FindStringSubmatch(strings.TrimSuffix(line, "\n"))
+		if m == nil {
+			continue
+		}
+		o, _ := strconv.Atoi(m[1])
+		n, _ := strconv.Atoi(m[2])
+		label := ""
+	hunk:
+		for _, l := range lines[i+1:] {
+			switch {
+			case strings.HasPrefix(l, "-"):
+				label = at(oldCtx, o)
+				break hunk
+			case strings.HasPrefix(l, "+"):
+				label = at(newCtx, n)
+				break hunk
+			case strings.HasPrefix(l, " "):
+				o, n = o+1, n+1
+			case strings.HasPrefix(l, `\`):
+			default:
+				break hunk
+			}
+		}
+		if label != "" {
+			lines[i] = strings.TrimSuffix(line, "\n") + " " + label + "\n"
+		}
+	}
+	return strings.Join(lines, "")
+}
+
+// at returns the context of line n (1-based), or "".
+func at(ctx []string, n int) string {
+	if n < 1 || n > len(ctx) {
+		return ""
+	}
+	return ctx[n-1]
+}
+
+// blockContext returns, for each line of a named.conf file, the blocks
+// open at its start: `view "internal" { zone "example.com" {`, or "" at
+// the top level. It knows named.conf's strings and comments.
+func blockContext(text string) []string {
+	var (
+		out     []string
+		stack   []string
+		stmt    strings.Builder // the current statement, up to a { or ;
+		inStr   bool
+		inBlock bool // in a /* */ comment
+	)
+	label := func() string {
+		if len(stack) == 0 {
+			return ""
+		}
+		return strings.Join(stack, " { ") + " {"
+	}
+	for _, line := range strings.SplitAfter(text, "\n") {
+		if line == "" {
+			continue
+		}
+		out = append(out, label())
+		for i := 0; i < len(line); i++ {
+			c := line[i]
+			switch {
+			case inBlock:
+				if c == '*' && i+1 < len(line) && line[i+1] == '/' {
+					inBlock = false
+					i++
+				}
+			case inStr:
+				stmt.WriteByte(c)
+				if c == '\\' && i+1 < len(line) {
+					stmt.WriteByte(line[i+1])
+					i++
+				} else if c == '"' {
+					inStr = false
+				}
+			case c == '"':
+				inStr = true
+				stmt.WriteByte(c)
+			case c == '#' || c == '/' && i+1 < len(line) && line[i+1] == '/':
+				i = len(line) // a comment to the end of the line
+			case c == '/' && i+1 < len(line) && line[i+1] == '*':
+				inBlock = true
+				i++
+			case c == '{':
+				stack = append(stack, shortLabel(stmt.String()))
+				stmt.Reset()
+			case c == '}':
+				if len(stack) > 0 {
+					stack = stack[:len(stack)-1]
+				}
+				stmt.Reset()
+			case c == ';':
+				stmt.Reset()
+			default:
+				stmt.WriteByte(c)
+			}
+		}
+	}
+	return out
+}
+
+// shortLabel tidies the text before a { for a hunk header.
+func shortLabel(s string) string {
+	s = strings.Join(strings.Fields(s), " ")
+	if len(s) > 60 {
+		s = s[:57] + "..."
+	}
+	return s
+}

internal/engine/diffcontext_test.go

@@ -0,0 +1,63 @@
+package engine
+
+import (
+	"strings"
+	"testing"
+)
+
+func TestLabelHunks(t *testing.T) {
+	old := `// header
+options {
+	directory "/var/cache/bind";
+	forwarders { 1.1.1.1; 9.9.9.9; };
+	listen-on { any; };
+	recursion no;
+	version "a { b"; # a { in a string and comments: /* {
+	dnssec-validation auto;
+};
+
+view "internal" {
+	match-clients { 10.0.0.0/8; };
+	recursion yes;
+	allow-transfer { localhost; };
+	/* a { comment
+	   over lines */
+	zone "example.com" {
+		type primary;
+		file "example.com.db";
+		notify yes;
+	};
+	zone "other.example" { type primary; file "o.db"; };
+};
+`
+	new := strings.Replace(old, "\tforwarders { 1.1.1.1; 9.9.9.9; };\n", "", 1)
+	new = strings.Replace(new, "\tallow-transfer { localhost; };\n", "\tallow-transfer { localhost; };\n\tforward first;\n\tforwarders { 1.1.1.1; 9.9.9.9; };\n", 1)
+	new = strings.Replace(new, "\t\tnotify yes;\n", "\t\tnotify no;\n", 1)
+	d := fileDiff("/etc/bind/named.conf", []byte(old), true, []byte(new), false, false)
+	var headers []string
+	for _, l := range strings.Split(d, "\n") {
+		if strings.HasPrefix(l, "@@") {
+			headers = append(headers, l)
+		}
+	}
+	want := []string{
+		`@@ -1,7 +1,6 @@ options {`,
+		`@@ -12,12 +11,14 @@ view "internal" {`,
+	}
+	if strings.Join(headers, "\n") != strings.Join(want, "\n") {
+		t.Errorf("headers:\n%s\n\ndiff:\n%s", strings.Join(headers, "\n"), d)
+	}
+
+	// The first change of a hunk decides, and a change inside a zone
+	// names both blocks.
+	new2 := strings.Replace(old, "\t\tnotify yes;\n", "\t\tnotify no;\n", 1)
+	d = fileDiff("/etc/bind/named.conf", []byte(old), true, []byte(new2), false, false)
+	if !strings.Contains(d, ` @@ view "internal" { zone "example.com" {`+"\n") {
+		t.Errorf("zone change:\n%s", d)
+	}
+	// Zone files aren't named.conf syntax.
+	d = fileDiff("/var/lib/bind/example.com.db", []byte(old), true, []byte(new2), false, false)
+	if strings.Contains(d, "@@ view") {
+		t.Errorf("zone file labelled:\n%s", d)
+	}
+}

internal/model/model.go

@@ -506,7 +506,9 @@ func (s *State) ServedRecordsByZone() map[int64][]*Record {
 	return out
 }
 
-// ScopedOptions returns the options of a scope, in position order.
+// ScopedOptions returns the options of a scope: in a fixed order by name
+// for a block (see optionLess), in the order they were added for
+// top-level statements, which are written as entered.
 func (s *State) ScopedOptions(scope string, scopeID int64) []*Option {
 	var out []*Option
 	for _, o := range s.Options {
@@ -514,6 +516,10 @@ func (s *State) ScopedOptions(scope string, scopeID int64) []*Option {
 			out = append(out, o)
 		}
 	}
+	if scope != ScopeTopLevel {
+		sort.Slice(out, func(i, j int) bool { return optionLess(out[i], out[j]) })
+		return out
+	}
 	sort.Slice(out, func(i, j int) bool {
 		if out[i].Position != out[j].Position {
 			return out[i].Position < out[j].Position

internal/model/optorder.go

@@ -0,0 +1,55 @@
+package model
+
+// optionGroups orders the options of a block (options, a view, a zone) the
+// same way everywhere: in named.conf, the web UI and the CLI. Related
+// options are grouped; within a group they are alphabetical, and options
+// not listed here come last, alphabetically. named doesn't care about the
+// order of options in a block.
+var optionGroups = [][]string{
+	// Files and identity.
+	{"bindkeys-file", "directory", "dump-file", "hostname", "managed-keys-directory", "memstatistics-file", "pid-file", "recursing-file", "secroots-file", "server-id", "session-keyfile", "statistics-file", "version"},
+	// Listening and source addresses.
+	{"listen-on", "listen-on-v6", "notify-source", "notify-source-v6", "query-source", "query-source-v6", "transfer-source", "transfer-source-v6"},
+	// Access control.
+	{"allow-query", "allow-query-cache", "allow-query-cache-on", "allow-query-on", "allow-recursion", "allow-recursion-on", "allow-update", "allow-update-forwarding", "blackhole", "deny-answer-addresses", "deny-answer-aliases", "update-policy"},
+	// Recursion, forwarding and the cache.
+	{"forward", "forwarders", "max-cache-size", "max-cache-ttl", "max-ncache-ttl", "minimal-responses", "prefetch", "qname-minimization", "rate-limit", "recursion", "response-policy", "serve-stale", "stale-answer-enable", "stale-answer-ttl", "stale-cache-enable"},
+	// DNSSEC.
+	{"dnssec-accept-expired", "dnssec-must-be-secure", "dnssec-policy", "dnssec-validation", "inline-signing", "key-directory", "trust-anchor-telemetry", "validate-except"},
+	// Transfers and notify.
+	{"allow-notify", "allow-transfer", "also-notify", "catalog-zones", "ixfr-from-differences", "masters", "max-transfer-idle-in", "max-transfer-idle-out", "max-transfer-time-in", "max-transfer-time-out", "notify", "notify-to-soa", "primaries", "provide-ixfr", "request-ixfr", "serial-query-rate", "transfer-format", "transfers-in", "transfers-out", "transfers-per-ns"},
+}
+
+var optionGroup = func() map[string]int {
+	m := map[string]int{}
+	for i, g := range optionGroups {
+		for _, n := range g {
+			m[n] = i
+		}
+	}
+	return m
+}()
+
+// optionLess orders two options of the same block: by group, then name,
+// then (for an option given more than once, like listen-on) the order they
+// were added in.
+func optionLess(a, b *Option) bool {
+	ga, ok := optionGroup[a.Name]
+	if !ok {
+		ga = len(optionGroups)
+	}
+	gb, ok := optionGroup[b.Name]
+	if !ok {
+		gb = len(optionGroups)
+	}
+	if ga != gb {
+		return ga < gb
+	}
+	if a.Name != b.Name {
+		return a.Name < b.Name
+	}
+	if a.Position != b.Position {
+		return a.Position < b.Position
+	}
+	return a.ID < b.ID
+}

internal/model/optorder_test.go

@@ -0,0 +1,40 @@
+package model
+
+import (
+	"strings"
+	"testing"
+)
+
+func TestScopedOptionsOrder(t *testing.T) {
+	s := NewState()
+	add := func(scope string, names ...string) {
+		for i, n := range names {
+			id := int64(len(s.Options) + 1)
+			s.Options[id] = &Option{ID: id, Scope: scope, Name: n, Position: i}
+		}
+	}
+	// Added in an arbitrary order; listen-on twice.
+	add(ScopeView, "zzz-unknown", "forwarders", "listen-on", "recursion", "dnssec-validation", "aaa-unknown", "allow-query", "forward", "directory", "listen-on", "allow-transfer")
+	var got []string
+	for _, o := range s.ScopedOptions(ScopeView, 0) {
+		got = append(got, o.Name)
+	}
+	want := "directory listen-on listen-on allow-query forward forwarders recursion dnssec-validation allow-transfer aaa-unknown zzz-unknown"
+	if strings.Join(got, " ") != want {
+		t.Errorf("order:\n got %s\nwant %s", strings.Join(got, " "), want)
+	}
+	// An option given twice keeps the order it was added in.
+	l := s.ScopedOptions(ScopeView, 0)
+	if l[1].Position > l[2].Position {
+		t.Error("repeated listen-on swapped")
+	}
+	// Top-level statements stay as entered.
+	add(ScopeTopLevel, "logging", "acl", "key")
+	got = nil
+	for _, o := range s.ScopedOptions(ScopeTopLevel, 0) {
+		got = append(got, o.Name)
+	}
+	if strings.Join(got, " ") != "logging acl key" {
+		t.Errorf("top level: %v", got)
+	}
+}