From 2b7e145e56c7f1fbb4498d4ec6c6f9d237405db7 Mon Sep 17 00:00:00 2001 From: scarf Date: Tue, 28 Nov 2023 00:32:12 +0900 Subject: [PATCH] feat(fmt): support formatting code blocks in Jupyter notebooks (#21310) --- .dprint.json | 1 + Cargo.lock | 14 +++ cli/Cargo.toml | 1 + cli/args/flags.rs | 4 +- cli/build.rs | 6 +- cli/tests/integration/fmt_tests.rs | 23 +++- cli/tests/testdata/fmt/badly_formatted.ipynb | 105 +++++++++++++++++ .../testdata/fmt/badly_formatted_fixed.ipynb | 106 ++++++++++++++++++ .../expected_fmt_check_verbose_tests_dir.out | 2 - cli/tools/fmt.rs | 36 +++--- 10 files changed, 272 insertions(+), 26 deletions(-) create mode 100644 cli/tests/testdata/fmt/badly_formatted.ipynb create mode 100644 cli/tests/testdata/fmt/badly_formatted_fixed.ipynb delete mode 100644 cli/tests/testdata/fmt/expected_fmt_check_verbose_tests_dir.out diff --git a/.dprint.json b/.dprint.json index 3d2f63eb88..13924beafd 100644 --- a/.dprint.json +++ b/.dprint.json @@ -30,6 +30,7 @@ "cli/tests/testdata/file_extensions/ts_with_js_extension.js", "cli/tests/testdata/fmt/badly_formatted.json", "cli/tests/testdata/fmt/badly_formatted.md", + "cli/tests/testdata/fmt/badly_formatted.ipynb", "cli/tests/testdata/byte_order_mark.ts", "cli/tests/testdata/encoding", "cli/tests/testdata/fmt/", diff --git a/Cargo.lock b/Cargo.lock index c7f15fd93d..1a9f189710 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -884,6 +884,7 @@ dependencies = [ "dissimilar", "dotenvy", "dprint-plugin-json", + "dprint-plugin-jupyter", "dprint-plugin-markdown", "dprint-plugin-typescript", "encoding_rs", @@ -1940,6 +1941,19 @@ dependencies = [ "text_lines", ] +[[package]] +name = "dprint-plugin-jupyter" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b65cf5dd43b15bf9dbb69a2d0e331106d6e3d33d8678d32385245993e8855c02" +dependencies = [ + "anyhow", + "dprint-core", + "jsonc-parser", + "serde", + "serde_json", +] + [[package]] name = "dprint-plugin-markdown" version = "0.16.3" diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 259a8447e8..e56f53a02d 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -84,6 +84,7 @@ dotenvy = "0.15.7" dprint-plugin-json = "=0.19.1" dprint-plugin-markdown = "=0.16.3" dprint-plugin-typescript = "=0.88.5" +dprint-plugin-jupyter = "=0.1.1" encoding_rs.workspace = true env_logger = "=0.10.0" fancy-regex = "=0.10.0" diff --git a/cli/args/flags.rs b/cli/args/flags.rs index d7d51b3741..555f2f312a 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -1579,7 +1579,9 @@ Ignore formatting a file by adding an ignore comment at the top of the file: .help("Set content type of the supplied file") // prefer using ts for formatting instead of js because ts works in more scenarios .default_value("ts") - .value_parser(["ts", "tsx", "js", "jsx", "md", "json", "jsonc"]), + .value_parser([ + "ts", "tsx", "js", "jsx", "md", "json", "jsonc", "ipynb", + ]), ) .arg( Arg::new("ignore") diff --git a/cli/build.rs b/cli/build.rs index 01505653a1..4a481885e6 100644 --- a/cli/build.rs +++ b/cli/build.rs @@ -359,11 +359,7 @@ fn create_cli_snapshot(snapshot_path: PathBuf) -> CreateSnapshotOutput { // Ideally we could deduplicate that code. fn deno_version() -> String { if env::var("DENO_CANARY").is_ok() { - format!( - "{}+{}", - env!("CARGO_PKG_VERSION"), - git_commit_hash()[..7].to_string() - ) + format!("{}+{}", env!("CARGO_PKG_VERSION"), &git_commit_hash()[..7]) } else { env!("CARGO_PKG_VERSION").to_string() } diff --git a/cli/tests/integration/fmt_tests.rs b/cli/tests/integration/fmt_tests.rs index b8c77c529e..5d555abf53 100644 --- a/cli/tests/integration/fmt_tests.rs +++ b/cli/tests/integration/fmt_tests.rs @@ -27,18 +27,24 @@ fn fmt_test() { testdata_fmt_dir.join("badly_formatted.json"); let badly_formatted_json = t.path().join("badly_formatted.json"); badly_formatted_original_json.copy(&badly_formatted_json); - // First, check formatting by ignoring the badly formatted file. + let fixed_ipynb = testdata_fmt_dir.join("badly_formatted_fixed.ipynb"); + let badly_formatted_original_ipynb = + testdata_fmt_dir.join("badly_formatted.ipynb"); + let badly_formatted_ipynb = t.path().join("badly_formatted.ipynb"); + badly_formatted_original_ipynb.copy(&badly_formatted_ipynb); + + // First, check formatting by ignoring the badly formatted file. let output = context .new_command() .current_dir(&testdata_fmt_dir) .args_vec(vec![ "fmt".to_string(), format!( - "--ignore={badly_formatted_js},{badly_formatted_md},{badly_formatted_json}", + "--ignore={badly_formatted_js},{badly_formatted_md},{badly_formatted_json},{badly_formatted_ipynb}", ), format!( - "--check {badly_formatted_js} {badly_formatted_md} {badly_formatted_json}", + "--check {badly_formatted_js} {badly_formatted_md} {badly_formatted_json} {badly_formatted_ipynb}", ), ]) .run(); @@ -57,6 +63,7 @@ fn fmt_test() { badly_formatted_js.to_string(), badly_formatted_md.to_string(), badly_formatted_json.to_string(), + badly_formatted_ipynb.to_string(), ]) .run(); @@ -72,6 +79,7 @@ fn fmt_test() { badly_formatted_js.to_string(), badly_formatted_md.to_string(), badly_formatted_json.to_string(), + badly_formatted_ipynb.to_string(), ]) .run(); @@ -81,12 +89,15 @@ fn fmt_test() { let expected_js = fixed_js.read_to_string(); let expected_md = fixed_md.read_to_string(); let expected_json = fixed_json.read_to_string(); + let expected_ipynb = fixed_ipynb.read_to_string(); let actual_js = badly_formatted_js.read_to_string(); let actual_md = badly_formatted_md.read_to_string(); let actual_json = badly_formatted_json.read_to_string(); + let actual_ipynb = badly_formatted_ipynb.read_to_string(); assert_eq!(expected_js, actual_js); assert_eq!(expected_md, actual_md); assert_eq!(expected_json, actual_json); + assert_eq!(expected_ipynb, actual_ipynb); } #[test] @@ -198,6 +209,12 @@ itest!(fmt_stdin_json { output_str: Some("{ \"key\": \"value\" }\n"), }); +itest!(fmt_stdin_ipynb { + args: "fmt --ext=ipynb -", + input: Some(include_str!("../testdata/fmt/badly_formatted.ipynb")), + output_str: Some(include_str!("../testdata/fmt/badly_formatted_fixed.ipynb")), +}); + itest!(fmt_stdin_check_formatted { args: "fmt --check -", input: Some("const a = 1;\n"), diff --git a/cli/tests/testdata/fmt/badly_formatted.ipynb b/cli/tests/testdata/fmt/badly_formatted.ipynb new file mode 100644 index 0000000000..c8600564fb --- /dev/null +++ b/cli/tests/testdata/fmt/badly_formatted.ipynb @@ -0,0 +1,105 @@ +{ + "cells": [ + { + "cell_type": "markdown", + "metadata": {}, + "source": [ + "# Hello Markdown\n", + "this isn't formatted properly" + ] + }, + { + "cell_type": "code", + "execution_count": 5, + "metadata": {}, + "outputs": [ + { + "name": "stdout", + "output_type": "stream", + "text": [ + "Hello World\n" + ] + } + ], + "source": [ + "console.log(\"Hello World\"\n", + "\n", + ");" + ] + }, + { + "cell_type": "raw", + "metadata": {}, + "source": [ + "raw text\n", + " here too\n" + ] + }, + { + "cell_type": "code", + "execution_count": 6, + "metadata": {}, + "outputs": [ + { + "name": "stdout", + "output_type": "stream", + "text": [ + "alice\n" + ] + } + ], + "source": [ + "function hello(name: string ) {\n", + " console.log(name);\n", + "};\n", + "\n", + "hello( \"alice\");\n" + ] + }, + { + "cell_type": "code", + "execution_count": 7, + "metadata": {}, + "outputs": [], + "source": [ + "function foo(): number {\n", + " return 2;\n", + "}\n" + ] + }, + { + "cell_type": "code", + "execution_count": 8, + "metadata": {}, + "outputs": [ + { + "name": "stdout", + "output_type": "stream", + "text": [ + "loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong\n" + ] + } + ], + "source": [ + "console.log(\"loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong\");" + ] + } + ], + "metadata": { + "kernelspec": { + "display_name": "Deno", + "language": "typescript", + "name": "deno" + }, + "language_info": { + "file_extension": ".ts", + "mimetype": "text/x.typescript", + "name": "typescript", + "nb_converter": "script", + "pygments_lexer": "typescript", + "version": "5.2.2" + } + }, + "nbformat": 4, + "nbformat_minor": 2 +} diff --git a/cli/tests/testdata/fmt/badly_formatted_fixed.ipynb b/cli/tests/testdata/fmt/badly_formatted_fixed.ipynb new file mode 100644 index 0000000000..a26a95e24e --- /dev/null +++ b/cli/tests/testdata/fmt/badly_formatted_fixed.ipynb @@ -0,0 +1,106 @@ +{ + "cells": [ + { + "cell_type": "markdown", + "metadata": {}, + "source": [ + "# Hello Markdown\n", + "\n", + "this isn't formatted properly" + ] + }, + { + "cell_type": "code", + "execution_count": 5, + "metadata": {}, + "outputs": [ + { + "name": "stdout", + "output_type": "stream", + "text": [ + "Hello World\n" + ] + } + ], + "source": [ + "console.log(\"Hello World\");" + ] + }, + { + "cell_type": "raw", + "metadata": {}, + "source": [ + "raw text\n", + " here too\n" + ] + }, + { + "cell_type": "code", + "execution_count": 6, + "metadata": {}, + "outputs": [ + { + "name": "stdout", + "output_type": "stream", + "text": [ + "alice\n" + ] + } + ], + "source": [ + "function hello(name: string) {\n", + " console.log(name);\n", + "}\n", + "\n", + "hello(\"alice\");" + ] + }, + { + "cell_type": "code", + "execution_count": 7, + "metadata": {}, + "outputs": [], + "source": [ + "function foo(): number {\n", + " return 2;\n", + "}" + ] + }, + { + "cell_type": "code", + "execution_count": 8, + "metadata": {}, + "outputs": [ + { + "name": "stdout", + "output_type": "stream", + "text": [ + "loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong\n" + ] + } + ], + "source": [ + "console.log(\n", + " \"loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong\",\n", + ");" + ] + } + ], + "metadata": { + "kernelspec": { + "display_name": "Deno", + "language": "typescript", + "name": "deno" + }, + "language_info": { + "file_extension": ".ts", + "mimetype": "text/x.typescript", + "name": "typescript", + "nb_converter": "script", + "pygments_lexer": "typescript", + "version": "5.2.2" + } + }, + "nbformat": 4, + "nbformat_minor": 2 +} diff --git a/cli/tests/testdata/fmt/expected_fmt_check_verbose_tests_dir.out b/cli/tests/testdata/fmt/expected_fmt_check_verbose_tests_dir.out deleted file mode 100644 index 04cd5ec642..0000000000 --- a/cli/tests/testdata/fmt/expected_fmt_check_verbose_tests_dir.out +++ /dev/null @@ -1,2 +0,0 @@ -[WILDCARD] -error: Found 1 not formatted file in [WILDCARD] files diff --git a/cli/tools/fmt.rs b/cli/tools/fmt.rs index 5c47b5497f..36828e23ce 100644 --- a/cli/tools/fmt.rs +++ b/cli/tools/fmt.rs @@ -223,23 +223,29 @@ pub fn format_json( dprint_plugin_json::format_text(file_path, file_text, &config) } -/// Formats a single TS, TSX, JS, JSX, JSONC, JSON, or MD file. +/// Formats a single TS, TSX, JS, JSX, JSONC, JSON, MD, or IPYNB file. pub fn format_file( file_path: &Path, file_text: &str, fmt_options: &FmtOptionsConfig, ) -> Result, AnyError> { let ext = get_extension(file_path).unwrap_or_default(); - if matches!( - ext.as_str(), - "md" | "mkd" | "mkdn" | "mdwn" | "mdown" | "markdown" - ) { - format_markdown(file_text, fmt_options) - } else if matches!(ext.as_str(), "json" | "jsonc") { - format_json(file_path, file_text, fmt_options) - } else { - let config = get_resolved_typescript_config(fmt_options); - dprint_plugin_typescript::format_text(file_path, file_text, &config) + + match ext.as_str() { + "md" | "mkd" | "mkdn" | "mdwn" | "mdown" | "markdown" => { + format_markdown(file_text, fmt_options) + } + "json" | "jsonc" => format_json(file_path, file_text, fmt_options), + "ipynb" => dprint_plugin_jupyter::format_text( + file_text, + |file_path: &Path, file_text: String| { + format_file(file_path, &file_text, fmt_options) + }, + ), + _ => { + let config = get_resolved_typescript_config(fmt_options); + dprint_plugin_typescript::format_text(file_path, file_text, &config) + } } } @@ -667,7 +673,7 @@ where /// This function is similar to is_supported_ext but adds additional extensions /// supported by `deno fmt`. fn is_supported_ext_fmt(path: &Path) -> bool { - if let Some(ext) = get_extension(path) { + get_extension(path).is_some_and(|ext| { matches!( ext.as_str(), "ts" @@ -686,10 +692,9 @@ fn is_supported_ext_fmt(path: &Path) -> bool { | "mdwn" | "mdown" | "markdown" + | "ipynb" ) - } else { - false - } + }) } #[cfg(test)] @@ -721,6 +726,7 @@ mod test { assert!(is_supported_ext_fmt(Path::new("foo.JSONC"))); assert!(is_supported_ext_fmt(Path::new("foo.json"))); assert!(is_supported_ext_fmt(Path::new("foo.JsON"))); + assert!(is_supported_ext_fmt(Path::new("foo.ipynb"))); } #[test]