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

Add ability to show both CFBundleVersion and CFBundleShortVersionString in UI #2267

Closed
ychin opened this issue Sep 17, 2022 · 7 comments · Fixed by #2321
Closed

Add ability to show both CFBundleVersion and CFBundleShortVersionString in UI #2267

ychin opened this issue Sep 17, 2022 · 7 comments · Fixed by #2321
Milestone

Comments

@ychin
Copy link

ychin commented Sep 17, 2022

Summary

Sparkle currently automatically picks whether to show the build number (CFBundleVersion in Info.plist and sparkle:version in appcast) and "short version string" (CFBundleShortVersionString in Info.plist and sparkle:shortVersionString in appcast) when showing the user what version they have.

This kind of works, but for some apps (e.g. the one I maintain, MacVim), both short version and bundle version are useful info to have. Because of how it's set up historically, the "short version string" refers to the Vim version (e.g. 9.1.234), whereas the bundle version (e.g. 175) refers to the release number (which is incremented by 1 every time I make a release, but the Vim version could have jumped by hundreds). I would ideally like the UI to say "MacVim 9.1.234 (r175) is now available, you have MacVim 8.7.890 (r173)." Instead, Sparkle seems to always prefer short version string, and only show bundle version in certain special cases (if short version doesn't exist, or if both old and new builds have the same short version string).

While I could modify my short version string to include the bundle version as well (e.g. I can make it look like "9.1.234 (r175)" in the appcast and CFBundleShortVersionString), I don't really want to, and it seems to go against the official documentation saying that CFBundleShortVersionString should just be 3 numbers separated by dots.

Note that there is a SUVersionDisplay delegate but it doesn't quite do what I'm saying here as I don't see an easy way to customize both versions' strings, and there are places where that delegate isn't called (e.g. when Sparkle determines there is no new version and shows a "You're up to date" dialog.

Note that #1496 was asking for a similar thing.

Possible Fix

Is there a way that Sparkle can expose a setting like SUShowBothShortAndBundleVersion to allow us to customize the behavior? I think each app is always going to have their different ways of handling short vs bundle versions, as Apple makes these two fields pretty flexible meaning that it's up to each app to decide what these numbers / strings should mean. Seems like Sparkle could allow the developer to choose.

An alternative is just to provide a more generic API than SUVersionDisplay's formatVersion:(NSString**)inOutVersionA andVersion:(NSString**)inOutVersionB API. To me, it should just be something like that (NSString*) versionString:(NSString*)shortVersionString (NSString*)bundleVersion (or the API could just pass a version info struct so it's more extensible in the future if people want access to the pubDate or sparkle:minimumSystemVersion etc). So in my case, I could just implement it so that I have the following (pseudocode):

// Implementing SUVersionDisplay
string macvimVersionString(string shortVersion, string bundleVersion) {
  return "$shortVersion (r$bundleVersion)"
}

Version

2.2.2.

@zorgiepoo
Copy link
Member

zorgiepoo commented Sep 18, 2022

I think SUVersionDisplay should be enhanced.

Maybe

- (void)formatUpdateDisplayVersion:(NSString * _Nonnull * _Nonnull)inOutUpdateDisplayVersion andBundleDisplayVersion:(NSString * _Nonnull * _Nonnull)inOutBundleDisplayVersion fromUpdate:(SUAppcastItem *)update bundleVersion:(NSString *)bundleVersion;

Sparkle itself should use/create a standard version displayer and unify its usage across both places (e.g, you're up-to-date and X is now available, you have Y..). There are three cases I think: we only display the host display version, we only display the update item display version, or we display both. However I think in all cases we know both the host and update item in question -- and they're both relevant.

Across these two scenarios, unfortunately the updater driver creates some of these strings (in you're up-to-date case), while the standard user driver handles the other case and generally owns the version displayer from the delegate. Perhaps that can be fixed by the update driver querying the version displayer from user drivers that have a version displayer (which gets it from the delegate), otherwise defaulting to a standard one.

@ychin
Copy link
Author

ychin commented Sep 18, 2022

Right, I think have a more unified way to control display versions would be nice, both for extensibility and just to be able to understand what gets displayed more.

In the API you proposed though seems like I'm getting both the "update display version" and "bundle display version", but only one "bundle version"? I would imagine I need four pieces of info? (current app's display + bundle version, new app's display + bundle version)

@zorgiepoo
Copy link
Member

You can get the update's "bundle" version from update.versionString and display version from update.displayVersionString.

It could alternatively be like this (may make more sense):

- (NSString *)formatUpdateDisplayVersionWithUpdate:(SUAppcastItem *)update andBundleDisplayVersion:(NSString * _Nonnull * _Nonnull)inOutBundleDisplayVersion withBundleVersion:(NSString *)bundleVersion;

@ychin
Copy link
Author

ychin commented Sep 18, 2022

Ah you are right. Having access to the update itself is better.

@zorgiepoo
Copy link
Member

@ychin Please check the pull request in #2321 to resolve this if you've a chance. Most notably, the additions/changes to the SUVersionDisplayProtocol.h file.

@ychin
Copy link
Author

ychin commented Feb 21, 2023

I tried this out locally and it seems to work as expected! Thanks!

What did you mean by "resolve" this? Did you mean closing the issue?

@zorgiepoo
Copy link
Member

Yes.

ychin added a commit to ychin/macvim that referenced this issue Mar 20, 2023
Sparkle 2.4 added support for customizing the displayed version string
instead of just showing the "display version" from the manifest (see
sparkle-project/Sparkle#2267). Since MacVim
has a somewhat non-standard usage of bundle vs display version (bundle
is release number, and display version is the upstream Vim version), we
really want to show both to the user, in a concise manner. Support this
customization so it looks like "r123 (Vim 9.1.2345)" (here, "123" is the
bundle version / release number, and "9.1.2345" is the display version /
Vim version).

Fix macvim-dev#1293
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants