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

Feature absgraph #24

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open

Conversation

scrouthtv
Copy link

@scrouthtv scrouthtv commented Dec 4, 2020

On #23
This is only a wip, I want to keep track of various things during development. There are still things tbd:

  • translation of the new options in the help text & man page (I don't speak dutch or french so I can't do this)
  • support for absolute graphs in html output
  • support for absolute graphs in tex output
  • maybe also add an option the the rc file
  • what should we do with the summary line?

Anyways, here is a very basic version that works in text mode and showcases my idea.

This first commit showcases some basic functionality. It currently only
works in text mode and with color disabled. For this to work:
 - I added an agflag (*a*bsolute *g*raph) which is for now always set to
   1 (enabled)
 - I changed the signature of the print_bar method and their
   implementations to consume three doubles:
   the used & total space on this volume as well as the total space on
   the greatest volume
 - I added an maxfssize variable alongside the max variable that stores
   the greatest volume's total space which is calculated whenever the
   latter is updated (see util.c)
Every other impl other than the one in text is simply a stub for now.
I decided to go with -A, it is off by default (default behaviour
is the same as before)
I also added documentation in the english man page and help text,
translated versions still need to add documentation for this as I
speak neither french nor dutch :)
@scrouthtv
Copy link
Author

For the record, on my server it looks like this:

/dev/sdb1       [=================---]  85.1%    138.8G 931.5G /media/00      
/dev/mapper/vc1 [===========--       ]  91.7%     48.5G 585.8G /media/01      
/dev/mapper/vc2 [===============-----]  82.0%    164.6G 915.9G /media/02      
/dev/mapper/vc3 [========-----       ]  69.0%    181.4G 585.8G /media/03

or in wide mode:

/dev/sdb1       [===========================================-------]  85.1%    138.8G 931.5G /media/00      
/dev/mapper/vc1 [===========================----                   ]  91.7%     48.5G 585.8G /media/01      
/dev/mapper/vc2 [======================================----------- ]  82.0%    164.6G 915.9G /media/02      
/dev/mapper/vc3 [====================-----------                   ]  69.0%    181.4G 585.8G /media/03      

I think it worked out rather nicely. Am going to continue work on the other exports, have a nice evening.

@scrouthtv scrouthtv mentioned this pull request Dec 4, 2020
@scrouthtv

This comment has been minimized.

@scrouthtv
Copy link
Author

Here is another thing I've noticed (this is with upstream dfc):

dev              [--------------------]   0.0%      2.8G   2.8G /dev           
run              [=-------------------]   0.0%      2.8G   2.8G /run  

Notice how both have 0.0% usage but the first one displays only dashes while the second row shows a single = followed by dashes.

This is because the 0.0 % of the first are accurately 0.0000 % while the second one is 0.04 % which gets rounded down on output.

I don't know whether that's the desired behaviour.

@scrouthtv
Copy link
Author

