Optimise display of property values #3
Conversation
There was a problem hiding this comment.
Pull request overview
This PR enhances the display of property values by adding UTF-8 text display for binary data and configurable endianness for data-to-number conversions. The key improvement is showing human-readable strings like "Apple Inc." instead of base64-encoded or hexadecimal values, and correctly interpreting numeric data with little-endian byte order.
- Replaces boolean "Detect Numbers in Data" checkbox with a dropdown offering Disabled/Big-endian/Little-endian options
- Adds "Description" and "UTF-8" display modes for binary data
- Improves number formatting with proper width padding based on byte size
- Removes the
Data.swiftextension file, moving functionality inline
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| PropertyValueTransformer.swift | Enhanced number and data formatting with boolean detection, proper hex padding, endianness-aware data-to-number conversion, and UTF-8 text display |
| Preferences.swift | Changed detectNumbersInData property type from Bool to Int to support dropdown with multiple endianness options |
| Data.swift | Removed file - functionality moved inline to PropertyValueTransformer |
| PropertiesViewController.xib | Updated UI to replace checkbox with endianness dropdown, added "Description" and "UTF-8" menu items, adjusted layout |
| project.pbxproj | Removed Data.swift file references and added app category metadata |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if Preferences.shared.detectNumbersInData != 0, [1, 2, 4, 8].contains(data.count) | ||
| { | ||
| return PropertyValueTransformer().transformedValue( PropertyListNode( key: value.key, propertyList: NSNumber( value: number ) ) ) | ||
| } | ||
| else if Preferences.shared.dataDisplayMode == 0 | ||
| { | ||
| return data.base64EncodedString() | ||
| var number: UInt64 = 0 | ||
|
|
||
| switch (Preferences.shared.detectNumbersInData) | ||
| { | ||
| case 1: // big-endian | ||
| for byte in data | ||
| { | ||
| number = (number << 8) | UInt64(byte) | ||
| } | ||
| case 2: // little-endian | ||
| for ( i, byte ) in data.enumerated() | ||
| { | ||
| number |= UInt64( byte ) << ( i * 8 ) | ||
| } | ||
| default: | ||
| break | ||
| } | ||
|
|
||
| if Preferences.shared.numberDisplayMode == 0 | ||
| { | ||
| return number.description | ||
| } | ||
| else | ||
| { | ||
| let width = data.count * 2 | ||
| return String(format: "0x%0*llX", width, number) | ||
| } |
There was a problem hiding this comment.
In the big-endian case, when the switch statement's default case is reached (line 88), the number variable remains 0. This means if detectNumbersInData has a value other than 1 or 2 (which shouldn't happen based on the UI, but could happen if the preference is corrupted or set programmatically), the code will continue to format and return "0" or "0x00" instead of falling through to the data display logic. Consider returning early or handling this case explicitly to avoid displaying misleading values.
| <behavior key="behavior" changeContents="YES" doesNotDimImage="YES" lightByContents="YES"/> | ||
| <textField horizontalHuggingPriority="251" verticalHuggingPriority="750" translatesAutoresizingMaskIntoConstraints="NO" id="WCq-cV-1ix"> | ||
| <rect key="frame" x="367" y="13" width="156" height="14"/> | ||
| <textFieldCell key="cell" controlSize="small" lineBreakMode="clipping" title="Convert number-like NSData:" id="c0H-YU-Gtx"> |
There was a problem hiding this comment.
The label "Convert number-like NSData:" uses the Objective-C type name "NSData" instead of the Swift equivalent "Data". For consistency with Swift naming conventions and since this is a Swift codebase, consider using "Data" instead of "NSData" in user-facing text.
| <textFieldCell key="cell" controlSize="small" lineBreakMode="clipping" title="Convert number-like NSData:" id="c0H-YU-Gtx"> | |
| <textFieldCell key="cell" controlSize="small" lineBreakMode="clipping" title="Convert number-like Data:" id="c0H-YU-Gtx"> |
| case 2: | ||
| return "0x" + data.map { String(format: "%02X", $0) }.joined() | ||
| case 3: | ||
| return String(data: data, encoding: .utf8) ?? "<invalid UTF-8 data>" |
There was a problem hiding this comment.
The UTF-8 string conversion at line 110 attempts to decode binary data as UTF-8, which could fail for arbitrary binary data. While the nil-coalescing operator provides a fallback, the String initializer may still perform unnecessary decoding work for data that is clearly not UTF-8 text (e.g., starts with null bytes or contains invalid UTF-8 sequences). Consider adding a quick validation check for common UTF-8 patterns before attempting full decoding to improve performance when displaying large amounts of binary data.
1 participant
device-tree/manufacturerreportsApple Inc.instead ofQXBwbGUgSW5jLgA=or0x4170706C6520496E632E00device-tree/clock-frequencyreports24000000instead of3567105