Skip to content
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

Jump to bottom

fix: Handling Dangling Commas in delete_hanging_comma Function #417

Closed
wants to merge 5 commits into from
Closed

fix: Handling Dangling Commas in delete_hanging_comma Function #417

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a377622
fix isssue of danlging comma
varshith257 Jul 11, 2024
4a4fb7d
add test to evaluate delete_hanging_comma fn
varshith257 Jul 11, 2024
a27a41f
refined approach
chintu-777 Jul 12, 2024
6531fca
Update inline_snippets.rs
chintu-777 Jul 12, 2024
adb33e8
fix lint
varshith257 Jul 12, 2024
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
10 changes: 9 additions & 1 deletion crates/core/src/inline_snippets.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,10 +348,17 @@ fn delete_hanging_comma(
.iter()
.map(|r| (r.0.effective_range(), r.1.len()))
.collect();


// Flag to track if the last character was a comma
let mut last_was_comma = false;

for (index, c) in chars {
if Some(&index) != next_comma {
if c == ',' && last_was_comma {
continue;
}
result.push(c);
last_was_comma = c == ',';
} else {
// Keep track of ranges we need to expand into, since we deleted code in the range
// This isn't perfect, but it's good enough for tracking cell boundaries
Expand All @@ -363,6 +370,7 @@ fn delete_hanging_comma(
}
ranges_updates = update_range_shifts(index + offset, &ranges_updates, &ranges);
next_comma = to_delete.next();
last_was_comma = false;
}
}

Expand Down
35 changes: 35 additions & 0 deletions crates/core/src/test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6330,6 +6330,41 @@ fn inline_list_length() {
.unwrap();
}

#[test]
fn test_delete_hanging_comma() {
run_test_expected(TestArgExpected {
pattern: r#"
|language python
|
|`TaskMetadata($args)` => `TaskMetadata($args)`
|"#
.trim_margin()
.unwrap(),
source: r#"
|TaskMetadata(
| name="TestTask",
| description="This is a test task.",
| type="TestType",
| ,
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't a representative test. We specifically want to test cases where an argument is removed, and therefore the trailing comma after it is also removed.

Copy link
Contributor

@abhishek818 abhishek818 Jul 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@morgante I was also working on the issue since no one was assigned and recently pushed my PR .

| reference="https://example.com/test",
|)
|"#
.trim_margin()
.unwrap(),
expected: r#"
|TaskMetadata(
| name="TestTask",
| description="This is a test task.",
| type="TestType",
| reference="https://example.com/test",
|)
|"#
.trim_margin()
.unwrap(),
})
.unwrap();
}

#[test]
fn string_length() {
run_test_expected({
Expand Down
Loading