search

flipperzero-rs / flipperzero

Rust on the Flipper Zero
MIT License
518 stars 34 forks source link

Dialogs app file browser wrapper #80

Closed mogenson closed 1 year ago

mogenson commented 1 year ago

Add a safe Rust wrapper for the dialog file browser methods and the DialogsFileBrowserOptions type. Rename the DialogsApp::show() method to show_message() and create new show_file_browser() method. Add a getter method to FuriString to provide access to the inner sys::FuriString via a raw pointer because a sys::FuriString is needed by a few of the Furi C API functions. Extend the storage example to open a file browser and have the user select the file that was just written to the file system.

Note: The Furi C struct is named DialogsFileBrowserOptions, but all other dialog types and functions are singular instead of plural. The Rust type is named DialogFileBrowserOptions.

The DialogFileBrowserOptions type uses setter methods that return Self so they can be chained together in the builder pattern. The DialogFileBrowserOptions includes a Rust unit type phantom data member. The type or size does not really matter, but PhantomData does require a sized type. We just need one struct member to carry the lifetime parameter so that CStr and Icon references are not dropped until the struct goes out of scope.

mogenson commented 1 year ago

@JarvisCraft are you ok with merging this now?

JarvisCraft commented 1 year ago

I actually am not the one with push permissions, so I summon @dcoles and @str4d for this purpose :)

mogenson commented 1 year ago

After inspecting the flipperzero-firmware code, dialog_show_file_browser() will not modify the preselected file path FuriString, but some common storage processing functions will replace /ext, /app, /any prefixes depending on the context. I think pub(crate) fn as_mut_ptr(&mut self) -> *mut sys::FuriString make sense, at least for the show_file_browser() usage. Is everyone in agreement?

mogenson commented 1 year ago

Also, is there anything I need to do with this PR to get the Clippy (MSRV) lints CI to succeed?

mogenson commented 1 year ago

@dcoles @JarvisCraft @str4d Are there any remaining issues or is this PR able to be merged?

dcoles commented 1 year ago

Also, is there anything I need to do with this PR to get the Clippy (MSRV) lints CI to succeed?

@str4d Would you be able to take a look at this? I'm not quite sure why the beta is fine, but MSRV is throwing a Unable to run @augu/clippy-action: HttpError: Resource not accessible by integration.

dcoles commented 1 year ago

@dcoles @JarvisCraft @str4d Are there any remaining issues or is this PR able to be merged?

Yes, let's get this merged. Thanks very much for your patience and for the PR!

mogenson commented 1 year ago

Cheers, thanks!