Also, I don't have any experience using cmake.
At some point during the build it changes things in the /po/*.po:

diff --git a/po/fr.po b/po/fr.po
index 1faabb9..14253d7 100644
--- a/po/fr.po
+++ b/po/fr.po
@@ -7,7 +7,7 @@ msgid ""
 msgstr ""
 "Project-Id-Version: dfc 3.1.1\n"
 "Report-Msgid-Bugs-To: [email protected]\n"
-"POT-Creation-Date: 2017-09-11 13:26+0200\n"
+"POT-Creation-Date: 2020-12-05 09:09+0100\n"
 "PO-Revision-Date: 2012-04-15 13:52+0200\n"
 "Last-Translator: Robin Hahling <[email protected]>\n"
 "Language-Team: French\n"
@@ -17,41 +17,43 @@ msgstr ""
 "Content-Transfer-Encoding: 8bit\n"
 "Plural-Forms: nplurals=2; plural=(n > 1);\n"
 
-#: src/dfc.c:214
+#: src/dfc.c:218
 #, c-format
... many more

Notice the change in the second-to-last-line. Should I commit these two or simply discard those changes?

@scrouthtv
Copy link
Author

scrouthtv commented Dec 5, 2020

There are still things to do but this'd be the first part

@scrouthtv scrouthtv marked this pull request as ready for review December 5, 2020 08:24
@scrouthtv
Copy link
Author

From what I can see, none of the comparable runtime flags can be set through the rc file, so I guess an option to set this new flag throught the rc file isn't wanted either.

in absolute graph mode, it is wider than the other bars, so I needed
support for flexible graph / bar width. before this commit, the color
steps in the absolute summary graph would be on the same level as the
ones above but not at the correct percentages, think something like this:
sda1 ggggggyyyyrr--
sda2 ggggggy-------
sda3 ggggggyy------
sum: ggggggyyyyrrrrrrrrrrrrrrrrr---------------
but it should look like this:
     ggggggggggggggggggyyyyyyyrr---------------
This issue is fixed in this commit.
@scrouthtv
Copy link
Author

d17a5df are the newly generated po files, I still don't know if the changes to them are needed. Discard the commit if not.

@scrouthtv
Copy link
Author

Ready for review @rolinh

@scrouthtv scrouthtv mentioned this pull request Dec 5, 2020
Copy link
Owner

@rolinh rolinh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this PR! I think the result is probably good only if there aren't large difference in capacity between the various volumes. I don't think I'd ever use this mode but I have no objection merging this PR.

[] translation of the new options in the help text & man page (I don't speak dutch or french so I can't do this)

I speak French and can take care of the translation later on. There is no need to include the changes to the po files with this PR. The idea is to update the po files before a release only except for adding new translation/missing translation strings.

I see a glitch when using the following options:

$ dfc -sTA
FILESYSTEM     TYPE     (=) USED      FREE (-)  %USED AVAILABLE  TOTAL MOUNTED ON     
dev            devtmpfs [-                   ]   0.0%     31.4G  31.4G /dev           
run            tmpfs    [=                   ]   0.0%     31.4G  31.4G /run           
/dev/nvme1n1p2 ext4     [==========----------]  48.7%    469.5G 915.4G /              
tmpfs          tmpfs    [=                   ]   0.6%     31.2G  31.4G /dev/shm       
tmpfs          tmpfs    [-                   ]   0.0%      4.0M   4.0M /sys/fs/cgroup 
tmpfs          tmpfs    [=                   ]   0.0%     31.4G  31.4G /tmp           
/dev/nvme1n1p1 vfat     [=                   ]   0.1%    510.7M 511.0M /efi           
tmpfs          tmpfs    [=                   ]   0.0%      6.3G   6.3G /run/user/1001 
tmpfs          tmpfs    [=                   ]   0.0%      6.3G   6.3G /run/user/1000 
SUM:                    [=========---------------]
                                         37.9%    608.0G   1.0T

It looks like the size of the bar for the sum is off. I haven't tested the other export types yet.

AUTHORS.md Outdated Show resolved Hide resolved
@rolinh
Copy link
Owner

rolinh commented Feb 3, 2021

@scrouthtv Are you still working on this?

@scrouthtv
Copy link
Author

Here it is.
I missed your comment on authors.md somehow. Tell me if there's anything else to fix.

@rolinh
Copy link
Owner

rolinh commented Feb 3, 2021

Tell me if there's anything else to fix.

Yes, what I reported above:

It looks like the size of the bar for the sum is off.

@scrouthtv
Copy link
Author

What do you mean by off?
I want the bar for the sum to be bigger than the rest (as big as all individual bars together) as all bars share the same scale.
In your example, each character represents ~ 25.8 GB so

  • /dev/nvme1n1p2 with 496 GB has 496 / 25.8 = 19 characters
  • total of 608 GB has 608 / 25.8 = 24 characters

That's why the total bar is bigger than the rest. And I think even in your example, its size is correct.

@rolinh
Copy link
Owner

rolinh commented Feb 4, 2021

Mmh I see. However, the columns after the bar are misaligned. It's probably just because the width of the sum bar is not taken into account to compute the column's width. This should be adjusted.

@scrouthtv
Copy link
Author

scrouthtv commented Feb 4, 2021

Ah, now I've got you. Looks like the 39 in text.c in line 172 is off. I will look into it.

48 is the correct value, but the other columns will still be off in wide mode.

As the columns are always in the same order, the offset I need adds up by

  • max.fsname
  • max.fstype
  • max.bar

@scrouthtv
Copy link
Author

scrouthtv commented Feb 4, 2021

Fixed:
image
Now I have to check if this is happening in other modes too.

@scrouthtv
Copy link
Author

scrouthtv commented Feb 4, 2021

Sorry for the slow work, I got drawn away by college stuff, should be done now.

@rolinh
Copy link
Owner

rolinh commented Feb 13, 2021

@scrouthtv Sorry for my late reply. I think there still are issues with alignment of the sum row:

% ./build/bin/RELEASE/dfc -As
FILESYSTEM     (=) USED      FREE (-)  %USED AVAILABLE  TOTAL MOUNTED ON     
dev            [-                   ]   0.0%     31.4G  31.4G /dev           
run            [=                   ]   0.0%     31.4G  31.4G /run           
/dev/nvme1n1p2 [========------------]  37.1%    575.4G 915.4G /              
tmpfs          [=                   ]   0.2%     31.3G  31.4G /dev/shm       
tmpfs          [-                   ]   0.0%      4.0M   4.0M /sys/fs/cgroup 
tmpfs          [=                   ]   0.0%     31.4G  31.4G /tmp           
/dev/nvme1n1p1 [=                   ]   0.1%    510.7M 511.0M /efi           
tmpfs          [=                   ]   0.0%      6.3G   6.3G /run/user/1000 
SUM:           [=======----------------]
                                                28.0%    707.7G   1.0T

If dfc decides at runtime to only display short  bars instead of wide
bars, because the tty is too narrow, the `max.bar` still indicates
`GRAPHBAR_WIDE` but it should be `GRAPHBAR_SHORT`.
So for calculating the detail fields offset I am now using the ternary
thing instead of `max.bar`.

This requires another look into it because currently, the titlebar
is off if we are using the `-w` option and the tty is too narrow.
@scrouthtv
Copy link
Author

Sorry for my late reply ;) I have a lot going on with college at the moment.

I think I finally fixed it:

 ~ git rev-parse --short HEAD
5da773a

 ~ ./build/bin/RELEASE/dfc -As
FILESYSTEM       (=) USED      FREE (-)  %USED AVAILABLE  TOTAL MOUNTED ON     
dev              [-                   ]   0.0%      3.7G   3.7G /dev           
run              [=                   ]   0.0%      3.7G   3.7G /run           
/dev/sda1        [==-------           ]  18.0%    160.6G 195.9G /              
tmpfs            [=                   ]   9.4%      3.4G   3.7G /dev/shm       
tmpfs            [-                   ]   0.0%      4.0M   4.0M /sys/fs/cgroup 
tmpfs            [=                   ]   1.4%      3.7G   3.7G /tmp           
/dev/sdb1        [==------------------]   5.1%    434.1G 457.4G /data          
/dev/mapper/home [==--------          ]  12.9%    187.8G 215.5G /home          
tmpfs            [=                   ]   0.2%    761.1M 763.0M /run/user/1000 
SUM:             [==-------------------------------------]
                                          4.8%    797.8G 884.5G
 ~ ./build/bin/RELEASE/dfc -Asw
FILESYSTEM       (=) USED                                    FREE (-)  %USED AVAILABLE  TOTAL MOUNTED ON     
dev              [-                                                 ]   0.0%      3.7G   3.7G /dev           
run              [=                                                 ]   0.0%      3.7G   3.7G /run           
/dev/sda1        [====------------------                            ]  18.0%    160.6G 195.9G /              
tmpfs            [=                                                 ]   9.7%      3.4G   3.7G /dev/shm       
tmpfs            [-                                                 ]   0.0%      4.0M   4.0M /sys/fs/cgroup 
tmpfs            [=                                                 ]   1.4%      3.7G   3.7G /tmp           
/dev/sdb1        [===-----------------------------------------------]   5.1%    434.1G 457.4G /data          
/dev/mapper/home [====--------------------                          ]  12.9%    187.8G 215.5G /home          
tmpfs            [=                                                 ]   0.2%    761.1M 763.0M /run/user/1000 
SUM:             [=====--------------------------------------------------------------------------------------------]
                                                                        4.8%    797.7G 884.5G

Here is a bug I noticed: If the tty can only fit normal graph mode, but wide graph mode is passed as a cli parameter, the title bar is also off:

 ~ git checkout v3.1.1
 ~ git rev-parse --short HEAD
e5f8c3b
 ~ make
// Now resize the terminal to around 100 columns
 ~ ./build/bin/RELEASE/dfc -w
WARNING: TTY too narrow. Some options have been disabled to make dfc output fit (use -f to override).
FILESYSTEM       (=) USED                                    FREE (-)  %USED AVAILABLE  TOTAL MOUNTED ON     
dev              [--------------------]   0.0%      3.7G   3.7G /dev           
run              [=-------------------]   0.0%      3.7G   3.7G /run           
/dev/sda1        [====----------------]  18.0%    160.6G 195.9G /              
tmpfs            [===-----------------]  10.2%      3.3G   3.7G /dev/shm       
tmpfs            [--------------------]   0.0%      4.0M   4.0M /sys/fs/cgroup 
tmpfs            [=-------------------]   1.4%      3.7G   3.7G /tmp           
/dev/sdb1        [==------------------]   5.1%    434.1G 457.4G /data          
/dev/mapper/home [===-----------------]  12.9%    187.8G 215.5G /home          
tmpfs            [=-------------------]   0.2%    761.1M 763.0M /run/user/1000 

I'd say that this can be fixed by changing the value of max.bar in util.c#auto_adjust() (depending on which path was taken).

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.

2 participants