Skip to content

Commit

Permalink
Merge pull request #731 from inexorabletash/lint-tool-fixes
Browse files Browse the repository at this point in the history
Update documentation and fix some minor issues with lint tool.
  • Loading branch information
huningxin authored Jul 19, 2024
2 parents c4de3ef + de2a932 commit 6be80a4
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 15 deletions.
3 changes: 3 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ Bug fixes and new content changes should proceed as follows:

2. **Prepare the change in a pull request** and put a reference to the active issue(s) the change is addressing in the description. We prefer that a pull request is represented by a single type of change as outlined in the previous section for a speedy review and approval. Conversely, a specific change should also be captured by a single and not multiple pull requests. This helps to reduce the dependency between pull requests and the chance for the specification to be left in a transient state between multiple pull requests. Exceptions to this should be discussed and approved by the Working Group in one of our bi-weekly calls.

- See [Bikeshed](https://speced.github.io/bikeshed/) for the tooling used for authoring and building the spec. Additionally, a ["lint tool"](tools/lint.mjs) is used to catch common errors and enforce style. GitHub Actions will run these automatically for any pull request that modifies the spec source, but it's helpful to run the tools locally to test and verify your changes.
- When updating your pull request in response to reviewer feedback, *do not squash the commits*. This allows reviewers to inspect only new commits to the pull request on GitHub.

3. **Close the issue** once the pull request is reviewed and merged. Make sure to resolve any error that arises during the merge and check the post-merged published result. The Bikeshed document format isn't very good for an automatic merge, you may need to intervene and manually correct the merge's mistakes if any. You also want to make sure all the GitHub Actions that are put in place to catch document issues are all clean before merging the change into the main branch.

A pull request can be merged when it has received an adequate review from the Working Group. The editors and the chair are responsible for ensuring review is sought from appropriate people. For substantive changes, the adequate review is between two or more reviewers. The bi-weekly teleconference calls provide another venue for providing feedback and reviewing open pull requests.
Expand Down
8 changes: 7 additions & 1 deletion tools/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,13 @@ This directory contains command line utilities intended for use by spec editors

The tools assume a POSIX-like command line environment, and have dependencies on various languages and libraries. Read the sources for more details.

Node.js dependencies can be installed by running `npm install` from within the `tools/` directory.
Node.js dependencies can be installed/updated by running `npm install` from within the `tools/` directory:

```
cd tools
npm install
cd ..
```

Spec editors can use a flow like the following:

Expand Down
28 changes: 14 additions & 14 deletions tools/lint.mjs
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
#!/usr/bin/env node

// Requires Node.js and:
// * npm install node-html-parser
// Requires Node.js; to install dependencies, run these steps:
// cd tools
// npm install
// cd ..
//
// Run script from top level of spec repo after building spec.
// Example: bikeshed spec && node tools/lint.mjs
// Run this script from top level of spec repo after building the spec:
// bikeshed --die-on=warning spec
// node tools/lint.mjs
//
// Note that the '.mjs' extension is necessary for Node.js to treat the file as
// a module. There is an `--experimental-default-type=module` flag but
Expand Down Expand Up @@ -116,14 +119,16 @@ const AsyncFunction = async function() {}.constructor;

log('running checks...');

const ALGORITHM_STEP_SELECTOR = '.algorithm li > p:not(.issue)';

// Checks can operate on:
// * `source` - raw Bikeshed markdown source
// * `html` - HTML source, with style/script removed
// * `text` - rendered text content
// * `root.querySelectorAll()` - operate on DOM-like nodes

// Look for merge markers
for (const match of text.matchAll(/[<=>]{7}/g)) {
for (const match of source.matchAll(/<{7}|>{7}|^={7}$/mg)) {
error(`Merge conflict marker: ${format(match)}`);
}

Expand Down Expand Up @@ -181,8 +186,8 @@ for (const element of root.querySelectorAll('.idl dfn[data-dfn-type=dict-member]
error(`Dictionary member missing dfn: ${element.innerText}`);
}

// Look for [] used in algorithm for anything but issues, indexing, slots, and refs
for (const element of root.querySelectorAll('.algorithm li p:not(.issue)')) {
// Look for [] used in algorithm steps for anything but indexing, slots, and refs
for (const element of root.querySelectorAll(ALGORITHM_STEP_SELECTOR)) {
// Exclude \w[ for indexing (e.g. shape[n])
// Exclude [[ for inner slots (e.g. [[name]])
// Exclude [A for references (e.g. [WEBIDL])
Expand Down Expand Up @@ -270,13 +275,8 @@ for (const pre of root.querySelectorAll('pre.highlight:not(.idl)')) {
}

// Ensure algorithm steps end in '.' or ':'.
for (const p of root.querySelectorAll('.algorithm li > p')) {
let str = p.innerText;

// Strip "[Issue #123]" suffix.
str = str.replace(/\s+\[Issue #\d+\]/, '');

const match = str.match(/[^.:]$/);
for (const p of root.querySelectorAll(ALGORITHM_STEP_SELECTOR)) {
const match = p.innerText.match(/[^.:]$/);
if (match) {
error(`Algorithm steps should end with '.' or ':': ${format(match)}`);
}
Expand Down

0 comments on commit 6be80a4

Please sign in to comment.