Skip to content

Use Regex instead of taplo to extract channel in linter action #526

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

Merged
merged 5 commits into from
Jul 22, 2025

Conversation

BD103
Copy link
Member

@BD103 BD103 commented Jul 20, 2025

Fixes #510.

This PR modifies bevy_lint's action to use Regex to capture the toolchain channel in rust-toolchain.toml. Specifically, it uses this pattern:

^channel = "(.+)"$

I recommend looking at this in Regex101 to understand what its doing. In short, it looks for the line channel = "..." and captures everything between the double quotes.

Additionally, I switched the toolchain components to be completely hard-coded within the action. As these components are unlikely to ever change, I figured it was far simpler than trying to use more Regex to extract them from rust-toolchain.toml. This can be changed in the future!

@BD103 BD103 added A-Build-System Related to CI and GitHub Actions A-Linter Related to the linter and custom lints C-Dependencies A change related to dependencies C-Usability An improvement that makes the API more pleasant D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Needs-Review The PR needs to be reviewed before it can be merged C-Performance labels Jul 20, 2025
@BD103 BD103 added this to the `bevy_lint` v0.4.0 milestone Jul 20, 2025
BD103 added 2 commits July 20, 2025 12:51
I believe Bash is treating `"${REGEX}"` as a raw Regex pattern, and doesn't interpolate the value of `$REGEX` into the string. It seems to support the `$REGEX` syntax, however.
I couldn't get Bash's built-in Regex matching to work, so I switched to `sed`.
@BD103
Copy link
Member Author

BD103 commented Jul 20, 2025

I switched the action over to use sed, since I couldn't get Bash Regex to work for some reason. I think this should be working now!

Copy link
Collaborator

@DaAlbrecht DaAlbrecht left a comment

Choose a reason for hiding this comment

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

Looks good imo. Tested the commands locally and it works as expected!

toolchain: ${{ steps.channel.outputs.channel }}
# These components are hard-coded because they are unlikely to change. They should match
# `rust-toolchain.toml`.
components: rustc-dev, llvm-tools-preview
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit:

Could we not also use a sed command like:

sed -nE 's/^components = \[(.*)\]/\1/p' rust-toolchain.toml | tr -d '"'

Copy link
Member Author

Choose a reason for hiding this comment

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

Huh, I didn't know about tr until now! I was so concerned with parsing the array that I never considered simply stripping all of the ", clever!

@BD103 BD103 merged commit d2ee722 into main Jul 22, 2025
13 checks passed
@BD103 BD103 deleted the dont-install-taplo branch July 22, 2025 02:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Build-System Related to CI and GitHub Actions A-Linter Related to the linter and custom lints C-Dependencies A change related to dependencies C-Performance C-Usability An improvement that makes the API more pleasant D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Needs-Review The PR needs to be reviewed before it can be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Linter action shouldn't install Taplo
2 participants