Handles macOS and older GLibs better.
Some tests so far:
- [ ] Apple macOS - [ ] Win7 - [x] Win10: - `OS: Windows 10 2004 (Unknown)` - [x] Ubuntu 20.04 - `OS: Ubuntu 20.04.1 LTS (focal)` - [x] Ubuntu 20.04 with incompatible GLib - `OS: Linux`
If it gets enough testing, could make 1.37, else we can leave for next release. You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/2618
-- Commit Summary --
* Add utils function to get OS info string * Use new `utils_get_os_info_string` function
-- File Changes --
M src/libmain.c (16) M src/utils.c (68) M src/utils.h (2)
-- Patch Links --
https://github.com/geany/geany/pull/2618.patch https://github.com/geany/geany/pull/2618.diff
WFM, result in list above.
Debian:
OS: Debian GNU/Linux bullseye/sid
Windows 7: OS: Windows 7 SP17Vista SP2Vista SP1VistaXP SP3XP SP2XP SP1XP (Unknown)
@eht16 commented on this pull request.
+ os_str = g_string_new(pretty_name); + g_free(pretty_name); + + code_name = g_get_os_info(G_OS_INFO_KEY_VERSION_CODENAME); + if (code_name != NULL) + { + g_string_append_printf(os_str, " (%s)", code_name); + g_free(code_name); + } + + os_info = g_string_free(os_str, FALSE); + } +# else + /* on macOS, only `G_OS_INFO_KEY_NAME` is supported and returns the + * fixed string `macOS`, as of Oct 13, 2020 */
Is your timezone +48:00? :smile:
@elextr commented on this pull request.
+ os_str = g_string_new(pretty_name); + g_free(pretty_name); + + code_name = g_get_os_info(G_OS_INFO_KEY_VERSION_CODENAME); + if (code_name != NULL) + { + g_string_append_printf(os_str, " (%s)", code_name); + g_free(code_name); + } + + os_info = g_string_free(os_str, FALSE); + } +# else + /* on macOS, only `G_OS_INFO_KEY_NAME` is supported and returns the + * fixed string `macOS`, as of Oct 13, 2020 */
when he hopes it will be merged :grin:
@codebrainz commented on this pull request.
+ os_str = g_string_new(pretty_name); + g_free(pretty_name); + + code_name = g_get_os_info(G_OS_INFO_KEY_VERSION_CODENAME); + if (code_name != NULL) + { + g_string_append_printf(os_str, " (%s)", code_name); + g_free(code_name); + } + + os_info = g_string_free(os_str, FALSE); + } +# else + /* on macOS, only `G_OS_INFO_KEY_NAME` is supported and returns the + * fixed string `macOS`, as of Oct 13, 2020 */
*eek* they're on to me! back to the time machine!
Debian:
OS: Debian GNU/Linux bullseye/sid
Is the "bullseye/sid" part parenthesized or exactly as above?
As in the quote 
Well with the weird Debian naming scheme I guess it doesn't have a per version nickname like `fossa` for Ubuntu, so having no nickname makes (twisted Debian) sense. Anyway it provides information on the OS which is the main thing.
@elextr requested changes on this pull request.
Also making the strings untranslated makes it mergable now we are in string freeze :)
+# endif
+#else + /* if g_get_os_info() is not available, do it the old-fashioned way */ +# if defined(_WIN64) + os_info = g_strdup(_("Microsoft Windows (64-bit)")); +# elif defined(_WIN32) + os_info = g_strdup(_("Microsoft Windows")); +# elif defined(__APPLE__) + os_info = g_strdup(_("Apple macOS")); +# elif defined(__linux__) + os_info = g_strdup(_("Linux")); +# elif defined(__FreeBSD__) + os_info = g_strdup(_("FreeBSD")); +# elif defined(__ANDROID__) + os_info = g_strdup(_("Android")); +# endif
Why are the above marked as translatable, what we want is the OS name, and all of them are English.
@codebrainz commented on this pull request.
+# endif
+#else + /* if g_get_os_info() is not available, do it the old-fashioned way */ +# if defined(_WIN64) + os_info = g_strdup(_("Microsoft Windows (64-bit)")); +# elif defined(_WIN32) + os_info = g_strdup(_("Microsoft Windows")); +# elif defined(__APPLE__) + os_info = g_strdup(_("Apple macOS")); +# elif defined(__linux__) + os_info = g_strdup(_("Linux")); +# elif defined(__FreeBSD__) + os_info = g_strdup(_("FreeBSD")); +# elif defined(__ANDROID__) + os_info = g_strdup(_("Android")); +# endif
They are marked translatable in case some language(s) call them something else. No opinion really, other than that if they aren't already translated before release, then it will just put the English names anyway.
@elextr commented on this pull request.
+# endif
+#else + /* if g_get_os_info() is not available, do it the old-fashioned way */ +# if defined(_WIN64) + os_info = g_strdup(_("Microsoft Windows (64-bit)")); +# elif defined(_WIN32) + os_info = g_strdup(_("Microsoft Windows")); +# elif defined(__APPLE__) + os_info = g_strdup(_("Apple macOS")); +# elif defined(__linux__) + os_info = g_strdup(_("Linux")); +# elif defined(__FreeBSD__) + os_info = g_strdup(_("FreeBSD")); +# elif defined(__ANDROID__) + os_info = g_strdup(_("Android")); +# endif
It doesn't matter what its called in the users language, the info is for the project, which is in English, "مايكروسوفت ويندوز 7" (Google translate Arabic for Microsoft Windows 7) isn't any use to us.
@eht16 commented on this pull request.
+# endif
+#else + /* if g_get_os_info() is not available, do it the old-fashioned way */ +# if defined(_WIN64) + os_info = g_strdup(_("Microsoft Windows (64-bit)")); +# elif defined(_WIN32) + os_info = g_strdup(_("Microsoft Windows")); +# elif defined(__APPLE__) + os_info = g_strdup(_("Apple macOS")); +# elif defined(__linux__) + os_info = g_strdup(_("Linux")); +# elif defined(__FreeBSD__) + os_info = g_strdup(_("FreeBSD")); +# elif defined(__ANDROID__) + os_info = g_strdup(_("Android")); +# endif
What @elextr said. Almost all of the other debug output is not translated as well for the same reason.
Just for completeness: `OS: Arch Linux`
I get ``` 16:13:53: Geany INFO : OS: macOS ```
Question: the function is exported as an API function to plugins. Is this something plugins are actually interested in?
@elextr Bullseye is actually name of the next major one -- Running testing here
@codebrainz Just compiled your branch with nix package manager -- still describing itself as Debian:
``` Geany-INFO: 20:16:45.424: Geany 1.37, C Geany-INFO: 20:16:45.424: GTK 3.24.21, GLib 2.64.5 Geany-INFO: 20:16:45.426: OS: Debian GNU/Linux bullseye/sid Geany-INFO: 20:16:45.426: System data dir: /nix/store/cyk52jflc1v4aj5pqry8v57sv9p1ha9h-geany-1.37git/share/geany … ``` But I guess this is about g_get_os_inf() ist working.
@techee AFAICT its not actually in the API, the documentation message is not a doxygen comment and the function isn't marked `GEANY_API_SYMBOL`.
In theory a plugin might use the info, but given the variability (and as @frlan showed the fact that derived distros may show as the base distro) its probably not much use to a plugin.
@frlan the g_get_os_info() docs say "On Linux this comes from the /etc/os-release file." so if your distro is derived from Debian its probably not modifying that file. So as much as I like to blame G* for everything, in this case its probably not their fault.
@techee AFAICT its not actually in the API, the documentation message is not a doxygen comment and the function isn't marked `GEANY_API_SYMBOL`.
No, but it has an API version in the `@since` tag, which doesn't make much sense.
In theory a plugin might use the info, but given the variability (and as @frlan showed the fact that derived distros may show as the base distro) its probably not much use to a plugin.
Also, I don't see why a plugin would need that info if it's not an actual mean of identifying the platform (and even then, it's probably not a good metric to do anything). As is it's really meant as a human-readable clue rather than something formal. So ATM I don't see any reason for it to make it to plugin API anytime soon.
No, but it has an API version in the @since tag, which doesn't make much sense.
I just put that so it was easy to see when it was added, but agree the `(API 239)` part doesn't make a whole lot of sense.
@since sorry @codebrainz didn't mean to ping you, he just forgot to quote some Doxygen syntax.
Updated according the comments, squashed, rebased, and force pushed.
@elextr approved this pull request.
LGBI now
Should we merge this last minute, or wait till next release? I don't mind either way, it's not like it's a critical enhancement.
I'd like to have it merged. It will help users to help us when handling bug reports and the change is pretty harmless.
Merged #2618 into master.
github-comments@lists.geany.org