spacedriveapp / spacedrive

Spacedrive is an open source cross-platform file explorer, powered by a virtual distributed filesystem written in Rust.
https://spacedrive.com
GNU Affero General Public License v3.0
30.56k stars 896 forks source link

"Open With" list order is random #1560

Open dylanbr0wn opened 11 months ago

dylanbr0wn commented 11 months ago

Describe the bug

Opening context menu of a file and opening the Open With submenu produces an unordered list of results that changes each time it is opened. There is also a significant delay from when list is open to when it is populated or updated.

https://github.com/spacedriveapp/spacedrive/assets/40218657/8eda3954-98d9-4201-8913-0226c77b9abd

Reproduction

  1. Open Spacedrive and navigate to a file
  2. Open Context menu of the file
  3. Hover over the Open With option

Expected behavior

I would expect this list to be ordered alphabetically, perhaps with my default application for this specific file type as the first option. Would also expect this list to be populated when dropdown is opened and not after.

Platform and versions

pnpm --version && cargo --version && rustc --version:
8.9.0
cargo 1.73.0 (9c4383fb5 2023-08-26)
rustc 1.73.0 (cc66ad468 2023-10-03)

OS:
ProductName:        macOS
ProductVersion:     14.0
BuildVersion:       23A344

Stack trace

No response

Additional context

No response

killua1zoldyck commented 10 months ago

If no one is working on this, can I work on this?

HeavenVolkoff commented 10 months ago

Sure, go ahead @killua1zoldyck, I'll assign this issue to you. I agree with what you wrote in the expected behavior, only point I would add is that if there is an API that returns a list of apps already sorted by preference/most used to prefer that over alphabetic order (If I remember correctly this worked correctly at some point in time, some bug must have cropped up somewhat recently)

Below is where most of the logic for this feature is located. Native, backend and front-end:

Linux: https://github.com/spacedriveapp/spacedrive/blob/main/apps/desktop/crates/linux/src/app_info.rs#L75-L122

macOS: https://github.com/spacedriveapp/spacedrive/blob/main/apps/desktop/crates/macos/src-swift/files.swift#L30-L105

Windows: https://github.com/spacedriveapp/spacedrive/blob/main/apps/desktop/crates/windows/src/lib.rs#L55-L94

Backend: https://github.com/spacedriveapp/spacedrive/blob/main/apps/desktop/src-tauri/src/file.rs#L110-L256

Front-end: https://github.com/spacedriveapp/spacedrive/blob/main/interface/app/%24libraryId/Explorer/ContextMenu/OpenWith.tsx

MadisonKonig commented 5 days ago

Is it fine to just have this be fixed on the front-end? It's just a single line of code to sort the list that is shown.

...if there is an API that returns a list of apps already sorted by preference/most used to prefer that over alphabetic order...'

More wondering what's wanting to be done with this issue, as it's been nearly a year. (I know it's not top priority, but I'm knocking off bugs slowly)

iLynxcat commented 5 days ago

Hey Madison, thanks for looking into this one — I think sorting in Rust is always more ideal for performance impact, but at the same time the 'open with' list will always be such a short list that I think the impact here will be negligible if done in JS and will be completely fine

MadisonKonig commented 5 days ago

I have it figured out on the front end side of things. (tested for windows and macos), but can figure out a fix for rust too before making a PR. Thanks for the speedy reply :)

MadisonKonig commented 21 hours ago

I managed to figure it out in Rust, but it's hella ugly, and I'm still navigating through the learning process of Rust. Something about needing to make a copy and not allowing multiple values in the two different places, and single ownership? idk

image

I tried putting the sort_by() at the end of .unwrap_or(), but it would give the error that it's expecting Result<Vec<OpenWithApplication> and getting () instead.

Is there a better way to do this without making a mutable clone of the Vec in order to sort?

Edit: less variables used, but can't chain .sort_by() with the others, so not as pretty, but not as ugly too?