Skip to content

fix: detect go package clause after leading comments #1723

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
mokevnin wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix: detect go package clause after leading comments #1723

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ba0406a
fix: detect go package clause after leading comments
mokevnin Aug 29, 2026
8f98b6e
fix: skip leading comments when detecting go package clause
mokevnin Aug 29, 2026
1bd432c
chore: updated generated code
mokevnin Aug 29, 2026
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
4 changes: 3 additions & 1 deletion mdsf/src/parser/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ pub fn parse_go_codeblock(lines: &mut Enumerate<Lines>) -> (bool, String, usize)

// TODO: check for multiline comments
pub static GO_PACKAGE_RE: std::sync::LazyLock<Regex> =
std::sync::LazyLock::new(|| Regex::new(r"^\s*package\s+\w").unwrap());
std::sync::LazyLock::new(|| Regex::new(r"(?m)^\s*package\s+\w").unwrap());

@ccoVeille ccoVeille Aug 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This enable multine matching, won't this break some use cases.

package is a reserved word in Go, so it sounds safe

But you may something odd like this snippet without a package

/*

I like my
package manager
*/

const foo = 1

But I feel like there is no better solution without dealing with complicated regexp to remove comments

@mokevnin mokevnin Aug 29, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks, that example is a real hole and it pushed the fix to the right place.

Dropped the (?m) and matched Go's own rule instead: the package clause is the first token that is neither whitespace nor a comment. has_go_package skips leading // and /* */ and applies the existing anchored pattern to the remainder, so your snippet stays without a package clause and still gets the temporary one. No comment-removing regex needed, and the TODO: check for multiline comments above GO_PACKAGE_RE is gone with it.

Both directions are covered in a new test_has_go_package module, your snippet included.

@ccoVeille ccoVeille Aug 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you


#[inline]
pub fn remove_go_package(snippet: String) -> String {
Expand Down Expand Up @@ -81,6 +81,8 @@ mod test_go_package_re {
"\n package \tmdsf",
"\n package\tmdsf",
"\n \tpackage\t\n\nmdsf\n",
"// mdsf\npackage mdsf",
"//go:build integration\n\npackage mdsf",
] {
assert!(GO_PACKAGE_RE.is_match(s), "'{s}' did not match");
}
Expand Down