Enhance (main/fastfetch) Add DE & WM Detection #25266
Conversation
|
This look ready, I've tested it on aarch64 and I'm sure it will work elsewhere. |
There was a problem hiding this comment.
As far as I can tell this is pretty much perfect. I tested it on an x86_64 redroid. It installs normally, and shows my real computer's monitors (the running kernel is connected to a separate session outside docker using those four monitors, so somehow it has detected that from within docker). Then, after installing Tigervnc and xfce, launching a VNC server, and starting xfce, it automatically shows the DE correctly in the printout.
|
Actually I just noticed this is 11 commits. (This is a pre-written, saved reply.) Since squashing or amending commits changes the git history you will need to force push any such changes. |
|
Hello, I'm against this, especially the removal of displayserver_android.c. I feel affronted. Please don't do this. Thanks. |
Yeah that seems counterproductive. Edit: for context, this is the main upstream developer of |
Thank you for the explanation. Would it be ok for me to ask if there's a specific reason it's not ok to remove that? For example, just for some context, this is the problem we're trying to fix here:
our package of Do you know of a good way to improve this version so we don't need to remove certain code? |
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
I removed displayserver_android.c because it conflicts with displayserver_linux.c (in particular, ffConnectDisplayServerImpl). And, if I was to remove the conflicting function, I would only be left with an HDR check. and 2 unused functions As for the 2 functions, they only fill 3 somewhat specific use cases. being rooted devices, MiUI devices, and people who only access the terminal through adb |
|
It's fastfetch for android. We are talking about android system. The display server API is Display and the window manager API is WindowManager. We cant talk to it in C so I try to get some information by other way, hence displayserver_android. It works only in some specific environment but still better than nothing. I did my best. That's it. Sure you can install termux-x11 to run some x11 GUI program, however:
Say there is X11 compatibility layer in macOS (XQuartz) and Windows. You can even install xfce in macOS and run fastfetch inside it. Should fastfetch report xfwm/xfce as WM/DE in this case? No one ever made a feature request. Of course, some people want Linux DE in their Android phone. Although I dont get the use case to it in such a small screen (for fun maybe), it's ok. In this case, people make a full Linux subsystem in their phone so they should run Linux programs in it, not programs build for Android. This is my opinion. You can have your opinion. If you do want X11 support, you can argue more. It's ok. However you decided to bypass me, made the changes downstream directly to fulfill your requirement and removed some files randomly. I feel affronted. I absolutely hate this and this PR should be closed. |
I am not sure why the X11 detection features and the displayserver_android.c have to conflict. Maybe there is a way to patch it so that the code from both If that were hypothetically done, would that change anything about this affronting you?
I don't have mac but, I would guess |
|
I had made a change, so now if there is an X server running, then it will display info about the WM, if it's not, it will display "Window Manager" as before. |
Were you about to upload that version and just hadn't yet? If so then never mind, but just in case, when I test the current artifact here, it's true that the version in this PR currently doesn't show "
As I understand it, that is one of the major problems CarterLi has with this version, so I had been thinking of making a version myself that continues to show " If you were already trying that though, sorry I didn't realize. Let me know if you would like some help to try to make a version that's like that (basically one part will involve preserving the |
|
Sorry, its on my own fastfetch repo and forgot to get it here, ill do it now |
|
There is unfortunately now an error in CI related to the presence of a merge commit - merge commits are not allowed in termux-packages, so the branch will need to be rebased and force-pushed to remove it. I can see your version though, and it does look very interesting. I'll try adjusting it to see if I can make it any more concise and direct, keeping the number of lines of code changed to a minimum in order to maximize similarity to the vanilla version. |
|
Just tested the artifact and it works as intended as far as I can see.
Also, just to be sure, I built it for Ubuntu so that maybe it can be pushed upstream (but that's wishful thinking considering how he reacted here) and it worked fine there. |
|
Your version @Usinganame works quite well, but here is my version, which creates essentially similar behavior to your version, but with significantly fewer code edits.
my version--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -57,15 +57,15 @@ include(CheckIncludeFile)
include(CMakeDependentOption)
cmake_dependent_option(ENABLE_VULKAN "Enable vulkan" ON "LINUX OR APPLE OR FreeBSD OR OpenBSD OR NetBSD OR WIN32 OR ANDROID OR SunOS OR Haiku" OFF)
-cmake_dependent_option(ENABLE_WAYLAND "Enable wayland-client" ON "LINUX OR FreeBSD OR OpenBSD OR NetBSD" OFF)
-cmake_dependent_option(ENABLE_XCB_RANDR "Enable xcb-randr" ON "LINUX OR FreeBSD OR OpenBSD OR NetBSD OR SunOS" OFF)
-cmake_dependent_option(ENABLE_XRANDR "Enable xrandr" ON "LINUX OR FreeBSD OR OpenBSD OR NetBSD OR SunOS" OFF)
+cmake_dependent_option(ENABLE_WAYLAND "Enable wayland-client" ON "LINUX OR FreeBSD OR OpenBSD OR NetBSD OR ANDROID" OFF)
+cmake_dependent_option(ENABLE_XCB_RANDR "Enable xcb-randr" ON "LINUX OR FreeBSD OR OpenBSD OR NetBSD OR SunOS OR ANDROID" OFF)
+cmake_dependent_option(ENABLE_XRANDR "Enable xrandr" ON "LINUX OR FreeBSD OR OpenBSD OR NetBSD OR SunOS OR ANDROID" OFF)
cmake_dependent_option(ENABLE_DRM "Enable libdrm" ON "LINUX OR FreeBSD OR OpenBSD OR NetBSD OR SunOS" OFF)
cmake_dependent_option(ENABLE_DRM_AMDGPU "Enable libdrm_amdgpu" ON "LINUX OR FreeBSD" OFF)
cmake_dependent_option(ENABLE_GIO "Enable gio-2.0" ON "LINUX OR FreeBSD OR OpenBSD OR NetBSD OR SunOS" OFF)
cmake_dependent_option(ENABLE_DCONF "Enable dconf" ON "LINUX OR FreeBSD OR OpenBSD OR NetBSD OR SunOS" OFF)
cmake_dependent_option(ENABLE_DBUS "Enable dbus-1" ON "LINUX OR FreeBSD OR OpenBSD OR NetBSD OR SunOS OR Haiku" OFF)
-cmake_dependent_option(ENABLE_XFCONF "Enable libxfconf-0" ON "LINUX OR FreeBSD OR OpenBSD OR NetBSD OR SunOS" OFF)
+cmake_dependent_option(ENABLE_XFCONF "Enable libxfconf-0" ON "LINUX OR FreeBSD OR OpenBSD OR NetBSD OR SunOS OR ANDROID" OFF)
cmake_dependent_option(ENABLE_SQLITE3 "Enable sqlite3" ON "LINUX OR FreeBSD OR APPLE OR OpenBSD OR NetBSD OR SunOS" OFF)
cmake_dependent_option(ENABLE_RPM "Enable rpm" ON "LINUX" OFF)
cmake_dependent_option(ENABLE_IMAGEMAGICK7 "Enable imagemagick 7" ON "LINUX OR FreeBSD OR OpenBSD OR NetBSD OR APPLE OR WIN32 OR SunOS OR Haiku" OFF)
@@ -584,18 +584,32 @@ elseif(ANDROID)
src/detection/chassis/chassis_nosupport.c
src/detection/cpu/cpu_linux.c
src/detection/cpucache/cpucache_linux.c
- src/detection/cursor/cursor_nosupport.c
src/detection/cpuusage/cpuusage_linux.c
+ src/detection/cursor/cursor_linux.c
src/detection/disk/disk_linux.c
src/detection/dns/dns_linux.c
src/detection/physicaldisk/physicaldisk_linux.c
src/detection/physicalmemory/physicalmemory_nosupport.c
src/detection/diskio/diskio_linux.c
src/detection/displayserver/displayserver_android.c
- src/detection/font/font_nosupport.c
src/detection/gpu/gpu_nosupport.c
src/detection/host/host_android.c
- src/detection/icons/icons_nosupport.c
+ src/detection/displayserver/linux/displayserver_linux.c
+ src/detection/displayserver/linux/drm.c
+ src/detection/displayserver/linux/wayland/wayland.c
+ src/detection/displayserver/linux/wayland/global-output.c
+ src/detection/displayserver/linux/wayland/zwlr-output.c
+ src/detection/displayserver/linux/wayland/kde-output.c
+ src/detection/displayserver/linux/wayland/wlr-output-management-unstable-v1-protocol.c
+ src/detection/displayserver/linux/wayland/kde-output-device-v2-protocol.c
+ src/detection/displayserver/linux/wayland/kde-output-order-v1-protocol.c
+ src/detection/displayserver/linux/wayland/xdg-output-unstable-v1-protocol.c
+ src/detection/displayserver/linux/wmde.c
+ src/detection/displayserver/linux/xcb.c
+ src/detection/displayserver/linux/xlib.c
+ src/detection/font/font_linux.c
+ src/detection/gtk_qt/gtk.c
+ src/detection/icons/icons_linux.c
src/detection/initsystem/initsystem_linux.c
src/detection/keyboard/keyboard_nosupport.c
src/detection/libc/libc_android.c
@@ -613,20 +627,21 @@ elseif(ANDROID)
src/detection/packages/packages_linux.c
src/detection/poweradapter/poweradapter_nosupport.c
src/detection/processes/processes_linux.c
+ src/detection/gtk_qt/qt.c
src/detection/sound/sound_nosupport.c
src/detection/swap/swap_linux.c
src/detection/terminalfont/terminalfont_android.c
src/detection/terminalshell/terminalshell_linux.c
src/detection/terminalsize/terminalsize_linux.c
- src/detection/theme/theme_nosupport.c
+ src/detection/theme/theme_linux.c
src/detection/tpm/tpm_nosupport.c
src/detection/uptime/uptime_linux.c
src/detection/users/users_linux.c
- src/detection/wallpaper/wallpaper_nosupport.c
+ src/detection/wallpaper/wallpaper_linux.c
src/detection/wifi/wifi_android.c
- src/detection/wm/wm_nosupport.c
- src/detection/de/de_nosupport.c
- src/detection/wmtheme/wmtheme_nosupport.c
+ src/detection/wm/wm_linux.c
+ src/detection/de/de_linux.c
+ src/detection/wmtheme/wmtheme_linux.c
src/detection/camera/camera_android.c
src/detection/zpool/zpool_nosupport.c
src/util/platform/FFPlatform_unix.c
--- a/src/detection/displayserver/displayserver_android.c
+++ b/src/detection/displayserver/displayserver_android.c
@@ -145,7 +145,7 @@ static bool detectWithGetprop(FFDisplayServerResult* ds)
return false;
}
-void ffConnectDisplayServerImpl(FFDisplayServerResult* ds)
+void ffConnectDisplayServerImplAndroid(FFDisplayServerResult* ds)
{
ffStrbufSetStatic(&ds->wmProcessName, "WindowManager");
ffStrbufSetStatic(&ds->wmPrettyName, "Window Manager");
--- a/src/detection/displayserver/linux/displayserver_linux.c
+++ b/src/detection/displayserver/linux/displayserver_linux.c
@@ -43,6 +43,10 @@ static void getWMProtocolNameFromEnv(FFDisplayServerResult* result)
}
}
+#ifdef __ANDROID__
+void ffConnectDisplayServerImplAndroid(FFDisplayServerResult* ds);
+#endif
+
void ffConnectDisplayServerImpl(FFDisplayServerResult* ds)
{
if (instance.config.general.dsForceDrm == FF_DS_FORCE_DRM_TYPE_FALSE)
@@ -94,6 +98,11 @@ void ffConnectDisplayServerImpl(FFDisplayServerResult* ds)
//This fills in missing information about WM / DE by using env vars and iterating processes
ffdsDetectWMDE(ds);
}
+
+ #ifdef __ANDROID__
+ if(ds->displays.length == 0)
+ ffConnectDisplayServerImplAndroid(ds);
+ #endif
}
FFDisplayType ffdsGetDisplayType(const char* name)@CarterLi, I invite you to take a look at these revised patches, and in particular how achieving the desired behavior without compromising the behavior of the original, intended mode is possible with such few code changes. Do you still feel the same way about these other versions, that you did about the first proposed version? |
|
@robertkirkman You can make a PR in fastfetch repo. Before that, please make sure all things you enabled actually work. WM, DE, theme, icon, fonts, wayland, drm, etc. |
Thank you for the opportunity, I am just responding to let you know I am planning to make a PR, but I am also setting up MacOS to test Mac because, after what you said about MacOS, I believe fastfetch should be tested in all known XFCE distributions and it should be able to detect if running inside XFCE in any OS that XFCE currently exists precompiled for. This is my impression and what I plan to test, so that the PR can be uniform without an unfair treatment toward any particular platform.
|
Builds with
XCB_RANDR,XRANDR, andXFCONF. Also adds every file fromsrc/detection/displayserver/linuxAlso removes
src/detection/displayserver/displayserver_android.cI tried to get this merged upstream, but the PR was closed.