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

Wrong values for TERM and WM #69

Closed
leo-arch opened this issue Dec 8, 2021 · 16 comments
Closed

Wrong values for TERM and WM #69

leo-arch opened this issue Dec 8, 2021 · 16 comments

Comments

@leo-arch
Copy link

leo-arch commented Dec 8, 2021

Hi @wick3dr0se. Just reporting a few issues:

  • This is the value I get for term: openrc----supervise-daemo---------x---openbox---xterm---b. No matter what terminal I use, the result is always the same (except for the terminal name, of course). I guess this might be related to the init system (I use openrc btw), but not sure. Editing the strip lines as follows did the trick for me:
init_strip="s/login//g;s/startx//g;s/\<x\>//g;s/xinit//g;s/systemd//g;s/openrc//g;s/init//g"
dewm_strip="s/dwm//g;s/openbox//g"
shell_strip="s/fish//g;s/bash//g;s/zsh//g;s/ash//g"
launcher_strip="s/latte-dock//g;s/krunner//g;s/supervise-daemo//g;s/b+//g"

Perhaps you should add something like misc_strip to add there miscellaneous stuff like supervise-daemo and b+.
you should also expand init_strip to include alternative init systems.
However, I think there should be a way to get terminal name in a cleaner, simpler way: stripping a string in this way will soon o later fail in some manner.

  • As to the de/wm field, I get plain nothing (using openbox). I edited de-wm_theme.sh as follows to make it work for me:
