Skip to content

✨ Marker support for Rust file extension .rs #4827

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

✨ Marker support for Rust file extension .rs #4827

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bf546ba
add rust extension
mabulgu Feb 8, 2025
9d553a9
fix typoe
mabulgu Feb 8, 2025
5a6de81
Merge branch 'kubernetes-sigs:master' into rust-lang
mabulgu Apr 21, 2025
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
1 change: 1 addition & 0 deletions pkg/machinery/marker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ var commentsByExt = map[string]string{
".go": "//",
".yaml": "#",
".yml": "#",
".rs": "//",
Copy link
Member

Choose a reason for hiding this comment

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

We cannot add a file that we do not support have officially
Could the extension not be added in your code using the lib?

Copy link
Author

Choose a reason for hiding this comment

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

Yeah no problem, I got that:) What I meant: I will try what you suggest above, and actually the way I had implemented it was like what you suggested above. Let me revert to what I had before and compare with your suggestion and see if that works. I dont remember why I avoided that approach, maybe just not to duplicate things (thinking I can have the rs extension definition in kubebuilder)

Copy link
Author

Choose a reason for hiding this comment

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

@camilamacedo86 ok I've tried and remembered the reason as I tried to use the same approach. Here it is:

Screenshot 2025-05-25 at 22 04 02

Copy link
Author

Choose a reason for hiding this comment

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

So in other words, if we can make the marker struct use public fields instead of private ones, any other plugins can use their own custom markers.

I can create a PR for this if this sounds good to you?

Copy link
Author

@mabulgu mabulgu May 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Making the fields public worked for me: mabulgu@4a61855

I can create a PR for this (not via the above branch of course, with a new one) because without this change I dont think anyone can use a custom marker.

Here is the implementation on the plugin: https://github.com/SystemCraftsman/rust-operator-plugins/blob/main/pkg/plugins/rust/marker.go

This might help us create the documentation for the custom marker. I can help with that as well.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, sure. Thank you a lot !!!
Yes, we need to allow people does that and create their own markers for their own extensions.
Please, feel free to raise the PR.

// When adding additional file extensions, update also the NewMarkerFor documentation and error
}

Expand Down