Skip to content

Conversation

@BrianHung
Copy link
Owner

No description provided.

@BrianHung BrianHung added the ready to merge PRs ready to merge label Jul 14, 2025
@BrianHung BrianHung changed the title feat: insert delete rows cols insert delete rows cols Jul 14, 2025
@BrianHung
Copy link
Owner Author

cursor review
make sure there are comprehensive tests for each function, functions are ticked as available, and properly documented with their own page stub

cursor[bot]

This comment was marked as outdated.

@BrianHung
Copy link
Owner Author

cursor review can you find any other bugs with our implementation of this feature; would you make any improvements before merging?

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Bug: Empty Row Deletion Fails

The delete_rows method fails when attempting to delete empty rows (rows with no data), returning the error "Row number '{r}' is not valid." This occurs because the method incorrectly validates row existence by checking for cell data in worksheet.sheet_data, which is absent for empty rows. This prevents users from deleting empty rows or ranges that include them. The method should allow deletion of any valid row index, treating empty rows as valid and collecting an empty HashMap for their old_data in the diff.

base/src/user_model/common.rs#L926-L930

}
let data = match worksheet.sheet_data.get(&r) {
Some(s) => s.clone(),
None => return Err(format!("Row number '{r}' is not valid.")),
};

Fix in CursorFix in Web


Was this report helpful? Give feedback by reacting with 👍 or 👎

@BrianHung
Copy link
Owner Author

cursor review can you find any other bugs with our implementation of this feature; would you make any improvements before merging?

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no bugs!


@BrianHung BrianHung changed the base branch from main to wasm-bindgen-unchecked_return_type July 23, 2025 23:30
@BrianHung BrianHung changed the base branch from wasm-bindgen-unchecked_return_type to main July 23, 2025 23:30
@BrianHung BrianHung force-pushed the insert-delete-rows-cols branch from 3fffcd9 to 55810ad Compare July 24, 2025 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready to merge PRs ready to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants