-
Notifications
You must be signed in to change notification settings - Fork 0
fix: match requirement identity case-insensitively #11
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -119,8 +119,10 @@ impl CanonicalSpec { | |
| } | ||
|
|
||
| pub(super) fn requirement_index(&self, name: &str) -> Option<usize> { | ||
| let identity = requirement_identity(name); | ||
| self.body.iter().position(|element| { | ||
| matches!(element, BodyElement::Requirement(requirement) if requirement.name == name) | ||
| matches!(element, BodyElement::Requirement(requirement) | ||
| if requirement_identity(&requirement.name) == identity) | ||
| }) | ||
| } | ||
|
|
||
|
|
@@ -134,7 +136,7 @@ impl CanonicalSpec { | |
| requirement: &Requirement, | ||
| ) -> bool { | ||
| self.requirement(element_index).is_some_and(|existing| { | ||
| existing.name == requirement.name | ||
| requirement_identity(&existing.name) == requirement_identity(&requirement.name) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🚫 [runeseer] reported by reviewdog 🐶
|
||
| && existing.content | ||
| == normalize_line_endings(&requirement.content, &self.line_ending) | ||
| }) | ||
|
|
@@ -254,6 +256,18 @@ pub(super) struct ParseIssue { | |
| pub(super) message: String, | ||
| } | ||
|
|
||
| /// Requirement identity for lookups: case- and whitespace-insensitive, | ||
| /// so a Title Case delta finds its sentence-case canonical twin instead | ||
| /// of appending a duplicate. Equality checks that decide whether an | ||
| /// edit applies keep exact comparison, so a case-only rename still | ||
| /// lands. | ||
| fn requirement_identity(name: &str) -> String { | ||
| name.split_whitespace() | ||
| .collect::<Vec<_>>() | ||
| .join(" ") | ||
| .to_ascii_lowercase() | ||
| } | ||
|
|
||
| fn normalize_line_endings(content: &str, line_ending: &str) -> String { | ||
| content | ||
| .replace("\r\n", "\n") | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫 [runeseer] reported by reviewdog 🐶
High —
requirement_indexreturns the first case-insensitive match, so on a spec that already holds both case variants — the duplicate state this commit exists to stop — a REMOVED, MODIFIED, or RENAMED operation naming the Title Case requirement silently mutates the sentence-case one instead.src/spec/model.rs:123position()stops at the first identity match and no caller checks for a second.src/spec/apply.rs:111remove_requirement(element_index)deletes that wrong element with no name reconfirmation.