close
Skip to content

fix: use Pattern_White_Space for whitespace handling#22008

Open
vikashsiwach wants to merge 4 commits intorust-lang:masterfrom
vikashsiwach:fix-whitespace-miscompare
Open

fix: use Pattern_White_Space for whitespace handling#22008
vikashsiwach wants to merge 4 commits intorust-lang:masterfrom
vikashsiwach:fix-whitespace-miscompare

Conversation

@vikashsiwach
Copy link
Copy Markdown

@vikashsiwach vikashsiwach commented Apr 10, 2026

Description
This PR fixes incorrect whitespace handling in analysis_stats.rs by implementing Rust's definition of whitespace (Pattern_white_space).

Issue
Some parts fo code were using char::is_whitespace which was missing character like vertical tab ( \u{000B}).

Fixed

  • Export a parse helper ,parser::is_rust_whitespace which follows Rust's Pattern_White_Space.
  • Updated crates/parser/src/frontmatter.rs to use that shared Rust-whitespace logic.
  • Renamed the local helper in analysis_stats.rs from trim to drop_whitespace for clarity.

Related Issue
This PR addresses the Outreachy issue : non-macro code miscompare

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 10, 2026
@vikashsiwach vikashsiwach force-pushed the fix-whitespace-miscompare branch 2 times, most recently from cc38f6a to 0fa9bdb Compare April 10, 2026 09:35
Copy link
Copy Markdown
Contributor

@teor2345 teor2345 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, thanks for these fixes!

Usually we try to avoid moving or renaming functions. This makes it easier for reviewers to see what's actually changed.

Please edit the original functions to fix any bugs, and test them in their original locations.

If you think moves or renames are needed to make the code clearer, please put them in separate commits.

View changes since this review

@vikashsiwach vikashsiwach force-pushed the fix-whitespace-miscompare branch from 0fa9bdb to 04ab2f6 Compare April 11, 2026 22:36
}

#[cfg(test)]
mod term_search_syntax_compare_tests {
Copy link
Copy Markdown
Member

@lnicola lnicola Apr 12, 2026

Choose a reason for hiding this comment

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

View changes since the review

I don't think unit tests are necessary here.

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.

I used unit tests here to show the incorrect space handling. Now the issue has been fixed ,hence i removed the unit tests in the latest commit.

// }


fn trim(s: &str) -> String {
Copy link
Copy Markdown
Member

@lnicola lnicola Apr 12, 2026

Choose a reason for hiding this comment

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

View changes since the review

Maybe call this drop_whitespace or similar, trim feels like a bad name.

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.

Changed the fn trim to fn drop_whitespace as suggested.

@vikashsiwach vikashsiwach force-pushed the fix-whitespace-miscompare branch from 504d602 to 242fe0c Compare April 12, 2026 07:38
Copy link
Copy Markdown
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

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

This looks good to me, but the maintainers know more about this specific code than I do!

View changes since this review

@vikashsiwach
Copy link
Copy Markdown
Author

Hi @lnicola , I made some changes in code as suggested by you. Kindly take a look on it and give your feedback.

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

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants