Comments (10)
No worries, thanks for reaching out!
from rusty-hook.
Thanks for sharing your perspective.
I want to be mindful about having discussions or debate about one project/tool in the repository of a different project/tool. I happen to be a maintainer of both, so I just wanted to share some tips and food for thought.
However, I'll leave with a few thoughts on the topic:
- You're not the first person that's wanted to use unstable features on stable, and you certainly won't be the last 😄 While I'm personally somewhat sympathetic to this for rustfmt config options, using unstable things on stable runs starkly counter to Rust's core ethos and that's a big part of why the many past requests/arguments have been rejected
- People commonly have negative and incorrect connotations around nightly toolchains. There's certainly contexts where it's understandable that a project may want to require a stable toolchain, but the resultant binary output from a nightly compiler is just as trustworthy with the same guarantees as if it had been built with a stable compiler. The bigger considerations tend to be whether the nightly compiler may fail to compile code, or if/how much rusk one is willing to take on by writing code that is predicated on gated, unstable features (e.g. GATs) which could potentially be removed.
from rusty-hook.
Your script is working fine for me on linux using pre-commit = sh testing-this.sh
, including output and exit code (i added a non-zero exit to the script to confirm).
Would you mind elaborating with more details?
from rusty-hook.
Also, somewhat tangential, but with my rustfmt team lead hat on, I wanted to note that you can't cargo fmt
an individual file, and you'd want to run rustfmt ...
directly
from rusty-hook.
Seems like I was just missing the sh
, sorry about that. Also thanks for rustfmt
advice.
from rusty-hook.
@calebcartwright I was not sure if worth re-opening or a new issue, but I am experiencing perhaps a new error. When making commit on https://github.com/JonathanWoollett-Light/testing-git-hooks which results in pre-commit.sh
error-ing the commits still continue through:
PS C:\Users\jcawl\Projects\testing-git-hooks> git add *
PS C:\Users\jcawl\Projects\testing-git-hooks> git commit -m "test"
Found configured hook: pre-commit
Running command: sh pre-commit.sh
Fetching advisory database from `https://github.com/RustSec/advisory-db.git`
Loaded 417 security advisories (from C:\Users\jcawl\.cargo\advisory-db)
Updating crates.io index
Scanning Cargo.lock for vulnerabilities (13 crate dependencies)
error: expected expression, found keyword `as`
--> \\?\C:\Users\jcawl\Projects\testing-git-hooks\src\main.rs:7:21
|
7 | let _i = 12u32; as as dasd
| ^^ expected expression
warning: unused manifest key: logging
Checking testing-git-hooks v0.1.0 (C:\Users\jcawl\Projects\testing-git-hooks)
error: expected expression, found keyword `as`
--> src\main.rs:7:21
|
7 | let _i = 12u32; as as dasd
| ^^ expected expression
error: could not compile `testing-git-hooks` due to previous error
[master 0093fbe] test
1 file changed, 1 insertion(+), 1 deletion(-)
PS C:\Users\jcawl\Projects\testing-git-hooks>
The respective pre-commit.sh
:
#!/bin/bash
# I don't think there is a reasonable way to do these more specifically
cargo audit
# For every staged file
for i in $(git diff --name-only --cached); do
# Get the extension
filename=$(basename -- "$i")
extension="${filename##*.}"
if [ "$extension" = "rs" ]; then
# Maybe also consider https://crates.io/crates/cargo-spellcheck
# TODO We need to have some discussion on what configs to use for `fmt` and `clippy`.
# TODO Put the fmt config arguments in different lines.
# Run `cargo fmt` for this file
rustfmt $i --config comment_width=100,wrap_comments=true,format_code_in_doc_comments=true,format_strings=true,license_template_path=./license.txt,imports_granularity=Module,normalize_comments=true,normalize_doc_attributes=true,group_imports=StdExternalCrate
# Run `cargo-clippy` for this file
cargo-clippy $i -- -D warnings \
-D clippy::as_conversions \
-D clippy::map_err_ignore \
-D clippy::large_types_passed_by_value \
-D clippy::missing_docs_in_private_items \
-D clippy::used_underscore_binding \
-D clippy::wildcard_dependencies \
-D clippy::wildcard_imports
fi
if [ "$extension" == "py" ]; then
# Run `black` for this file
black $i
fi
# Add changes to this file (as a result of formatting) to the commit.
git add $i
done
.rust-hook.toml
:
[hooks]
pre-commit = "sh pre-commit.sh"
Cargo.toml
:
[package]
name = "testing-git-hooks"
version = "0.1.0"
edition = "2021"
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
[dependencies]
[dev-dependencies]
rusty-hook = "0.11.2"
from rusty-hook.
@JonathanWoollett-Light haven't had a chance to look closely, but I suspect you will probably want to consider incorporating some set
options at the top of your script (perhaps set -e
and/or set -o pipefail
)
# TODO Put the fmt config arguments in different lines.
Would actually suggest you just plop a rustfmt config file in your repo and remove the CLI settings. Otherwise if you do both you'll inevitably end up with drift/duplicative management, or if you only set them here you may create some less-than-ideal inner dev loops due to e.g. editor-format-on-save behaviors formatting with a different rule set.
from rusty-hook.
set -e
fixed the issue thank you.
Would actually suggest you just plop a rustfmt config file in your repo and remove the CLI settings. Otherwise if you do both you'll inevitably end up with drift/duplicative management, or if you only set them here you may create some less-than-ideal inner dev loops due to e.g. editor-format-on-save behaviors formatting with a different rule set.
The problem here is that passing the arguments on command line for many of them doesn't require the nightly toolchain while using a config file does. It is very strange but for now since the project cannot use the nightly toolchain this is the approach (I changed it to use fmt.toml
then just load them from their with bash and pass them as command line arguments).
from rusty-hook.
The problem here is that passing the arguments on command line for many of them doesn't require the nightly toolchain while using a config file does. It is very strange
Yes, and it's been argued that it's a bug that rustfmt on stable will accept nightly only config options provided from the CLI. I can't guarantee I won't have to remove that at some point down the road, so buyer beware if you want to stick with it
from rusty-hook.
I would argue if the tool doesn't require the nightly toolchain and these options are simply unstable, it should not require the nightly toolchain for them to work, but rather a different option e.g. unstable_features=true
. Given almost all projects will avoid the nightly toolchain but most would want simple options like wrapping comments e.g. wrap_comments
and would not consider it a significant security issue.
from rusty-hook.
Related Issues (20)
- Exit gracefully from hook scripts when requisite bins missing
- Don't attempt to init during bin install
- Fix clippy warnings
- Setup Testspace publishing
- Switch to GitHub Actions
- Hooks fail when using toml array syntax for multi-commands in rusty-hook.toml HOT 2
- Allow for alternative path for config HOT 7
- constantly upgrading HOT 10
- Rename existing commit hooks HOT 1
- Improve handling of projects in sub-directories HOT 12
- Clarify/document hook commands executed in git root directory
- Document git hook files are automanaged HOT 1
- error: Found argument '' which wasn't expected, or isn't valid in this context HOT 7
- Release `0.12.0` ? HOT 4
- Differences to the altenative? HOT 2
- The git arguments are not properly passed by `%rh!` HOT 2
- Allow setting environment variables and map them with specific hook HOT 2
- Invalid rusty-hook config file HOT 1
- Pre-push hook does not work when it has a list as value HOT 3
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from rusty-hook.