Skip to content

Sort columns numerically when all non-empty values are numeric#8

Open
Copilot wants to merge 2 commits into
masterfrom
copilot/add-intelligent-column-sorting
Open

Sort columns numerically when all non-empty values are numeric#8
Copilot wants to merge 2 commits into
masterfrom
copilot/add-intelligent-column-sorting

Conversation

Copilot AI commented Apr 1, 2026

Copy link
Copy Markdown
  • Scan column values before sorting to determine if all non-empty values are numeric
  • Use the same regex as the cell editor (parameterised with csvConfig.decimalMark) for numeric detection
  • Use decimalNumberWithString:locale: with the configured decimal separator for numeric parsing
  • If non-numeric, keep existing localizedCaseInsensitiveCompare: string comparison

@danaspiegel danaspiegel marked this pull request as ready for review April 1, 2026 15:53
Copilot AI review requested due to automatic review settings April 1, 2026 15:54

Copilot AI left a comment

Copy link
Copy Markdown

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 updates TableTool’s column-sorting behavior in Document.m so that columns containing only numeric (non-empty) values are sorted numerically rather than lexicographically, avoiding cases like 9 > 10 > 100.

Changes:

  • Adds a pre-sort scan of the target column to decide whether to sort numerically or alphabetically.
  • Uses NSDecimalNumber’s compare: for numeric columns, while keeping the existing case-insensitive string comparison as a fallback.
  • Treats empty cells as zero during numeric sorting.

Comment thread Table Tool/Document.m Outdated
Comment on lines +1272 to +1295
// Determine whether all non-empty values in the column are numeric so we
// can choose between a numeric and an alphabetic sort.
BOOL sortNumerically = YES;
for (NSMutableArray *row in _data) {
id val = colIndex < (NSInteger)row.count ? row[colIndex] : @"";
if ([val isKindOfClass:[NSDecimalNumber class]]) continue;
NSString *str = (NSString *)val;
if (str.length == 0) continue;
NSDecimalNumber *num = [NSDecimalNumber decimalNumberWithString:str];
if ([num isEqual:[NSDecimalNumber notANumber]]) {
sortNumerically = NO;
break;
}
}

[_data sortUsingComparator:^NSComparisonResult(NSMutableArray *row1, NSMutableArray *row2) {
id val1 = colIndex < (NSInteger)row1.count ? row1[colIndex] : @"";
id val2 = colIndex < (NSInteger)row2.count ? row2[colIndex] : @"";
NSString *str1 = [val1 isKindOfClass:[NSDecimalNumber class]] ? [(NSDecimalNumber *)val1 description] : (NSString *)val1;
NSString *str2 = [val2 isKindOfClass:[NSDecimalNumber class]] ? [(NSDecimalNumber *)val2 description] : (NSString *)val2;
NSComparisonResult result = [str1 localizedCaseInsensitiveCompare:str2];
NSComparisonResult result;
if (sortNumerically) {
NSDecimalNumber *num1 = [val1 isKindOfClass:[NSDecimalNumber class]] ? val1 :
([(NSString *)val1 length] > 0 ? [NSDecimalNumber decimalNumberWithString:(NSString *)val1] : [NSDecimalNumber zero]);
NSDecimalNumber *num2 = [val2 isKindOfClass:[NSDecimalNumber class]] ? val2 :
([(NSString *)val2 length] > 0 ? [NSDecimalNumber decimalNumberWithString:(NSString *)val2] : [NSDecimalNumber zero]);

Copilot AI Apr 1, 2026

Copy link

Choose a reason for hiding this comment

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

Numeric detection/parsing during sort ignores the configured decimal separator (self.csvConfig.decimalMark). Elsewhere in this file and in CSVReader numeric parsing uses decimalNumberWithString:locale:@{NSLocaleDecimalSeparator: ...}; with a comma decimal mark, numeric strings like "1,23" would be treated as non-numeric here and fall back to lexicographic sorting (or be parsed inconsistently). Use the same locale-aware parsing (and ideally the same regex/criteria) for both the detection loop and the numeric comparator conversion path.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@copilot apply changes based on this feedback

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.

Fixed in a71d11f. Numeric detection now uses the same regex as the cell editor (parameterised with self.csvConfig.decimalMark), and all decimalNumberWithString: calls now pass locale:@{NSLocaleDecimalSeparator: decimalMark} — consistent with how CSVReader and the cell-edit handler parse numbers. The descriptionWithLocale: equivalent is also used when stringifying an NSDecimalNumber in the alphabetic fallback path.

Copilot AI requested a review from danaspiegel April 1, 2026 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants