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

Display free disk space below directory pickers (Add & Move torrent) #127

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jpovixwm
Copy link
Contributor

Shown in red if not enough free space is available:

I've hardcoded the refetchInterval to 5 seconds. Should I make it configurable?
I had some doubts whether the implementation should try to be "smart" about the paths or just call the RPC method with whatever path we've got and don't care about it.
For example, an empty string passed as the path returns an error with transmission running on Windows. Not sure about other platforms.
Also, inexistent directories will also return an error, so we could potentially traverse the path upward until we get something, but this gets messy once we consider the possibility of the path containing symlinks.
So this would perhaps better be fixed in transmission itself, and not in a GUI client.

@qu1ck
Copy link
Member

qu1ck commented Dec 23, 2023

There are a few issues with implementation but I don't think it is worth discussing at the moment because unfortunately this will have to be shelved for now. Transmission's implementation of free space RPC is just not usable and until it's improved this feature will have to wait.

It is very common to set download directory to one that does not yet exist and in that case you get this:
image

There is no good way to fix it client side:

  1. Don't show the error or free space: confusing why sometimes free space doesn't show
  2. Traverse up until getting a response: can be slow, not reliable

I think this warrants a feature request on transmission side, I'll file one.

@qu1ck
Copy link
Member

qu1ck commented Dec 23, 2023

@jpovixwm
Copy link
Contributor Author

I see. Thanks for creating the issue in Transmission's repo.
The behavior you're seeing with inexistent directories doesn't match what I get on my end (Windows, Transmission 4.0.5):
image
The RPC call's response in this case is:

{
    "arguments": {
        "path": "C:/doesntexist",
        "size-bytes": -1,
        "total_size": -1
    },
    "result": "No error"
}

Which works because result !== "success" so the component can know the call errored out and falls back to the string "Unknown". No idea whether the behavior is platform-dependent or if it was changed in v4.1, as I didn't test with 4.1.

Anyway, I'm not in a rush here so it's fine if it has to wait. I've still got some ideas for improvements in other parts of the project.

@qu1ck qu1ck added the tracking-upstream Waiting for feature to be complete/finalized in transmission server label Dec 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tracking-upstream Waiting for feature to be complete/finalized in transmission server
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants