Skip to content

Conversation

@wesm
Copy link
Contributor

@wesm wesm commented Aug 8, 2025

This is the R side of posit-dev/positron#8911

@wesm wesm requested review from Copilot and lionel- August 8, 2025 19:25
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements sort-columns-by-type functionality for the data explorer schema search feature. It extends the existing sorting capabilities to include type-based sorting alongside the existing name-based sorting options.

Key Changes

  • Updated SearchSchemaSortOrder enum to include four distinct sorting options: ascending/descending by name and ascending/descending by type
  • Enhanced search schema implementation to support type-based sorting using case-insensitive type name comparison
  • Expanded test coverage with comprehensive test cases for all sorting scenarios, edge cases, and combined filters

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
crates/amalthea/src/comm/data_explorer_comm.rs Updated SearchSchemaSortOrder enum with granular sorting options and improved documentation
crates/ark/src/data_explorer/r_data_explorer.rs Implemented type-based sorting logic with case-insensitive comparison in the search schema method
crates/ark/tests/data_explorer.rs Added comprehensive test coverage for new sorting functionality and updated existing tests to use new enum variants

Comment on lines +24 to +25
/// The column indices that match the search parameters in the indicated
/// sort order.
Copy link

Copilot AI Aug 8, 2025

Choose a reason for hiding this comment

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

[nitpick] The comment formatting is inconsistent. Use consistent indentation with spaces instead of tabs for comments to match the surrounding code style.

Suggested change
/// The column indices that match the search parameters in the indicated
/// sort order.
/// The column indices that match the search parameters in the indicated
/// sort order.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is auto-generated code, perhaps we should change the generate-comms.ts to indent with spaces instead of tabs

@wesm wesm merged commit aad04d5 into main Aug 11, 2025
6 checks passed
@wesm wesm deleted the feature/de-sort-columns-by-type branch August 11, 2025 13:11
@github-actions github-actions bot locked and limited conversation to collaborators Aug 11, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants