Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# What an editor should do before anybody runs anything. This file is a request
# and not the rule: internal/format is what refuses a tracked file breaking one
# of the three properties below, and gofmt is what decides Go.
#
# The values are the tree as it already is rather than a preference imposed on
# it. Anything here that no check reads is a courtesy to an editor and nothing
# more.

root = true

[*]
charset = utf-8
end_of_line = lf
insert_final_newline = true
trim_trailing_whitespace = true
indent_style = space
indent_size = 4

# gofmt indents with tabs and is the authority for Go formatting, so this
# follows it rather than arguing with it.
[*.go]
indent_style = tab
indent_size = 4

# The workflow files and the site are two-space by convention in their own
# ecosystems, and the tree already follows both.
[*.{yml,yaml}]
indent_size = 2

[*.{html,css,js}]
indent_size = 2

# The manifest's own indentation is four spaces and it is fixed by
# decisions/manifest-schema.md rather than by taste, so this agrees with the
# encoder rather than with a habit.
[*.json]
indent_size = 4

# Markdown is prose, so an indent size is meaningless in it. A trailing space is
# not: two of them are a hard line break, which is a real construct somebody may
# want, and the check refuses one anyway. Use a backslash or a blank line, and
# there is one place to argue with that rather than two.
[*.md]
indent_size = 4
15 changes: 11 additions & 4 deletions .gitattributes
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
# What git stores, which is what every check reads and what every reviewer sees.
#
# Line endings are pinned here rather than left to core.autocrlf, so a file's
# stored bytes do not depend on the checkout setting of whoever committed it.
# The generated manifest and the golden fixture it is compared against are
# judged byte for byte, so their line endings cannot depend on the checkout
# setting of whoever ran the test. Pinned here rather than left to core.autocrlf.
# #23 is where this widens to the rest of the tree.
*.json text eol=lf
# judged byte for byte and would be the first casualty, but the reason is not
# theirs alone: a rule that only holds for one file type holds by luck.
#
# A working copy may still have CRLF, and that is fine. internal/format
# normalises before judging, so the formatting leg gives the same verdict on
# either checkout.
* text=auto eol=lf
16 changes: 15 additions & 1 deletion .github/workflows/gate.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# The merge gate: build, test, format.
# The merge gate. The jobs below are the whole of it, and `go run .` names the
# same legs from a shell, so neither list is written out in prose that drifts.
#
# One job per leg, and each job runs the same entry point a contributor runs
# before pushing. A job is not a place where a decision is taken here; every
Expand Down Expand Up @@ -95,6 +96,19 @@ jobs:
go-version-file: go.mod
- run: go run . gate format

editorconfig:
name: "Gate: editorconfig"
runs-on: ubuntu-latest
timeout-minutes: 10
steps:
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
persist-credentials: false
- uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0
with:
go-version-file: go.mod
- run: go run . gate editorconfig

tests-reach-nothing:
name: "Gate: tests-reach-nothing"
runs-on: ubuntu-latest
Expand Down
51 changes: 41 additions & 10 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,36 @@ ones that arrive with it.

The site under `docs/` still opens in a browser with no build step.

### Formatting

gofmt -w .

That is the whole format command, and on a tree that is already formatted it
changes nothing. gofmt decides Go. Everything else is decided by three
properties `.editorconfig` states and `internal/format` refuses: a final
newline, no trailing whitespace, and no tab indent outside Go.

go test ./internal/format -count=1

is what refuses them. The gate runs the two halves as two legs, `format` and
`editorconfig`, so a red says which of them it was without anybody opening a
log. There is no formatter for the HTML, the YAML or the prose, because one
would be a runtime this tree does not carry, and `.editorconfig` is what an
editor reads instead.

Line endings do not decide the verdict, and the two halves get there
differently. `internal/format` normalises before judging, so it answers the same
on either checkout, and a test holds that property rather than a sentence
claiming it. gofmt does not normalise: it lists a Go file whose working copy has
CRLF, on any platform. What keeps that from being a Windows-only red is
`.gitattributes`, which pins every tracked file to LF in the working copy as
well as in the object store, so a fresh clone is LF whatever `core.autocrlf`
says locally.

If gofmt lists a file you have not touched, that is the symptom: the working
copy predates `.gitattributes`. Deleting the file and checking it out again
fixes it, and nothing about the file's content was wrong.

### The dependency set is empty, and empty is not unlocked

There is no `go.sum` in the tree, because nothing is required yet. A build does
Expand Down Expand Up @@ -67,9 +97,9 @@ Its legs run in order, they stop at the first failure, and the run ends by sayin
how many of them it examined, so a run that covered two of three cannot be read
as one that covered three and found nothing.

The legs are build, test and format, which is what `decisions/means.md` settles
them as. Which legs exist is printed rather than restated here, because a list in
this file drifts against the one the command runs:
What each leg is made of comes from `decisions/means.md`. Which legs exist is
printed rather than restated here, because a list in this file drifts against the
one the command runs:

go run .

Expand All @@ -83,10 +113,10 @@ because `build`, `deploy` and `report-build-status` are already taken on `main`
by the Pages deployment, which has no file in this tree; `internal/gate` holds
the prefix, and the suite refuses a leg with no job and a job with no leg.

On a clone where the working copy has CRLF line endings, the format leg lists Go
files you have not touched. The content is not wrong: gofmt does not normalise
before judging, and only `*.json` is pinned to LF today. #23 is where that pin
widens to the rest of the tree.
On a clone made before `.gitattributes` pinned the whole tree to LF, the format
leg lists Go files you have not touched. The content is not wrong: gofmt does not
normalise before judging, and the working copy still holds the carriage returns
the old checkout wrote. The Formatting section above says what fixes it.

## Where things live

Expand Down Expand Up @@ -114,9 +144,10 @@ the comment at the top of its own file:

ls .github/workflows

They are supply-chain and hygiene checks. None of them builds or tests anything,
because there is nothing yet to build or test. That is worth knowing before you
read a green check as the tree being verified.
`gate.yml` is the one that compiles and tests this repository, one job per leg
of the entry point above. The rest are supply-chain and hygiene checks that build
and test nothing, which is worth knowing before you read one of their greens as
the tree being verified.

## Sign your commits

Expand Down
144 changes: 144 additions & 0 deletions internal/format/format.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
// Package format holds the whitespace rules .editorconfig states and the check
// that refuses a tracked file breaking one.
//
// The rules are here rather than in a workflow step because an editor
// configuration is a request and this is the refusal, and because a refusal
// that cannot be run against a planted input cannot be shown to bite. Nothing
// outside the standard library is needed to decide any of them.
//
// gofmt is not re-implemented here. It ships with the toolchain, it already
// decides Go formatting, and decisions/means.md names it. What this adds is the
// part gofmt does not reach: the HTML, the JSON, the workflow YAML and the
// prose, which are most of the tree.
package format

import (
"bytes"
"fmt"
"os"
"os/exec"
"path"
"path/filepath"
"strings"
)

// Finding is one broken rule at one place. Line is 1-based, or 0 when the
// finding is about the file as a whole.
type Finding struct {
Path string
Line int
Rule string
Detail string
}

func (f Finding) String() string {
if f.Line == 0 {
return fmt.Sprintf("%s: %s: %s", f.Path, f.Rule, f.Detail)
}
return fmt.Sprintf("%s:%d: %s: %s", f.Path, f.Line, f.Rule, f.Detail)
}

// Rule names, which are what a failure prints. They are constants so a message
// and a test cannot drift apart.
const (
RuleFinalNewline = "final-newline"
RuleTrailingWhitespace = "trailing-whitespace"
RuleTabIndent = "tab-indent"
)

// CheckFile judges one file's bytes under the rules for its path.
//
// CRLF is normalised to LF before anything is judged, so the verdict does not
// depend on the checkout that produced the bytes. That is deliberate and it is
// the reason this check can run on Windows and on Linux and agree: what is
// stored is fixed by .gitattributes, and a contributor whose working copy has
// CRLF is not told their tree is broken. A lone carriage return is not
// normalised and not judged; nothing in this tree has one.
//
// A file containing a NUL byte is treated as binary and judged not at all,
// because these rules are about text and a false finding on a binary file is
// how a check gets switched off.
func CheckFile(name string, content []byte) []Finding {
if bytes.IndexByte(content, 0) >= 0 {
return nil
}
content = bytes.ReplaceAll(content, []byte("\r\n"), []byte("\n"))

var findings []Finding
if len(content) > 0 && content[len(content)-1] != '\n' {
findings = append(findings, Finding{
Path: name,
Rule: RuleFinalNewline,
Detail: "the file does not end with a newline",
})
}

tabsAllowed := tabIndented(name)
lines := strings.Split(strings.TrimSuffix(string(content), "\n"), "\n")
for i, line := range lines {
if line == "" {
continue
}
if strings.HasSuffix(line, " ") || strings.HasSuffix(line, "\t") {
findings = append(findings, Finding{
Path: name,
Line: i + 1,
Rule: RuleTrailingWhitespace,
Detail: "the line ends in a space or a tab",
})
}
if !tabsAllowed && strings.HasPrefix(line, "\t") {
findings = append(findings, Finding{
Path: name,
Line: i + 1,
Rule: RuleTabIndent,
Detail: "the line is indented with a tab, and .editorconfig indents this file type with spaces",
})
}
}
return findings
}

// tabIndented says whether .editorconfig indents this path with tabs. Go is the
// only such type in the tree, because gofmt indents with tabs and rewriting it
// would be a fight with the toolchain's own formatter rather than a style.
func tabIndented(name string) bool {
return filepath.Ext(name) == ".go"
}

// TrackedFiles returns the paths git tracks under root, so an untracked scratch
// file in somebody's working copy never reds the check and every file that will
// reach a reviewer does.
func TrackedFiles(root string) ([]string, error) {
cmd := exec.Command("git", "-C", root, "ls-files", "-z")
out, err := cmd.Output()
if err != nil {
return nil, fmt.Errorf("listing tracked files in %s: %w", root, err)
}
var paths []string
for _, p := range strings.Split(string(out), "\x00") {
if p != "" {
paths = append(paths, p)
}
}
return paths, nil
}

// CheckTree judges every tracked file under root. Paths in the findings are the
// slash-separated ones git reports, so a failure reads the same on either
// platform.
func CheckTree(root string) ([]Finding, error) {
paths, err := TrackedFiles(root)
if err != nil {
return nil, err
}
var findings []Finding
for _, p := range paths {
content, err := os.ReadFile(filepath.Join(root, filepath.FromSlash(p)))
if err != nil {
return nil, fmt.Errorf("reading %s: %w", p, err)
}
findings = append(findings, CheckFile(path.Clean(p), content)...)
}
return findings, nil
}
Loading
Loading