Skip to content

Commit

Permalink
Merge pull request #7 from godot-rust/feature/markdown-lint
Browse files Browse the repository at this point in the history
Format/lint using markdownlint
  • Loading branch information
Bromeon authored Oct 11, 2023
2 parents d340637 + c983445 commit fa383c2
Show file tree
Hide file tree
Showing 15 changed files with 322 additions and 152 deletions.
70 changes: 70 additions & 0 deletions .github/other/.markdownlint.jsonc
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
// https://github.com/DavidAnson/markdownlint/blob/main/doc/Rules.md

{
"default": true,
"heading-style": {
"style": "atx"
},
// Unordered list bullets: - + *.
"ul-style": {
"style": "dash"
},
"ul-indent": {
"indent": 2
},
// Lists 1. 2. 3. or 1. 1. 1.
"ol-prefix": {
"style": "ordered"
},
"no-hard-tabs": {
"code_blocks": true,
"ignore_code_languages": ["gdscript"],
"spaces_per_tab": 4
},
"no-multiple-blanks": {
// Must not be smaller to allow for "blanks-around-headers" minimum.
"maximum": 2
},
"blanks-around-headings": {
"lines_above": 2,
"lines_below": 1
},
"line-length": {
"line_length": 150,
"strict": true,
"code_blocks": true,
// Code same line, due to admonish blocks. Real code should be max 100, but we can't check this.
"code_block_line_length": 150,
"tables": false
},
// Punct at end of titles
"no-trailing-punctuation": {
"punctuation": ".,;:!。,;:!"
},
// Code blocks surrounded by empty lines.
"blanks-around-fences": {
// Except when part of a list (to allow "tight lists")
"list_items": false
},
"proper-names": {
"names": ["Godot", "GDScript", "Rust", "GDExtension", "gdext", "godot-rust"],
"code_blocks": false
},
// Indented vs ```-fenced.
"code-block-style": {
"style": "fenced"
},
"code-fence-style": {
// Would allow "tilde" for ~~~rs style.
"style": "backtick"
},
"emphasis-style": {
"style": "underscore"
},
"strong-style": {
"style": "asterisk"
}
// If [links] inside code blocks are not recognized, and --fix removes footers, disable this.
// (Links in code blocks are required by our admonish plugin).
// "reference-links-images": false
}
24 changes: 21 additions & 3 deletions .github/workflows/cd.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# License, v. 2.0. If a copy of the MPL was not distributed with this
# file, You can obtain one at https://mozilla.org/MPL/2.0/.

name: "Deploy to GitHub Pages"
name: "Book CI"

on:
push:
Expand All @@ -19,8 +19,7 @@ env:

# Sets permissions of the GITHUB_TOKEN to allow deployment to GitHub Pages
permissions:
# contents: read
contents: write
contents: read
pages: write
id-token: write

Expand Down Expand Up @@ -64,6 +63,25 @@ jobs:
uses: actions/deploy-pages@v2


markdown-lint:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3

- name: "Install markdownlint-cli2"
run: npm install -g markdownlint-cli2

- name: "Run lint"
run: ./lint.sh

# Disabled because behavior differs slightly from markdownlint (cli)
# - name: "Run lint"
# uses: docker://avtodev/markdown-lint:v1 # fastest way; alternative: avto-dev/markdown-lint@v1
# with:
# args: ReadMe.md "src/**/*.md"
# config: .github/other/.markdownlint.jsonc


license-guard:
runs-on: ubuntu-20.04
steps:
Expand Down
31 changes: 23 additions & 8 deletions ReadMe.md
Original file line number Diff line number Diff line change
@@ -1,19 +1,33 @@
# The godot-rust book

The godot-rust book is a user guide for **gdext**, the Rust bindings to Godot 4. The book is still work-in-progress, and contributions are very welcome.
The godot-rust book is a user guide for **gdext**, the Rust bindings to Godot 4.
The book is still work-in-progress, and contributions are very welcome.

An online version of the book is available at [godot-rust.github.io/book][book-web].
An online version of the book is available at [godot-rust.github.io/book][book-web].
For the gdnative book, check out [gdnative-book].

The book is built with [mdBook] and the plugins [mdbook-toc] and [mdbook-admonish]. To install them and build the book locally, you can run:

```bash
$ cargo install mdbook mdbook-toc mdbook-admonish
$ mdbook build
cargo install mdbook mdbook-toc mdbook-admonish
mdbook build
```

To run a local server with automatic updates while editing the book, use:

```bash
mdbook serve --open
```


## Formatting and linting

We use [markdownlint] to enforce a consistent style across the Markdown files.
It is automatically run during CI, but if you have the `npm` toolchain, you can also run it locally:

```bash
$ mdbook serve --open
npm install --global markdownlint-cli2
./lint.sh
```


Expand All @@ -28,9 +42,10 @@ Please read the corresponding contributing guidelines in `Contributing.md`.
Like gdext itself, the gdext book is licensed under [MPL 2.0][mpl].

[book-web]: https://godot-rust.github.io/book
[gdext]: https://github.com/godot-rust/gdext
[gdnative-book]: https://github.com/godot-rust/gdnative-book
[mdBook]: https://github.com/rust-lang-nursery/mdBook
[mdbook-toc]: https://github.com/badboy/mdbook-toc
[markdownlint]: https://github.com/DavidAnson/markdownlint
[mdbook-admonish]: https://github.com/tommilligan/mdbook-admonish
[gdext]: https://github.com/godot-rust/gdext
[mdbook-toc]: https://github.com/badboy/mdbook-toc
[mdBook]: https://github.com/rust-lang-nursery/mdBook
[mpl]: https://www.mozilla.org/en-US/MPL
39 changes: 39 additions & 0 deletions lint.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
#!/bin/bash
# This Source Code Form is subject to the terms of the Mozilla Public
# License, v. 2.0. If a copy of the MPL was not distributed with this
# file, You can obtain one at https://mozilla.org/MPL/2.0/.

set -e

show_help() {
echo "Validate Markdown in the godot-rust book."
echo "Usage: $0 [fix]"
echo ""
echo " fix: Automatically apply fixes where possible"
echo " --help: Show this help menu"
echo ""
echo "Requires markdownlint-cli2 to be installed. You can do so as follows:"
echo " npm install markdownlint-cli2 --global"
}

# If 'fix' is provided, append '--fix' to the end of the command
if [[ "$1" == "fix" ]]; then
extra="--fix"
elif [[ "$1" == "help" ]] || [[ "$1" == "--help" ]]; then
show_help
exit 0
elif [[ -z "$1" ]]; then
extra=""
else
echo "Incorrect usage!"
show_help
exit 1
fi

# Could also use markdownlint-cli (command 'markdownlint') instead. Has more functionality, but the npm package is a bit heavier.

# shellcheck disable=SC2086
# do not quote $extra, it shouldn't be an argument if empty.
# do quote glob pattern, shell expands differently than tool itself.
# keep in sync with CI arguments.
markdownlint-cli2 --config .github/other/.markdownlint.jsonc ReadMe.md "src/**/*.md" $extra
94 changes: 47 additions & 47 deletions src/contribute/conventions.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,17 @@ pull request reviews.

In particular, we use the following tools:

* [**rustfmt**] for code formatting ([config options][rustfmt-config]).
* [**clippy**] for lints and style warnings ([list of lints][clippy-lints]).
* Clang's [**AddressSanitizer**] and [**LeakSanitizer**] for memory safety.
* Various specialized tools:
* [**skywalking-eyes**] to enforce license headers.
* [**cargo-deny**] and [**cargo-machete**] for dependency verification.

In addition, we have unit tests (`#[test]`), doctests and Godot integration tests (`#[itest]`).
- [**rustfmt**] for code formatting ([config options][rustfmt-config]).
- [**clippy**] for lints and style warnings ([list of lints][clippy-lints]).
- Clang's [**AddressSanitizer**] and [**LeakSanitizer**] for memory safety.
- Various specialized tools:
- [**skywalking-eyes**] to enforce license headers.
- [**cargo-deny**] and [**cargo-machete**] for dependency verification.

In addition, we have unit tests (`#[test]`), doctests and Godot integration tests (`#[itest]`).
See [Dev tools] for more information.

