review and stuff

Author: teh-cmc

Cargo.lock

@@ -60,9 +60,9 @@ pub fn iter_external_loaders() -> impl ExactSizeIterator<Item = std::path::PathB
 /// The external loaders are expected to log rrd data to their standard output.
 ///
 /// Refer to our `external_data_loader` example for more information.
-pub struct ExternalDataLoader;
+pub struct ExternalLoader;
 
-impl crate::DataLoader for ExternalDataLoader {
+impl crate::DataLoader for ExternalLoader {
     #[inline]
     fn name(&self) -> String {
         "rerun.data_loaders.External".into()

crates/re_data_source/src/data_loader/loader_external.rs

@@ -22,13 +22,14 @@ use re_log_types::{ArrowMsg, DataRow, LogMsg};
 /// [There are plans to make this generic over any URI](https://github.com/rerun-io/rerun/issues/4525).
 ///
 /// Rerun comes with a few [`DataLoader`]s by default:
-/// - [`RrdLoader`] for [Rerun files]
+/// - [`RrdLoader`] for [Rerun files].
 /// - [`ArchetypeLoader`] for:
 ///     - [3D models]
 ///     - [Images]
 ///     - [Point clouds]
 ///     - [Text files]
-/// - [`DirectoryLoader`] for recursively loading folders
+/// - [`DirectoryLoader`] for recursively loading folders.
+/// - `ExternalLoader`, which looks for user-defined data loaders in $PATH.
 ///
 /// ## Execution
 ///
@@ -210,7 +211,7 @@ static BUILTIN_LOADERS: Lazy<Vec<Arc<dyn DataLoader>>> = Lazy::new(|| {
         Arc::new(ArchetypeLoader),
         Arc::new(DirectoryLoader),
         #[cfg(not(target_arch = "wasm32"))]
-        Arc::new(ExternalDataLoader),
+        Arc::new(ExternalLoader),
     ]
 });
 
@@ -237,5 +238,5 @@ pub use self::loader_rrd::RrdLoader;
 pub(crate) use self::loader_external::EXTERNAL_LOADER_PATHS;
 #[cfg(not(target_arch = "wasm32"))]
 pub use self::loader_external::{
-    iter_external_loaders, ExternalDataLoader, EXTERNAL_DATA_LOADER_PREFIX,
+    iter_external_loaders, ExternalLoader, EXTERNAL_DATA_LOADER_PREFIX,
 };

crates/re_data_source/src/data_loader/mod.rs

@@ -146,16 +146,16 @@ pub(crate) fn load(
     contents: Option<std::borrow::Cow<'_, [u8]>>,
 ) -> Result<std::sync::mpsc::Receiver<LoadedData>, DataLoaderError> {
     #[cfg(target_arch = "wasm32")]
-    let no_external_loaders = true;
+    let has_external_loaders = false;
     #[cfg(not(target_arch = "wasm32"))]
-    let no_external_loaders = crate::data_loader::EXTERNAL_LOADER_PATHS.is_empty();
+    let has_external_loaders = !crate::data_loader::EXTERNAL_LOADER_PATHS.is_empty();
 
     let extension = extension(path);
     let is_builtin = is_associated_with_builtin_loader(path, is_dir);
 
     // If there are no external loaders registered (which is always the case on wasm) and we don't
     // have a builtin loader for it, then we know for a fact that we won't be able to load it.
-    if !is_builtin && no_external_loaders {
+    if !is_builtin && !has_external_loaders {
         return if extension.is_empty() {
             Err(anyhow::anyhow!("files without extensions (file.XXX) are not supported").into())
         } else {

crates/re_data_source/src/load_file.rs

@@ -9,4 +9,4 @@ publish = false
 [dependencies]
 rerun = { path = "../../../crates/rerun" }
 
-pico-args = { version = "0.5.0", features = ["eq-separator"] }
+argh = "0.1"

examples/rust/external_data_loader/Cargo.toml

@@ -1,31 +1,6 @@
 //! Example of an external data-loader executable plugin for the Rerun Viewer.
 
-use rerun::MediaType;
-
-const USAGE: &str = "
-This is an example executable data-loader plugin for the Rerun Viewer.
-
-It will log Rust source code files as markdown documents.
-To try it out, install it in your $PATH (`cargo install --path . -f`), then open a Rust source file with Rerun (`rerun file.rs`).
-
-USAGE:
-  rerun-loader-rust-file [OPTIONS] FILEPATH
-
-FLAGS:
-  -h, --help                    Prints help information
-
-OPTIONS:
-  --recording-id RECORDING_ID   ID of the shared recording
-
-ARGS:
-  <FILEPATH>
-";
-
-#[allow(clippy::exit)]
-fn usage() -> ! {
-    eprintln!("{USAGE}");
-    std::process::exit(1);
-}
+use rerun::{external::re_data_source::extension, MediaType};
 
 // The Rerun Viewer will always pass these two pieces of information:
 // 1. The path to be loaded, as a positional arg.
@@ -34,33 +9,27 @@ fn usage() -> ! {
 // It is up to you whether you make use of that shared recording ID or not.
 // If you use it, the data will end up in the same recording as all other plugins interested in
 // that file, otherwise you can just create a dedicated recording for it. Or both.
+
+/// This is an example executable data-loader plugin for the Rerun Viewer.
+///
+/// It will log Rust source code files as markdown documents.
+/// To try it out, install it in your $PATH (`cargo install --path . -f`), then open
+/// Rust source file with Rerun (`rerun file.rs`).
+#[derive(argh::FromArgs)]
 struct Args {
+    #[argh(positional)]
     filepath: std::path::PathBuf,
-    recording_id: Option<String>,
-}
 
-impl Args {
-    fn from_env() -> Result<Self, pico_args::Error> {
-        let mut pargs = pico_args::Arguments::from_env();
-        Ok(Self {
-            filepath: pargs.free_from_str()?,
-            recording_id: pargs.opt_value_from_str("--recording-id")?,
-        })
-    }
+    /// optional ID of the shared recording
+    #[argh(option)]
+    recording_id: Option<String>,
 }
 
 fn main() -> Result<(), Box<dyn std::error::Error>> {
-    let Ok(args) = Args::from_env() else {
-        usage();
-    };
+    let args: Args = argh::from_env();
 
     let is_file = args.filepath.is_file();
-    let is_rust_file = args
-        .filepath
-        .extension()
-        .unwrap_or_default()
-        .to_ascii_lowercase()
-        == "rs";
+    let is_rust_file = extension(&args.filepath) == "rs";
 
     // We're not interested: just exit silently.
     // Don't return an error, as that would show up to the end user in the Rerun Viewer!