head /usr/share/xsessions/* | grep -im1 'names=\|name=' | sed 's/DesktopNames=//;s/Name//g;s/CLASSIC//g;s/Ubuntu//;s/ubuntu//g;s/Classic//g;s/GNOME//2g' | tr -d '=:-;\n'

As a last resource, though relying on external applications is not a good idea, you could fallback to wmctrl -m, if available, in case everything else fails.

  • Minor issue, but here it goes: the value for ram has an extra slash. I just removed the ending slash after %d in cur_mem (main script)

Hope it helps!

@cheyngoodman
Copy link
Contributor

I had to fix the term value too. The code has alternaterm which may be an alternative to the current term query, but returns the correct value for me more elegantly:

alternaterm=$(pstree -sA $$ | head -n1 | awk -F--- '{print $(NF-1)}') #This is here for testing as a replacement of the above variable.

For you, does this command return the terminal name? pstree -sA $$ | head -n1 | awk -F--- '{print $(NF-1)}'

If it works better than the current "find and strip" method, it might be worth switching to.

For the de/wm fix, I wonder if you have multiple wms installed will it show only the running wm or also other installed ones in /usr/share/xsessions ?

@leo-arch
Copy link
Author

leo-arch commented Dec 8, 2021

alternaterm works fine for me, and it's far better than the strip method.

As to the de/wm value, this command will retrieve information from the first .desktop file listed in xsessions dir. I cannot test it myself, but, logically, that's what the code does (-m1 flag for grep).

@wick3dr0se
Copy link
Owner

Hi, Thank you @leo-arch! I appreciate you sharing how to get it the correct eay for your system. Everything hasn't been tested enough even on Arch and I'm still not the best with BASH syntax. I'll test and add your changes! I've been working a lot this week, I'm sorry for the long delay. I get majority of tomorrow off and the weekend so I'll have a good amount of time to work on everything. If you can find your theme and share where it exist, that would help a ton! I really don't know where all themes exist even on Arch..I only use i3 and most people use GTK or Gnome themes it seems

@cheyngoodman Thank you so much for adding that!! I will test that soon and verify output with you guys. If it's good for me as well I will deelte the old code in master. I definitely despise the strip method currently being used lol

@leo-arch
Copy link
Author

leo-arch commented Dec 9, 2021

Hi @wick3dr0se. You're welcome!

As to my GTK theme, it's being rightly retrieved by sysfetch, don't worry. However, just in case, my theme name is taken from ~/.config/gtk-3.0/settings.ini.

Glad you have replaced the "strip method"; it's much better now.

Finally, for the de/wm value, the current code works fine only provided you have only one DE/WM. As soon as you face a system with more than one DE/WM, you have no way to tell which .desktop file in the xsessions dir is the currently used one. Sadly, the best, more secure approach here is to rely on wmctrl. There should be a better way, for sure: maybe taking a look at wmcrtl source code to find out what it actually does. But even then, it might rely on some API or library not accessible from the command line. I just don't know.

EDIT: Maybe you could take a look at some other sys info fetchers to see how they get this info.

@wick3dr0se
Copy link
Owner

@cheyngoodman

I had to fix the term value too. The code has alternaterm which may be an alternative to the current term query, but returns the correct value for me more elegantly:

alternaterm=$(pstree -sA $$ | head -n1 | awk -F--- '{print $(NF-1)}') #This is here for testing as a replacement of the above variable.

For you, does this command return the terminal name? pstree -sA $$ | head -n1 | awk -F--- '{print $(NF-1)}'

If it works better than the current "find and strip" method, it might be worth switching to.

Thank you for all your work! I ran the command you gave above and it prints my shell instead. I'm trying to work on getting this to properly work for all of us without a strip command but I don't think the -NF flag will work. I get print $2 to print my terminal within any shell but other process trees may have the terminal name in the 3rd slot instead which would cause issues there.

@leo-arch

Finally, for the de/wm value, the current code works fine only provided you have only one DE/WM. As soon as you face a system with more than one DE/WM, you have no way to tell which .desktop file in the xsessions dir is the currently used one. Sadly, the best, more secure approach here is to rely on wmctrl. There should be a better way, for sure: maybe taking a look at wmcrtl source code to find out what it actually does. But even then, it might rely on some API or library not accessible from the command line. I just don't know.

EDIT: Maybe you could take a look at some other sys info fetchers to see how they get this info.

Maybe I should split de and wm to their own categories so that we can print both if exist, which on some system I believe they do. Then that would cleanup that issue for you likely and I'll also work on adding wmctrl initially if command exist. I used it in the beginning of the project then dropped it to get rid of dependencies. I'm starting to realize now, it makes more sense to utilize the dependencies if they are there. They output way better results than scouring several files. Thanks for your help! I will definitely be working on this today

wick3dr0se added a commit that referenced this issue Dec 24, 2021
@wick3dr0se
Copy link
Owner

@cheyngoodman I wrote a different method to get terminal value on commit 4ecb274. Hopefully it works as expected

@leo-arch I did add wmctrl as you suggested, a while back but looking in to this more; I see what you mean about having multiple environments with xsessions being an issue. I have a plasma.desktop and an i3.desktop. Running i3wm will output Plasma X11 for me. I'm not sure there is a viable method to return the operating distro from xsessions. I may have to remove the code for that all together

@leo-arch
Copy link
Author

I @wick3dr0se. Lots of nice improvements: everything looks more organized and robust. However:

  • term still doesn't work for me; I get just a blank
  • Colors aren't honored in Aterm

As to the script itself:

  • What's the meaning of the compiled field for pacman? I have a lot of custom scripts in /usr/local/bin and, as scripts, none of them is compiled.
  • It's becoming quite slow (slower than neofetch, for example) considering what it does. Consider working on some optimizations. For example:
    • Whenever possible, avoid using ls(1) to get a list of files: it might be slowing things down. Consider using a for loop instead.
    • Avoid pacman -Q to get the amount of installed packages: /usr/lib/pacman/local could a better alternative. Btw, running just pacman -Qq instead of pacman -Qn and pacman -Qm is much faster.

@wick3dr0se
Copy link
Owner

wick3dr0se commented Dec 24, 2021

@leo-arch Hey, thank you!! The 'compiled' field actually just prints as src meaning from source, which is proper but if you have a better name for output, please suggest! I'm so bad with naming conventions; I'm terribly uncreative though lol. The compiled variable is just incorrect, I didn't think about scripts existing. I'll update the variable name. The color issue could be because I used \x1b instead of \033 but I'm not sure theres a difference. I'm against using tput because then we are just adding an uncessary dependency to ncurses.

Keep in mind that Neofetch, although having more code, outputs quite a bit less information. I don't look at Neofetch's code because it's a proper mess. If read is faster than shell commands like sed/awk/grep/cat/ls, etc then I can replace that. The main reason I haven't is due to the code it takes to replace a simple awk line. Well that and I'm still learning a TON! If it's faster then I would rather go that route though. Did you benchmark test this? I just tested this opening two terminals at once and running the commands simultaneously and neofetch initiates right before but has a slow animation and ultimately ends after sysfetch does. I'm not sure how it initially shows up quicker but I do not like how Neofetch has like a glitchy, tearing affect before full output. I will definitely be making all the changes you suggest so we can focus on speed!

Edit: You are so right about pacman -Qn. It is significally slower than Qq. I feel stupid for listing package versions to get a number of installed packages lol

@leo-arch
Copy link
Author

\x1b and \033 are the same value (hex and octal respectively), namely, the escape char (27 decimal). The three of them should work interchangeably, but depends on the interpreter (mostly the shell).

Didn't make any benkmarking: it's slow for the naked eye. A few optimizations will do the trick, don't worry.

As to pacman the bottleneck is not so much the version flag (-q), but the -n flag: it filters the full list of packages to list only native packages (excluding AUR and homemade scripts/binaries). pacman -Qq is significantly faster, but it lists both native and AUR packages at once.

@wick3dr0se
Copy link
Owner

I was just about to update my comment again with my findings. I realized that I was using the -Qn flag to list natively installed packages so we can seperate the AUR packages. I could wrap the output of the pacman -Qqm to strip those names from the output of pacman -Qq

@leo-arch
Copy link
Author

leo-arch commented Dec 24, 2021

That's possible, of course, but it won't come without performance penalties.

Faster way to get total installed packages (native and AUR):

ls /var/lib/pacman/local | wc -l

Another way:

for pkg in /var/lib/pacman/local/*; do
    [[ n++ ]]
done
printf "%d" "$n"

Recall to substract the number of no-package files, like ALPM_DB_VERSION

Using time(1):

ls method: 0.008 secs
pacman -Qq | wc -l: 0.135 secs

As you can see, the ls method is a lot faster!

@wick3dr0se
Copy link
Owner

Sorry, I'm typing from Android and my lady is a little talkative today. It's crazy how much you know.. I'm not sure what the best method to output color is at all. ANSII escape characters is the only way I know how. I would be more than open to a different method. I'll have to learn more about it.

Thanks for the code snippet! I always learn a lot more when people share their ideas. I made this just to learn BASH and because I wanted a more featured output than Neofetch. Your knowledge of BASH and Linux in general is so far superior to mine lol. I'm not sure what ALPM_DB_VERSION is and that's the first time I've seen time. Defintely using this in some test I'm working on!!

@leo-arch
Copy link
Author

leo-arch commented Dec 24, 2021

As to printing colors, escape sequences is the standard and desired way: stick to it. My guess is that you're using 256 colors, while Aterm only supports 16 colors.

Keep learning!

EDIT: if you want a truly portable script, stick to 16 colors. Or, check how many colors the terminal supports first, and then print 16 or 256 colors accordingly.

@leo-arch
Copy link
Author

leo-arch commented Dec 24, 2021

Yeap, you're using 9x colors (\x1b[1;91, for example), which ain't supported by Aterm. Just replace 9 by 3, say, write \x1b[1;31m instead of \x1b[1;91m to get bold red. 31 is not as bright as 91, but it's more portable.

@wick3dr0se
Copy link
Owner

Ok good! I'll look into 16 color escape characters. Another change I made was on the end of the color sequence, I changed it from 31m, 32m, 33m, etc to like 91m, 92m, 93m, so that could be different. Love the idea to look for 256 color support then output full color.

You're 1 step ahead of me everytime lol. I will make that change! I can't remember the difference in color between 3 and 9 now. I know for me 3 looks the same

@wick3dr0se
Copy link
Owner

wick3dr0se commented Dec 24, 2021

If you have any suggestions for terminal, feel free to send them my way. I tried to get parent PPID but I can't figure out how to do it properly. This is the code I tried:
ppid=$(awk -F ' ' '/PPid/ {print $2}' /proc/$PPID/status)
ppid=$(ps -o ppid= "$ppid")
term=$(pstree -s -p "$ppid")

Current method that failed for you, greps the output of pstree -hp from shell and then attempts to print the first process running on that line

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

No branches or pull requests

3 participants