Free space indicator

[taem]

Hi,

I made patch which adds free space indicator for session default dir and torrent one, as well.

In second patch I introduced Torrent::getMySize and Torrent::setMySize to handle int64_t values. Maybe rewrite getSize and setMySize methods to work with int64_t variables?

Also, perhaps Formatter::sizeToString needs rewrite to accept signed integers. See #3914.

Thanks.

[taem]

This ticket depends on #3833.

[taem]

Hi,

Please find attached a refreshed patch. Please ignore previous patches.

Thanks.

[taem]

Hi,

Please find attached patch that refreshed against the trunk.

Thanks.

[jordan]

This looks pretty good!

I changed the patch to update the label when mySession emits the 'session updated' signal and to initialize the session's free space variable in its constructor.

Actually, adding this new label to the statusbar got me thinking about how the statusbar icons are grouped. I've tweaked the statusbar a bit s.t. the ratio, upload, and download labels are grouped together a little tighter, instead of the previous approach of having equal spacing between the statusbar items.

I'm mostly happy with r13883 and am open to feedback.

[taem]

jordan, looks good to me. Thanks.

[rb07]

The status bar is going to confuse users, it looks like its saying: "You have N torrents with M GiB", you have to look into the tool tip to see its free space, and it has nothing to do with the torrent's size (or selection size, or any of the other similar statistics most people are used to).

[livings124]

I agree. Just a size displayed with no context is confusing.

[jordan]

livings124, imo you're the UI expert here. What would you suggest?

A few ideas:

• add "free" or "free space" text to it

• remove it altogether

• move it to the preferences dialog near where the download directory is specified, same as the port check is located near where the port number is specified

[livings124]

I would go with either the first of last option. This also applies to #5214.

[jordan]

r13927: move the visible torrent count indicator so that it's not visually grouped with the freespace indicator.

r13928: add 'Free" to the end to give more context about what the freespace number means.

[rb07]

The "visible torrent count" disappeared, I don't have anything new on the filter bar.

[jordan]

rb07, try filtering some torrents :)

[rb07]

Replying to jordan:

rb07, try filtering some torrents :)

Oh, I see the change, a single zero in the "Show:" label, and the count is already on the filter drop-down, so it was redundant anyway... not really redundant since you got "N of M Torrents", and now there is no info about the total.

Isn't that label count useless? I mean the label only changes from "Show:" to "Show: 0" and "Show: 1", the number disappears when the count is 6.

[jordan]

No, I think you're misunderstanding it -- the visible count is shown when it differs from the total count. The intent there was to avoid showing redundant information, but I'm open to suggestions on this.

[rb07]

You're right, I (still) don't understand it.

I think you are wrong about "the visible count is shown when it differs from the total count", when it shows, the count is the same as the count on the drop-down... but I'm testing with very few torrents (1 to 6), maybe you are making reference to large lists of torrents (larger than the list shows at one time -- that would make more sense, "Show N:" [of] <filter> M).

[jordan]

Let me rephrase, I mean that the visible count is compared to the total number of torrents in the session, not to the number in the drop-down.

If I understand you right, you're suggesting we could hide the number if it matches either of the two visible in the activity/tracker filters?

[jordan]

I tried that out and think it's an improvement... I like the removal of the redundant number.

I've committed that iteration to trunk for Qt and GTK+

[rb07]

A nice improvement.

[taem]

Is there anything should I do in this ticket?

[jordan]

taem, maybe. I'm really happy with the code you've done so far and find the freespace feature to be useful, but I'm thinking about the statusbar's changes in transmission-qt 2.80 and wondering if the freespace indicator's placement could be improved.

• I like the new location of the Filtered Torrent Count Label. It used to be floating out in the middle of the statusbar. Now it has context next to the filter dropdowns on the filterbar.

• I like the new location of the Ratio Label. It used to be floating out in the middle of the statusbar. Its new grouping with the download & upload speeds matches the layout in the individual rows of the torrent list, especially in compact mode.

• And now we have the new Free Space Label, which is... floating out in the middle of the statusbar :)

You probably see where I'm going with this. Just like the Ratio and Filter Count labels, I'd like to find a way to give that freespace indicator some context.

1. It could go into the Preferences dialog next to the Download Folder selector, just as we do with the Port Check Button next to the Port Check Selector.

2. More importantly, we could also add it to the "Torrent Options" dialog that pops up when you're adding a torrent, since most of the time that's when you want to know how much space is free. It would be even better if we put it side-by-side with how much space the torrent will take up based on which files are selected. We'd need to modify the RPC to get the freespace for whatever directory the user selects, but getting the target folder's freespace when adding the torrent would be a nice usability win.

taem, rb07, anyone else have any thoughts on this?

[rb07]

Moving the indicator out of the status bar, into the Preferences:Download, and Open Torrent options (just below the file list, not below Destination folder) sounds good.

I would prefer this version instead of the current implementation.

[taem]

I agree with rb07.

[lucke]

It'd be nice to see in the "add torrent" dialog how much space is free or would be free after downloading the selected files, as it is in ktorrent or utorrent. However, I use transmission-daemon on my headless server and I find the current way very helpful - I can quickly see how much space is free in qtr, without having to use df in the terminal, and then decide if the torrent will fit or not, Putting it in the "add torrent" dialog wouldn't help me, because it doesn't even pop up. Using df/transmission-remote sounds easier than going to a non-default tab in the preferences dialog.

ps. I'd rather not open a separate ticket for an issue in work-in-progress, so I'll put it here: in the development version (of qtr}, ratio is truncated - by default I seed to 1.25, the goal is shown to be 1.2.

[jordan]

first draft of this is in r13991. Feedback welcome.

I'm a little unsure about the placement in the options dialog. I kind of prefer rb07's suggestion from comment:26 but haven't implemented it yet.

[lucke]

Wouldn't it perhaps make sense to show free space in the statusbar in qtr remote and webclient, where one usually does not get to see the options ("open torrent") dialog? To me, the indicator in the preferences seems more like a way to see the amount of free space when changing download locations and not to monitor free space on local or remote systems.

[jordan]

Replying to lucke:

Putting it in the "add torrent" dialog wouldn't help me, because it doesn't even pop up. Using df/transmission-remote sounds easier than going to a non-default tab in the preferences dialog.

It sounds like the problem here is the lack of the pop up, not the statusbar.

[jordan]

Xref: #5284

[lucke]

I got a notification of a substantial answer of yours to my comment, jordan, yet I don't see it here. You asked in it for an use case.

I wrote about problems with qtr and the option/add torrent dialog in #5280 and #5281.

Let me tell you how I use transmission. I have transmission-daemon running on a headless server, I control it with qtr unless I don't have it handy, then I use the webclient or transmisson-remote directly on the server. I normally access the download dir via nfs. My disk on the server is usually full, because I remove torrents and their files only when I need space. Thus whenever I want to add a torrent (with its size denoted on a website) to transmission, I open a new session in my terminal, run "df -h", scan for my nfs share to see how much space is free, if not enough, I try to make space or decide not to download that torrent.

Nowadays I use firefox and its extension which simply adds torrents to transmission-daemon via rpc. qtr is running most of the time, so it's not a problem for me to copy the link and add use "Open URL", and I'd especially like to do that for selective downloads, but currently it doesn't show the options dialog. I can make firefox open torrent files with qtr, but then it kindly notifies me it downloaded the file, which is a bit jarring. I can make firefox open magnet links with qtr, but then I get the "Open URL" dialog which does not show the options dialog, and magnetized torrents currently don't show the options dialog anyway. And I never saw any dialog letting one choose what files to download on adding the torrent in the webclient. So, it's "df -h" for me. (I have to ssh into the server to run that when I'm using the webclient.)

Being able to quickly see how much space I have available in qtr, then adding torrents via rpc made me happy. If the options dialog was shown for "Open URL" and magnet links, then it'd be a step forward from 2.76 for me. But the problem in the webclient would remain, I presume, unless the options dialog was added.

[reardon]

Did I misread 13991? Why was the existing RPC method _removed_?? I understand adding functionality, but why needlessly break existing clients? The 'download-dir' concept still exists, it is still a session variable, so download-dir-free-space still makes sense.

[jordan]

lucke: the web client's "Add Torrent" dialog now lets you specify the download directory and has a freespace indicator (#5214) and transmission-qt now shows a freespace indicator when adding via URL or Magnet Link (#5280), so that handles most of the cases you outlined.

Freespace info is at its most useful when you're adding a torrent and trying to figure out if you have room. I'm not interested in using the statusbar to constantly monitor a directory's freespace. IMO a third party tool would be more appropriate for that.

[jordan]

reardon, I disagree with it being a useful variable. However, it's a fair point about avoiding breakage.

r14009 reinstates the field, marks it as deprecated, and documents the timetable for its future removal.

[reardon]

Replying to jordan:

Freespace info is at its most useful when you're adding a torrent and trying to figure out if you have room.

Hmm, most of the time I am adding via RSS or some other automated tool. I periodically check freespace status just to make sure there is sufficient room, but rarely am I adding torrents manually and checking for available space. But I concede that even in the automated case a more sophisticated query should be supported.

[rb07]

Changeset 13991 is not portable, in platform.c: tr_device_info_create() you are using 2 new functions (getblkdev(), getfstype()) that break a build for Windows, and probably not needed as shown before with the code for handling quotas, and also as the previous free space indicator was working fine w/o them.

[taem]

Hi jordan,

Please find attached patch with small fixes.

Thanks.

[rb07]

Replying to rb07:

Changeset 13991 is not portable...

Changing platform.c :

@@ -1014,8 +1029,10 @@

   info = tr_new0 (struct tr_device_info, 1);
   info->path = tr_strdup (path);
+#ifndef WIN32
   info->device = tr_strdup (getblkdev (path));
   info->fstype = tr_strdup (getfstype (path));
+#endif

   return info;
 }

fixes that.

But there's another problem, probably introduced with freespace-label.cc and the timer in it: the whole GUI is very slow refreshing, very noticeable if you open a torrent's properties, tracker tab, and... nothing, its blank for several seconds (more than 5, maybe the mouse events make the info appear).

Also there is a detail in freespace-label.cc:

Index: qt/freespace-label.cc
===================================================================
--- qt/freespace-label.cc       (revision 14017)
+++ qt/freespace-label.cc       (working copy)
@@ -47,7 +47,7 @@
 {
   if (myPath != path)
     {
-      setText (tr("<i>Counting Free Space...</i>"));
+      setText (tr("<i>Calculating Free Space...</i>"));
       myPath = path;
       onTimer ();
     }

minor detail.

[jordan]

rb07, I've added your comment:40 patches in r14033.

However I'm not seeing the lag that you mentioned...

[taem]

jordan, please add https://trac.transmissionbt.com/attachment/ticket/4076/0001-Misc-fixes.patch :)

[rb07]

Replying to jordan:

However I'm not seeing the lag that you mentioned...

Me neither, it seems to be a glitch in my build environment (I rebuild everything after some strange crashes -- after uptating jpeg Qt started crashing; after updating curl... that one does crash Transmission because it has a real bug, I patched the latest curl version).

Anyway I just released 2.77 for Windows, so if there is a lag... run for cover ;-)

[rb07]

May I ask: could you lower the frequency on free space calculation?

Its really fast now, and probably costly if someone uses an USB key as storage.

Even better would be to refresh the value on demand, only when it is shown. But a lower frequency would be an improvement (I'm using 60 s instead of the 5 set originally -- but while using the debugger it did seem way faster than 5, making debugging with a break-point there impossible).

[rb07]

Two points:

1. Enhancement: don't show an error message, which for the user will be useless, and cryptic; instead maybe this:

Index: qt/freespace-label.cc
===================================================================
--- qt/freespace-label.cc       (revision 14042)
+++ qt/freespace-label.cc       (working copy)
@@ -82,7 +82,7 @@
   int64_t bytes = -1;
   tr_variantDictFindInt (arguments, TR_KEY_size_bytes, &bytes);
   if (bytes < 0)
-    str = tr("<i>Error: %1</i>").arg(result);
+    str = tr("<i>Unknown free</i>");
   else
     str = tr("%1 free").arg(Formatter::sizeToString (bytes));
   setText (str);

One interesting use case is Tr-Qt with a remote session to an older daemon. The error returned is "unknown method", or something like that.

2. Maybe bug: I think the GUI blinking, and loss of drop-down menu focus is caused by the refresh in this feature. In other words, the implementation is wrong, its changing the focus from what the user is doing every time the value refreshes... unlike the other values that are refreshing all the time in the status bar.

Since I changed the refresh interval, I don't see the problem as often, and it also helped moving the field to Preferences, but I still see the problem occasionally.

[jordan]

I agree with point 1, I'd even go further and say let's show nothing if freespace can't be determined. (r14062)

Regarding point 2, I see the loss of drop-downs occasionally too, but the executed signal should be safe. If it was a thread issue in session.cc, that should be handled by session.cc delegating work back to the Qt thread via onResponseReceived...

[rb07]

r14062 doesn't work as you expected, now it shows "Calculating Free Space..." when the free space is unknown.

[jordan]

Argh! facepalm

Thank you rb07 :)

[jordan]

Regression fixed in r14064.

[x190]

Jenkins: "regression reported by x190" s/b rb07.