Skip to content

Conversation

@lemorage
Copy link
Contributor

Close #531

@j178 j178 added the enhancement New feature or request label Aug 26, 2025
@j178 j178 changed the title Implement check_yaml as built hook Implement check_yaml as builtin hook Aug 26, 2025
Copy link
Owner

@j178 j178 left a comment

Choose a reason for hiding this comment

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

Thanks! I’m a bit worried because we’re missing the --unsafe and --allow-multiple-documents features, and I know it’s pretty tough to implement these exactly in Rust.

I’m thinking we should only use our built-in implementation if the check-yaml hook doesn’t include --unsafe or --allow-multiple-documents arguments. Basically, we should improve our detection in the check_fast_path logic for better compatibility. What do you think?

@lemorage
Copy link
Contributor Author

Fair considerations to me. But simply adding a special case to check_fast_path makes things bloated, also is not quite clean and extensible. If this is not a burden on your side, I can make this addition later.

@j178
Copy link
Owner

j178 commented Aug 27, 2025

I added a basic check over at dc1ccf7, I think it's fine for now.

@j178 j178 merged commit fec9627 into j178:master Aug 27, 2025
21 checks passed
@lemorage
Copy link
Contributor Author

Thx!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: support check-yaml as a builtin hook

2 participants