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

feat: rocks check #289

Merged
merged 1 commit into from
Dec 30, 2024
Merged

feat: rocks check #289

merged 1 commit into from
Dec 30, 2024

Conversation

vhyrro
Copy link
Contributor

@vhyrro vhyrro commented Dec 29, 2024

Adds a simple implementation of rocks check, which ensures luacheck is installed and runs it for the whole source directory of a given project.

Similarly to rocks fmt, we'll want to expand this to take in a path as an argument for finer control.
I don't believe we should let the user control luacheck with flags, that should be up to the luacheckrc file.

@vhyrro
Copy link
Contributor Author

vhyrro commented Dec 29, 2024

Will fix the failing build in a bit.

rocks-bin/src/check.rs Outdated Show resolved Hide resolved
rocks-bin/src/check.rs Outdated Show resolved Hide resolved
@vhyrro vhyrro requested a review from mrcjkb December 29, 2024 16:46
Comment on lines +28 to +36
vec![
project.root().to_string_lossy().into(),
"--exclude-files".into(),
project
.tree(LuaVersion::from(&config)?)?
.root()
.to_string_lossy()
.to_string(),
],
Copy link
Member

Choose a reason for hiding this comment

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

suggestion:

Suggested change
vec![
project.root().to_string_lossy().into(),
"--exclude-files".into(),
project
.tree(LuaVersion::from(&config)?)?
.root()
.to_string_lossy()
.to_string(),
],
vec![
format!("{}", project.root().display()),
"--exclude-files".into(),
format!("{}", project
.tree(LuaVersion::from(&config)?)?
.root()
.display()),
],

Copy link
Member

Choose a reason for hiding this comment

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

Not sure if that's actually better, but apparently display() is safer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

display() does essentially the same thing as to_string_lossy by replacing all invalid unicode chars with the replacement character. Amounts to the same code in the end :)

@vhyrro vhyrro merged commit fd3a50f into master Dec 30, 2024
13 checks passed
@vhyrro vhyrro deleted the push-mlmvtkovvnmy branch December 30, 2024 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants