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.

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

fix int format in search results #479

Closed
wants to merge 1 commit into from

Conversation

algorythmic
Copy link

Since trackId is an Int and can be a number greater than Int32.max, it should be printed with the %ld format specifier.

Fixes #478

@plandem
Copy link

plandem commented Aug 22, 2024

still relevant bug

@@ -26,9 +26,9 @@ enum SearchResultFormatter {
let price = result.price ?? 0.0

if includePrice {
output += String(format: "%12d %@ $%5.2f (%@)\n", appId, appName, price, version)
output += String(format: "%12ld %@ $%5.2f (%@)\n", appId, appName, price, version)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please change from %12ld to %12lu (signed to unsigned)

Copy link
Author

Choose a reason for hiding this comment

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

The ld format specifier matches what is being printed -- the trackId member of the SearchResult struct, which currently has type Int, not UInt.

Presumably the API does not actually return negative trackId's, so it probably makes sense to change both the type and the format specifier. But I'm not sure about changing one and not the other?

Since `trackId` is an `Int` and can be a number greater than `Int32.max`,
it should be printed with the `%ld` format specifier [1].

[1]: https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/Strings/Articles/formatSpecifiers.html#//apple_ref/doc/uid/TP40004265-SW5
xyzzy-plugh-plover added a commit to xyzzy-plugh-plover/mas that referenced this pull request Sep 11, 2024
Supersede mas-cli#479
Fix mas-cli#478

Signed-off-by: xyzzy-plugh-plover <114089973+xyzzy-plugh-plover@users.noreply.github.com>
@rgoldberg
Copy link
Contributor

Closing since PR #535 includes the small fix here within a more comprehensive fix.

@rgoldberg rgoldberg closed this Sep 13, 2024
rgoldberg pushed a commit to xyzzy-plugh-plover/mas that referenced this pull request Oct 6, 2024
Supersede mas-cli#479
Fix mas-cli#478

Signed-off-by: xyzzy-plugh-plover <114089973+xyzzy-plugh-plover@users.noreply.github.com>
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.

App ID should be UInt64 instead of Int
3 participants