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

Change memory rock default full text #94

Merged
merged 5 commits into from
Dec 16, 2020

Conversation

matthewrossi
Copy link
Contributor

Memory rock default full text shows memory available, while swap shows the one currently in use.
This aligns the default full text to the swap behavior.

It may be a matter of preference, but I feel like showing the memory currently in use is what a user would expect from this rock.

@kgilmer kgilmer requested review from kgilmer and cheginit December 10, 2020 16:06
@kgilmer
Copy link
Member

kgilmer commented Dec 10, 2020

Hi @matthewrossi , thank you for your contribution! The net-traffic and cpu also relay the the amount used, rather than free. But, on the other hand the battery indicator relays the amount remaining. I like that your change makes memory more consistent w/ CPU, but on the other hand there is value to knowing how much memory is free over how much is taken: low memory situations is what I think most people are concerned with. If the metric emits "memory used" then the user must do the math to figure out how much is left. If the metric emits the amount free, then it's clear. Ideally I suppose converting this to a percentage would be most consistent w/ CPU and provide an objective metric w/out requiring user to do math. What do you think?

@matthewrossi
Copy link
Contributor Author

Hi @kgilmer, I did not thought about it, but I totally agree, showing the amount of free memory highlights low memory situations way more than memory used would. The use of the percentage to show current memory usage could help with that, but it might also make CPU and memory blocks harder to distinguish. Do you think this might be an issue?

g
%

@cheginit
Copy link
Contributor

I agree with @kgilmer that total available memory is more useful. I think we can add an option to switch to percentage instead changing the default behavior. A flag that can be passed to script or based on an i3 xresource key.

@kgilmer
Copy link
Member

kgilmer commented Dec 11, 2020

@matthewrossi we can always take a look and find another icon that is more representative of "ram". The generic "IC chip" icons can only go so far I guess haha. FWIW others have requested visualizing ram as percent free, so I think it's worth doing if someone has the ability and time.

https://github.com/regolith-linux/regolith-desktop/issues/514

@matthewrossi
Copy link
Contributor Author

Hi, I have added some changes based on your feedback. It is now possible to select what the memory block shows depending on the display and percentage variables (both variables can be specified within the conf.d or using Xresources).

The memory displayed depends on the display variable or i3xrocks.memory.display:

  • free: shows the memory free (default)
  • used: shows the memory used

Showing the memory as percentage depends on the variable percentage or i3xrocks.memory.percentage:

  • false: shows the memory in gigs (default)
  • true: shows the memory as percentage

By default the memory block shows the free memory as gigs, same as before.

@cheginit
Copy link
Contributor

Thanks for the new commit!

I tried to set the percentage to true but it's still showing it in G. Am I missing something? I set:

i3xrocks.memory.percentage: true

In my Xresource file but it doesn't seem to work.

@matthewrossi
Copy link
Contributor Author

Your configuration is correct. So I reproduced it, it looks like what was working with display, did not with percentage so the percentage variable was always looking empty when set using Xresources.
After multiple trials with some small changes in the percentage assignment I was honestly about to give up as to me that line had no reason not to work. As the whole thing was not having any sense, I decided to go for a last nonsense change, something it should produce no effective change, and therefore not solve the problem, I moved the percentage assignment up one line. Well, that solved it...at least for me...
I have committed the change, let me know if it works also for you.

BTW why is this change working? It doesn't make any sense to me.

@cheginit
Copy link
Contributor

Yep, it works now! I am not really sure,

I found out what's the issue. This display line throws an error:
image

So when you first try to set the display it throws this error and doesn't go to the next line to read the percentage. I wonder why's it throws this error?

@cheginit
Copy link
Contributor

I figured the fix. As soon as you change the DISPLAY variable which is an environmental variable the system throws this error, so if you change DISPLAY to anything else it works.

@kgilmer
Copy link
Member

kgilmer commented Dec 16, 2020

Great, thanks for sticking through with this one @matthewrossi ! And thanks for your help and review @cheginit . Matthew if you're interested in a few Regolith laptop stickers I'll mail you some (international is OK) I just need a postal address. You can email it to me at kgilmer at gmail.com.

@kgilmer kgilmer merged commit fc9469a into regolith-linux:master Dec 16, 2020
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.

3 participants