From 76afa743208ccd09cc749f25f8f37b55018efdba Mon Sep 17 00:00:00 2001 From: Douglas <32344964+NotTheDr01ds@users.noreply.github.com> Date: Tue, 31 Dec 2024 21:05:43 -0500 Subject: [PATCH] `open`: Assign `content_type` metadata for filetypes not handled with a `from` converter (#14670) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description Filetypes which are converted during `open` should not have (and have not had) a `content_type` metadata field. However, filetypes which aren't converted now behave the same as with `--raw` and assign the appropriate `content_type`. ## Before ```nushell open toolkit.nu | metadata # => ╭────────┬────────────────────────────────────────────╮ # => │ source │ /home/ntd/src/ntd-forks/nushell/toolkit.nu │ # => ╰────────┴──────────────────────────────────────────── open --raw toolkit.nu | metadata # => ╭──────────────┬────────────────────────────────────────────╮ # => │ source │ /home/ntd/src/ntd-forks/nushell/toolkit.nu │ # => │ content_type │ application/x-nuscript │ # => ╰──────────────┴────────────────────────────────────────────╯ open script.py | metadata # => ╭────────┬─────────────────────────────╮ # => │ source │ /home/ntd/testing/script.py │ # => ╰────────┴─────────────────────────────╯ open Cargo.toml | metadata # => ╭────────┬────────────────────────────────────────────╮ # => │ source │ /home/ntd/src/ntd-forks/nushell/Cargo.toml │ # => ╰────────┴────────────────────────────────────────────╯ ``` ## After ```nushell # Not converted, so adds content_type open toolkit.nu | metadata # => ╭──────────────┬────────────────────────────────────────────╮ # => │ source │ /home/ntd/src/ntd-forks/nushell/toolkit.nu │ # => │ content_type │ application/x-nuscript │ # => ╰──────────────┴────────────────────────────────────────────╯ # Not converted, so adds content_type open --raw toolkit.nu | metadata # => ╭──────────────┬────────────────────────────────────────────╮ # => │ source │ /home/ntd/src/ntd-forks/nushell/toolkit.nu │ # => │ content_type │ application/x-nuscript │ # => ╰──────────────┴────────────────────────────────────────────╯ # Not converted, so adds content_type open script.py | metadata # => ╭──────────────┬─────────────────────────────╮ # => │ source │ /home/ntd/testing/script.py │ # => │ content_type │ text/plain │ # => ╰──────────────┴───────────────────────────── # Converted, so does not add content_type (no change) open Cargo.toml | metadata # => ╭────────┬────────────────────────────────────────────╮ # => │ source │ /home/ntd/src/ntd-forks/nushell/Cargo.toml │ # => ╰────────┴────────────────────────────────────────────╯ ``` # User-Facing Changes `open ` assigns the appropriate content type when the filetype is not converted via a `from `. # Tests + Formatting - :green_circle: `toolkit fmt` - :green_circle: `toolkit clippy` - :green_circle: `toolkit test` - :green_circle: `toolkit test stdlib` # After Submitting N/A --- crates/nu-command/src/filesystem/open.rs | 29 ++++++++------- crates/nu-command/tests/commands/open.rs | 45 ++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 13 deletions(-) diff --git a/crates/nu-command/src/filesystem/open.rs b/crates/nu-command/src/filesystem/open.rs index d9240cfae5..7cbce3902b 100644 --- a/crates/nu-command/src/filesystem/open.rs +++ b/crates/nu-command/src/filesystem/open.rs @@ -145,22 +145,12 @@ impl Command for Open { } }; - // Assigning content type should only happen in raw mode. Otherwise, the content - // will potentially be in one of the built-in nushell `from xxx` formats and therefore - // cease to be in the original content-type.... or so I'm told. :) - let content_type = if raw { - path.extension() - .map(|ext| ext.to_string_lossy().to_string()) - .and_then(|ref s| detect_content_type(s)) - } else { - None - }; - + // No content_type by default - Is added later if no converter is found let stream = PipelineData::ByteStream( ByteStream::file(file, call_span, engine_state.signals().clone()), Some(PipelineMetadata { data_source: DataSource::FilePath(path.to_path_buf()), - content_type, + content_type: None, }), ); @@ -203,7 +193,20 @@ impl Command for Open { } })?); } - None => output.push(stream), + None => { + // If no converter was found, add content-type metadata + let content_type = path + .extension() + .map(|ext| ext.to_string_lossy().to_string()) + .and_then(|ref s| detect_content_type(s)); + + let stream_with_content_type = + stream.set_metadata(Some(PipelineMetadata { + data_source: DataSource::FilePath(path.to_path_buf()), + content_type, + })); + output.push(stream_with_content_type); + } } } } diff --git a/crates/nu-command/tests/commands/open.rs b/crates/nu-command/tests/commands/open.rs index c3e7db137f..986b621c99 100644 --- a/crates/nu-command/tests/commands/open.rs +++ b/crates/nu-command/tests/commands/open.rs @@ -403,3 +403,48 @@ fn test_content_types_with_open_raw() { assert!(result.out.contains("application/yaml")); }) } + +#[test] +fn test_metadata_without_raw() { + Playground::setup("open_files_content_type_test", |dirs, _| { + let result = nu!(cwd: dirs.formats(), "(open random_numbers.csv | metadata | get content_type?) == null"); + assert_eq!(result.out, "true"); + let result = nu!(cwd: dirs.formats(), "open random_numbers.csv | metadata | get source?"); + assert!(result.out.contains("random_numbers.csv")); + let result = nu!(cwd: dirs.formats(), "(open caco3_plastics.tsv | metadata | get content_type?) == null"); + assert_eq!(result.out, "true"); + let result = nu!(cwd: dirs.formats(), "open caco3_plastics.tsv | metadata | get source?"); + assert!(result.out.contains("caco3_plastics.tsv")); + let result = nu!(cwd: dirs.formats(), "(open sample-simple.json | metadata | get content_type?) == null"); + assert_eq!(result.out, "true"); + let result = nu!(cwd: dirs.formats(), "open sample-simple.json | metadata | get source?"); + assert!(result.out.contains("sample-simple.json")); + // Only when not using nu_plugin_formats + let result = nu!(cwd: dirs.formats(), "open sample.ini | metadata"); + assert!(result.out.contains("text/plain")); + let result = nu!(cwd: dirs.formats(), "open sample.ini | metadata | get source?"); + assert!(result.out.contains("sample.ini")); + let result = nu!(cwd: dirs.formats(), "(open sample_data.xlsx | metadata | get content_type?) == null"); + assert_eq!(result.out, "true"); + let result = nu!(cwd: dirs.formats(), "open sample_data.xlsx | metadata | get source?"); + assert!(result.out.contains("sample_data.xlsx")); + let result = nu!(cwd: dirs.formats(), "open sample_def.nu | metadata | get content_type?"); + assert_eq!(result.out, "application/x-nuscript"); + let result = nu!(cwd: dirs.formats(), "open sample_def.nu | metadata | get source?"); + assert!(result.out.contains("sample_def")); + // Only when not using nu_plugin_formats + let result = nu!(cwd: dirs.formats(), "open sample.eml | metadata | get content_type?"); + assert_eq!(result.out, "message/rfc822"); + let result = nu!(cwd: dirs.formats(), "open sample.eml | metadata | get source?"); + assert!(result.out.contains("sample.eml")); + let result = nu!(cwd: dirs.formats(), "(open cargo_sample.toml | metadata | get content_type?) == null"); + assert_eq!(result.out, "true"); + let result = nu!(cwd: dirs.formats(), "open cargo_sample.toml | metadata | get source?"); + assert!(result.out.contains("cargo_sample.toml")); + let result = + nu!(cwd: dirs.formats(), "(open appveyor.yml | metadata | get content_type?) == null"); + assert_eq!(result.out, "true"); + let result = nu!(cwd: dirs.formats(), "open appveyor.yml | metadata | get source?"); + assert!(result.out.contains("appveyor.yml")); + }) +}