[**AddressSanitizer**]: https://clang.llvm.org/docs/AddressSanitizer.html
[**AddressSanitizer**]: https://clang.llvm.org/docs/AddressSanitizer.html
[**cargo-deny**]: https://embarkstudios.github.io/cargo-deny
[**cargo-machete**]: https://github.com/bnjbvr/cargo-machete
[**clippy**]: https://doc.rust-lang.org/stable/clippy/usage.html
Expand All @@ -40,35 +40,35 @@ See [Dev tools] for more information.

## API design principles

Different Rust libraries have different goals, which is reflected in API design choices. gdext is written in Rust, but interoperates with
Different Rust libraries have different goals, which is reflected in API design choices. gdext is written in Rust, but interoperates with
a C++/GDScript game engine, which means that we sometimes need to go unconventional ways to achieve good user experiences.
This may sometimes be surprising for Rust developers.

We envision the following core principles as a guideline for API design:

1. **Simplicity**
Prefer self-explanatory, straightforward interfaces.
* Avoid abstractions that don't add value to the user.
- Avoid abstractions that don't add value to the user.
Do not over-engineer prematurely just because it's possible; follow [YAGNI][wiki-yagni]. Avoid [premature optimization][wiki-premature-opt].
* Examples to avoid: traits that are not used polymorphically, type-state pattern, many generic parameters,
- Examples to avoid: traits that are not used polymorphically, type-state pattern, many generic parameters,
layers of wrapper types/functions that simply delegate logic.
* Sometimes, runtime errors are better than compile-time errors. Most users are building a game, where fast iteration is key.
Use `Option`/`Result` when errors are recoverable, and panics when the user must fix their code.
- Sometimes, runtime errors are better than compile-time errors. Most users are building a game, where fast iteration is key.
Use `Option`/`Result` when errors are recoverable, and panics when the user must fix their code.
See also [Ergonomics and panics][lib-ergonomics-panics].

2. **Maintainability**
Every line of code added **must be maintained, potentially indefinitely**.
* Consider that it may not be you working with it in the future, but another contributor or maintainer, maybe a year from now.
* Try to see the bigger picture -- how important is a specific in the overall library? How much detail is necessary?
- Consider that it may not be you working with it in the future, but another contributor or maintainer, maybe a year from now.
- Try to see the bigger picture -- how important is a specific in the overall library? How much detail is necessary?
Balance the amount of code with its real-world impact for users.
* Document non-trivial thought processes and design choices as inline `//` comments.
* Document behavior, invariants and limitations in `///` doc comments.
- Document non-trivial thought processes and design choices as inline `//` comments.
- Document behavior, invariants and limitations in `///` doc comments.

3. **Consistency**
As a user, having a uniform experience when using different parts of the library is important.
This reduces the cognitive load of learning and using the library, requires less doc lookup and makes users more efficient.
* Look at existing code and try to understand its patterns and conventions.
* Before doing larger refactorings or changes of existing systems, try to understand the underlying design choices.
- Look at existing code and try to understand its patterns and conventions.
- Before doing larger refactorings or changes of existing systems, try to understand the underlying design choices.

See these as guidelines, not hard rules. If you are unsure, please don't hesitate to ask questions and discuss different ideas.

Expand Down Expand Up @@ -97,20 +97,20 @@ We use separators starting with `// ---` to visually divide sections of related
### Code organization

1. Anything that is not intended to be accessible by the user, but must be `pub` for technical reasons, should be marked as `#[doc(hidden)]`.
* This does [**not** constitute part of the public API][lib-public-api].
- This does [**not** constitute part of the public API][lib-public-api].

1. We do not use the `prelude` inside the project, except in examples and doctests.
2. We do not use the `prelude` inside the project, except in examples and doctests.

1. Inside `impl` blocks, we _roughly_ try to follow the order:
* Type aliases in traits (`type`)
* Constants (`const`)
* Constructors and associated functions
* Public methods
* Private methods (`pub(crate)`, private, `#[doc(hidden)]`)
3. Inside `impl` blocks, we _roughly_ try to follow the order:
- Type aliases in traits (`type`)
- Constants (`const`)
- Constructors and associated functions
- Public methods
- Private methods (`pub(crate)`, private, `#[doc(hidden)]`)

1. Inside files, there is no strict order yet, except `use` and `mod` at the top. Prefer to declare public-facing symbols before private ones.
4. Inside files, there is no strict order yet, except `use` and `mod` at the top. Prefer to declare public-facing symbols before private ones.

1. Use flat import statements. If multiple paths have different prefixes, put them on separate lines. Avoid `self`.
5. Use flat import statements. If multiple paths have different prefixes, put them on separate lines. Avoid `self`.
```rs
// Good:
use crate::module;
Expand All @@ -126,23 +126,23 @@ We use separators starting with `// ---` to visually divide sections of related

1. Avoid tuple-enums `enum E { Var(u32, u32) }` and tuple-structs `struct S(u32, u32)` with more than 1 field. Use named fields instead.

1. Derive order is `#[derive(GdextTrait, ExternTrait, Default, Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug)]`.
* `GdextTrait` is a custom derive defined by gdext itself (in any of the crates).
* `ExternTrait` is a custom derive by a third-party crate, e.g. `nanoserde`.
* The standard traits follow order _construction, comparison, hashing, debug display_.
2. Derive order is `#[derive(GdextTrait, ExternTrait, Default, Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug)]`.
- `GdextTrait` is a custom derive defined by gdext itself (in any of the crates).
- `ExternTrait` is a custom derive by a third-party crate, e.g. `nanoserde`.
- The standard traits follow order _construction, comparison, hashing, debug display_.
More expressive ones (`Copy`, `Eq`) precede their implied counterparts (`Clone`, `PartialEq`).


### Functions

1. Getters don't have a `get_` prefix.

1. Use `self` instead of `&self` for `Copy` types, unless they are really big (such as `Transform3D`).
2. Use `self` instead of `&self` for `Copy` types, unless they are really big (such as `Transform3D`).

1. For `Copy` types, avoid in-place mutation `vector.normalize()`.
3. For `Copy` types, avoid in-place mutation `vector.normalize()`.
Instead, use `vector = vector.normalized()`. The past tense indicates a copy.
1. Annotate with `#[must_use]` when ignoring the return value is likely an error.

4. Annotate with `#[must_use]` when ignoring the return value is likely an error.
Example: builder APIs.


Expand All @@ -151,18 +151,18 @@ We use separators starting with `// ---` to visually divide sections of related
Concerns both `#[proc_macro_attribute]` and the attributes attached to a `#[proc_macro_derive]`.

1. Attributes always have the same syntax: `#[attr(key = "value", key2, key_three = 20)]`
* `attr` is the outer name grouping different key-value pairs in parentheses.
- `attr` is the outer name grouping different key-value pairs in parentheses.
A symbol can have multiple attributes, but they cannot share the same name.
* `key = value` is a key-value pair. just `key` is a key-value pair without a value.
* Keys are always `snake_case` identifiers.
* Values are typically strings or numbers, but can be more complex expressions.
* Multiple key-value pairs are separated by commas. Trailing commas are allowed.
- `key = value` is a key-value pair. just `key` is a key-value pair without a value.
- Keys are always `snake_case` identifiers.
- Values are typically strings or numbers, but can be more complex expressions.
- Multiple key-value pairs are separated by commas. Trailing commas are allowed.

2. In particular, avoid these forms:
* `#[attr = "value"]` (top-level assignment)
* `#[attr("value")]` (no key -- note that `#[attr(key)]` is allowed)
* `#[attr(key(value))]`
* `#[attr(key = value, key = value)]` (repeated keys)
- `#[attr = "value"]` (top-level assignment)
- `#[attr("value")]` (no key -- note that `#[attr(key)]` is allowed)
- `#[attr(key(value))]`
- `#[attr(key = value, key = value)]` (repeated keys)

The reason for this choice is that each attribute maps nicely to a map, where values can have different types.
This allows for a recognizable and consistent syntax across all proc-macro APIs. Implementation-wise, this pattern is
Expand Down
Loading

0 comments on commit fa383c2

Please sign in to comment.