Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add pre-commit hook #565

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
21 changes: 21 additions & 0 deletions .pre-commit-hooks.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
---
- id: selene-system
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rust-based pre-commit does not work because pre-commit hardcodes the --path to . (source) which leads to build failure with error: found a virtual manifest at <path>/Cargo.toml instead of a package manifest.

Pre-commit maintainer disagrees with making --path configurable so it is not possible to add a rust based hook: pre-commit/pre-commit#2931 unless we somehow make it possible to install the binary from root level.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's possible to point entry to a custom install script. But we'd need to handle the caching, unless we distribute the executable directly.

Copy link
Author

@amitds1997 amitds1997 Oct 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was unsure if we wanted a custom install script just for the pre-commit use case. I can add it if that's something we are okay with.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would depend on how it would look like. Caching binaries by version may not be trivial and we don't get the same advantages language: rust gets, which makes me think it's likely not worth the effort to support this.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree. That's why went ahead with only setting up system and docker workflows. Is there some trivial way to make this work by may be a configuration change at our end? I did some preliminary research but did not find much (but then I have not worked with rust ever before).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you looked at how StyLua handles the pre-commit hook?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Their rust workflow works because they do not use workspaces.

The python workflow seems interesting. We can integrate it, but it would depend on a python package (https://pypi.org/project/release-gitter/) to do the release download for us. If that is something we are okay with, I can add it 👍

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added GitHub release based pre-commit hook in 5f1a755

name: selene (system)
description: An opinionated Lua code linter
entry: selene
language: system
types: [lua]

- id: selene-docker
name: selene (docker)
description: An opinionated Lua code linter
entry: /selene
language: docker
types: [lua]

- id: selene-github
name: selene (GitHub)
description: An opinionated Lua code linter
entry: selene
language: python
types: [lua]
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- Added `Path2DControlPoint.new` to the Roblox standard library
- [Adds `lua_versions` to standard library definitions](https://kampfkarren.github.io/selene/usage/std.html#lua_versions). Specifying this will only allow the syntax used by those languages. The default standard libraries now specify these, meaning that invalid syntax for that language will no longer be supported.
- Added missing third parameter to `PathWaypoint.new` in the Roblox standard library
- Added support for selene as a `pre-commit` hook

### Changed
- Upgrades to [full-moon 1.0.0](https://github.com/Kampfkarren/full-moon/blob/main/CHANGELOG.md#100---2024-10-08), which should provide faster parse speeds, support for multiple parsing errors at the same time, and support for some new Luau syntax.
Expand Down
8 changes: 4 additions & 4 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ FROM bash AS selene-light
COPY --from=selene-light-builder /usr/local/cargo/bin/selene /
CMD ["/selene"]

FROM bash AS selene-musl
COPY --from=selene-musl-builder /usr/local/cargo/bin/selene /
CMD ["/selene"]

FROM bash AS selene-light-musl
COPY --from=selene-light-musl-builder /usr/local/cargo/bin/selene /
CMD ["/selene"]

FROM bash AS selene-musl
COPY --from=selene-musl-builder /usr/local/cargo/bin/selene /
CMD ["/selene"]
3 changes: 2 additions & 1 deletion docs/src/SUMMARY.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
- [Configuration](./usage/configuration.md)
- [Filtering](./usage/filtering.md)
- [Standard Library Format](./usage/std.md)
- [Git hooks (pre-commit)](./usage/git-hooks-pre-commit.md)
- [Roblox Guide](./roblox.md)
- [Contributing](./contributing.md)
- [Lints](./lints/index.md)
Expand Down Expand Up @@ -42,4 +43,4 @@
- [unscoped_variables](./lints/unscoped_variables.md)
- [unused_variable](./lints/unused_variable.md)
- [Archive](./archive/index.md)
- [TOML Standard Library Format](./archive/std_v1.md)
- [TOML Standard Library Format](./archive/std_v1.md)
35 changes: 35 additions & 0 deletions docs/src/usage/git-hooks-pre-commit.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# Pre-commit

`pre-commit` allows integration of `selene` into your Git workflow using git hooks. After [installing pre-commit](https://pre-commit.com/#install), add one of the following configurations to your `.pre-commit-config.yaml` file:

* Use the `selene` binary present on the system path (Should be pre-installed):

```yaml
repos:
- repo: https://github.com/Kampfkarren/selene
rev: ''
hooks:
- id: selene-system
```

* Use `selene` through GitHub releases:

```yaml
repos:
- repo: https://github.com/Kampfkarren/selene
rev: ''
hooks:
- id: selene-github
```

* Use the `selene` binary present in the `selene` docker image (Since this uses docker, it might take some time to bootstrap and is slower than the other two options):

```yaml
repos:
- repo: https://github.com/Kampfkarren/selene
rev: ''
hooks:
- id: selene-docker
```

You may see a `warning` being generated when pre-commit runs. To resolve that, set the `rev` key to any selene tag or commit, for e.g. `rev: '0.26.2'`.
9 changes: 9 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[build-system]
requires = ["release-gitter[builder]"]
build-backend = "pseudo_builder"

[tool.release-gitter]
git-url = "https://github.com/Kampfkarren/selene"
extract-files = ["selene"]
format = "selene-{version}-{system}.zip"
exec = "chmod +x selene"