flood icon indicating copy to clipboard operation
flood copied to clipboard

Added filter by filesystem location + bonus tooltip on overflowed filter names

Open FinalDoom opened this issue 3 years ago • 27 comments

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.

image image

  • [ ] 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)

FinalDoom avatar Feb 20 '22 06:02 FinalDoom

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.

codecov[bot] avatar Feb 20 '22 06:02 codecov[bot]

This is brilliant, thanks for working on this.

mouzzampk2014 avatar Feb 20 '22 07:02 mouzzampk2014

The general idea looks OK, but the operations could get prohibitively expensive when there are many entries.

jesec avatar Feb 24 '22 01:02 jesec

Open to other algorithms, but I can attest it takes minimal amount of time more than master with ~6k torrents. image And with 10^5 directories (actual insanity IMO); 10 directories each with 10 children, each with 10 children, and so on, 5 deep. image

Would it be worth making a default-disabled setting to enable the feature?

FinalDoom avatar Feb 24 '22 07:02 FinalDoom

Open to other algorithms, but I can attest it takes minimal amount of time more than master with ~6k torrents. image And with 10^5 directories (actual insanity IMO); 10 directories each with 10 children, each with 10 children, and so on, 5 deep. image

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.

jesec avatar Feb 25 '22 21:02 jesec

When will it be merge? Expect this awesome feature

boeto avatar Nov 12 '22 04:11 boeto

can you resolve conflicts?

trim21 avatar Dec 17 '23 20:12 trim21

I can, are you looking to actually merge it? (don't wanna play the fix/wait/fix/wait game)

FinalDoom avatar Dec 19 '23 22:12 FinalDoom

I can, are you looking to actually merge it? (don't wanna play the fix/wait/fix/wait game)

yes

trim21 avatar Dec 20 '23 04:12 trim21

Should be good now, also changed the mouseover to a hook, should be nicer.

FinalDoom avatar Dec 21 '23 01:12 FinalDoom

give me some time to test this locally

trim21 avatar Dec 21 '23 01:12 trim21

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

image

image image

download dir here are /srv/dev-disk-by-*/.../... and /export/3t-1/...

trim21 avatar Dec 21 '23 11:12 trim21

also, just a question, is it possible to build trees at client instead of server like tags?

trim21 avatar Dec 21 '23 11:12 trim21

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.

FinalDoom avatar Dec 22 '23 00:12 FinalDoom

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.

trim21 avatar Dec 22 '23 07:12 trim21

Very odd. I'll have to take another look and make some test cases when I get a chance.

FinalDoom avatar Dec 23 '23 00:12 FinalDoom

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

trim21 avatar Dec 23 '23 14:12 trim21

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'
]

trim21 avatar Dec 23 '23 14:12 trim21

lgtm, only one minor problem

trim21 avatar Dec 24 '23 09:12 trim21

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.

FinalDoom avatar Dec 24 '23 10:12 FinalDoom

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

trim21 avatar Dec 24 '23 10:12 trim21

handling case-insensitivity may introduce too much extra complexity,which will need to handle every component of path,I think current code works fine

trim21 avatar Dec 24 '23 10:12 trim21

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.

FinalDoom avatar Jan 02 '24 05:01 FinalDoom

please do not force push

trim21 avatar Jan 02 '24 17:01 trim21

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?

FinalDoom avatar Jan 02 '24 18:01 FinalDoom

please do not force push

It's in my repo and rebasing creates better changeset history, why not?

FinalDoom avatar Jan 02 '24 19:01 FinalDoom

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

trim21 avatar Jan 03 '24 10:01 trim21