flood
flood copied to clipboard
Added filter by filesystem location + bonus tooltip on overflowed filter names
New filter section that allows viewing space usage/torrent count by directory in tree format. Filter names overflow with ellipses, and in the case of root directories, this is common: so I added a title-based tooltip to filter elements where the text is overflowed. Only appears on overflowed elements, and on mouse event. Not sure if equivalent touch events would work the same way.
No issue submitted, just something I wanted to make.
- [ ] Breaking change (changes that break backward compatibility of public API or CLI - semver MAJOR)
- [x] New feature (non-breaking change which adds functionality - semver MINOR)
- [ ] Bug fix (non-breaking change which fixes an issue - semver PATCH)
Codecov Report
Attention: 2 lines
in your changes are missing coverage. Please review.
Comparison is base (
1510940
) 72.94% compared to head (916f8de
) 73.23%.
Files | Patch % | Lines |
---|---|---|
server/services/taxonomyService.ts | 97.67% | 1 Missing :warning: |
server/util/fileUtil.ts | 50.00% | 1 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## master #518 +/- ##
==========================================
+ Coverage 72.94% 73.23% +0.28%
==========================================
Files 62 62
Lines 11375 11409 +34
Branches 951 969 +18
==========================================
+ Hits 8298 8355 +57
+ Misses 3063 3040 -23
Partials 14 14
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
This is brilliant, thanks for working on this.
The general idea looks OK, but the operations could get prohibitively expensive when there are many entries.
Open to other algorithms, but I can attest it takes minimal amount of time more than master with ~6k torrents.
And with 10^5 directories (actual insanity IMO); 10 directories each with 10 children, each with 10 children, and so on, 5 deep.
Would it be worth making a default-disabled setting to enable the feature?
Open to other algorithms, but I can attest it takes minimal amount of time more than master with ~6k torrents.
And with 10^5 directories (actual insanity IMO); 10 directories each with 10 children, each with 10 children, and so on, 5 deep.
Numbers look acceptable, but I would assume there is a non-trivial amount of associated CPU usage. I am reluctant to add a potentially expensive operation to the poll routine. We can probably employ more memoization.
Would it be worth making a default-disabled setting to enable the feature?
Not a good idea when the server-side functions are involved. There could be API concerns.
I am tied up with some other matters at hand, and I will take a deeper look next week.
When will it be merge? Expect this awesome feature
can you resolve conflicts?
I can, are you looking to actually merge it? (don't wanna play the fix/wait/fix/wait game)
I can, are you looking to actually merge it? (don't wanna play the fix/wait/fix/wait game)
yes
Should be good now, also changed the mouseover to a hook, should be nicer.
give me some time to test this locally
this doesn't look right, in my understand shouldn't this display like a tree or multiple trees? but some non-root elements are displayed at top level
download dir here are /srv/dev-disk-by-*/.../...
and /export/3t-1/...
also, just a question, is it possible to build trees at client instead of server like tags?
shouldn't this display like a tree or multiple trees?
I'll have to take a look at it again, as it's been a while; but iirc the trees are built by comparing common paths, removing prefixes, and traversing down. I think the paths come from those reported on the torrent metadata.. so is it possible those torrents are using relative paths or something starting with the hex?
Which root path should those non-root elements be under?
is it possible to build trees at client instead of server like tags?
Probably? I'm guessing I mimicked something that needed to be server-side, but again it's been a while. I'll have to take a look at what metadata ends up on the client.
so is it possible those torrents are using relative paths or something starting with the hex?
Which root path should those non-root elements be under?
no, they are all abs path.
My torrents' download directories:
-
/srv/dev-disk-by-uuid-58b9efe4-428e-40d1-8d3f-97d8a5bac12a/u2-small/{hex}
-
/srv/dev-disk-by-uuid-3c509d6a-504e-4d71-a19d-5f81c62d9902/...
-
/srv/dev-disk-by-uuid-41a30911-024f-4bd6-bb21-2aa90bed52e9/...
-
/srv/dev-disk-by-uuid-58b9efe4-428e-40d1-8d3f-97d8a5bac12a/...
-
/export/3t-1/downloads/...
so all hex elements and elements starts by dev-disk-by-uuid
should be non-root elements, only /srv
/ and /export/3t-1/downloads
should be root element.
Very odd. I'll have to take another look and make some test cases when I get a chance.
looks like I have mixed case directories and they cause this bug:
-
/srv/dev-disk-by-uuid-58b9efe4-428e-40d1-8d3f-97d8a5bac12a/U2-small/e9b3a6c75170e0c14701e7095b94412cfd4c7c7d
-
/srv/dev-disk-by-uuid-58b9efe4-428e-40d1-8d3f-97d8a5bac12a/u2-small/00ac7be5eb069dd8af992fca3a40b3755aaad846
console.log(
[
'/srv/dev-disk-by-uuid-58b9efe4-428e-40d1-8d3f-97d8a5bac12a/u2-small/231d4f952201ba96628a9520740554c6ae6837db',
'/srv/dev-disk-by-uuid-58b9efe4-428e-40d1-8d3f-97d8a5bac12a/U2-small/232e75cf79187a07a7507f82275f1e8d3f7ddf73',
'/srv/dev-disk-by-uuid-58b9efe4-428e-40d1-8d3f-97d8a5bac12a/u2-small/2364ebde4155eaf3267af1011b4dbc9cc75d99e1',
].sort((a, b) => {
return a.localeCompare(b);
}),
);
=>
[
'/srv/dev-disk-by-uuid-58b9efe4-428e-40d1-8d3f-97d8a5bac12a/u2-small/231d4f952201ba96628a9520740554c6ae6837db',
'/srv/dev-disk-by-uuid-58b9efe4-428e-40d1-8d3f-97d8a5bac12a/U2-small/232e75cf79187a07a7507f82275f1e8d3f7ddf73',
'/srv/dev-disk-by-uuid-58b9efe4-428e-40d1-8d3f-97d8a5bac12a/u2-small/2364ebde4155eaf3267af1011b4dbc9cc75d99e1'
]
lgtm, only one minor problem
I have a reduced solution that can probably be moved client-side; but case-insensitivity causes problems when filtering torrents. I think the solution is for the tree to be client-side, and the server can tell the client if the OS it's running on is case-insensitive or not, allowing insensitive tree and filtering
The current solution means client has no clue on insensitivity, so filtering on non-matching lowercased names omits torrents incorrectly.
Another option would be to always treat everything as case sensitive, and you'd end up with duplicate paths where casing doesn't match.
But it's late, so I'll give it a shot tomorrow.
I have a reduced solution that can probably be moved client-side; but case-insensitivity causes problems when filtering torrents. I think the solution is for the tree to be client-side, and the server can tell the client if the OS it's running on is case-insensitive or not, allowing insensitive tree and filtering
The current solution means client has no clue on insensitivity, so filtering on non-matching lowercased names omits torrents incorrectly.
Another option would be to always treat everything as case sensitive, and you'd end up with duplicate paths where casing doesn't match.
But it's late, so I'll give it a shot tomorrow.
after reviewing the code I think there is no need to move it to client side, current code looks fine
handling case-insensitivity may introduce too much extra complexity,which will need to handle every component of path,I think current code works fine
I looked a bit at moving the treeing to client-side:
I'm guessing there's some reason for building the tag list and other sidebar filters server-side, whether it's shared efficiency for multiple clients, the taxonomy diff data savings, or something else. I reduced the complexity of computing the tree and just stored the counts/data size in-tree, so it should be a little faster/simpler overall.
There may be an argument for moving filter value computation to the client, but I think matching the existing sidebar filter logic makes more sense for now.
please do not force push
I don't see what's failing in the tests. It names the new test file but there's no failing test or other message. It passes locally. Github doesn't have a way to rerun workflows?
please do not force push
It's in my repo and rebasing creates better changeset history, why not?
please do not force push
It's in my repo and rebasing creates better changeset history, why not?
Force-push causes inconvenient in code reviewing