dylanaraps / neofetch

🖼️ A command-line system information tool written in bash 3.2+
MIT License
21.93k stars 1.76k forks source link

Enhancing --memory_unit functionality #2225

Open TriMoon opened 1 year ago

TriMoon commented 1 year ago

Description

Improving https://github.com/dylanaraps/neofetch/commit/0435dcd0cd44bb22afa9b986f15742cc05de7b20

Features

hykilpikonna commented 1 year ago

There are a lot of shellcheck warnings...

image
TriMoon commented 1 year ago

There are a lot of shellcheck warnings...

I'm editing online, will check/address those warnings later :wink:

hykilpikonna commented 1 year ago

I'm editing online, will check/address those warnings later 😉

I just fixed it for you:

    case $memory_unit in
        tib)
            mem_label=TiB
            memory_unit_divider=$((1024 * 1024))
        ;;

        gib)
            mem_label=GiB
            memory_unit_divider=1024
        ;;

        kib)
            mem_used=$((mem_used * 1024))
            mem_total=$((mem_total * 1024))
            mem_label=KiB
        ;;
    esac

    case $memory_unit in
        tib|gib)
            printf -v mem_used "%'.*f" \
                        "${mem_precision:-2}" \
                        "$((mem_used / memory_unit_divider)).$((mem_used % memory_unit_divider))"
            printf -v mem_total "%'.*f" \
                        "${mem_precision:-2}" \
                        "$((mem_total / memory_unit_divider)).$((mem_total % memory_unit_divider))"
        ;;
    esac

But I can't push to your branch though

TriMoon commented 1 year ago

Hahaha that's a way to do it also ofcourse, by creating an extra block.... Will think about it, let me first finish my addition of the precision documentation :wink:

I know i should have started with a "Draft" pull request but ahh well heheheh

hykilpikonna commented 1 year ago

Looking into this, I actually found another problem, which is that memory bars only work when the memory unit is MiB (the bar function takes in MiB values even though they have been converted).

Can be fixed by keeping track of an original mb value:

image
TriMoon commented 1 year ago

@hykilpikonna, i just noticed the current non-patched version does wrong arithmetic in it's attempt to convert KB into MiB, https://github.com/dylanaraps/neofetch/blob/0435dcd0cd44bb22afa9b986f15742cc05de7b20/neofetch#L2538-L2649

It does integer calculation instead of floating point... fe. 1000 KB should convert to 0.9765625 MiB, but current calculation does $((1000 / 1024)) = 0 in multiple places in above function.

I already know i will use the same kind of printf coding i used in here to do the necessary floating-point calculations in bash, probably will make it a function...

So I'm going to first correct that before going further, as it will change more stuff. (in due time when i get more time again)

TriMoon commented 1 year ago

Looking into this, I actually found another problem, which is that memory bars only work when the memory unit is MiB (the bar function takes in MiB values even though they have been converted).

Can be fixed by keeping track of an original mb value:

That won't be needed when we keep the original values in Bytes so we can display in more units as MiB-only like the current code does...

TriMoon commented 1 year ago

@hykilpikonna But I can't push to your branch though

Invited as collaborator :wink:

hykilpikonna commented 1 year ago

Invited as collaborator 😉

Thanks! I just pushed my commit fixing the memory progress bar issue

So I'm going to first correct that before going further, as it will change more stuff. (in due time when i get more time again)

Yea, I think creating a function for floating-point calculations would be nice, and we can keep the raw value in bytes or kib to reduce the number of calculations and conversions. Let me know when it's ready to merge!

TriMoon commented 1 year ago

@hykilpikonna Let me know when it's ready to merge!

These changes, with your fix could be merged first, because the floating point calculation functionality would be something completely different and extra to this MR :wink: Will create a new one especially for that in due time.

hykilpikonna commented 1 year ago

Okay, just merged into hyfetch!

HyFetch is a fork of neofetch with LGBTQ pride flags, but the repo also maintains an updated version of the original neofetch, addressing many pull requests that are not merged in the original repo.

Read the "Running Updated Original Neofetch" section for more info!