diff --git a/.config/jp/tools/src/cargo.rs b/.config/jp/tools/src/cargo.rs index 9b569a8..971a4b1 100644 --- a/.config/jp/tools/src/cargo.rs +++ b/.config/jp/tools/src/cargo.rs @@ -1,4 +1,4 @@ -use crate::{Context, Error, Tool}; +use crate::{Context, Tool, util::ToolResult}; mod check; mod expand; @@ -8,11 +8,15 @@ use check::cargo_check; use expand::cargo_expand; use test::cargo_test; -pub async fn run(ctx: Context, t: Tool) -> std::result::Result { +pub async fn run(ctx: Context, t: Tool) -> ToolResult { match t.name.trim_start_matches("cargo_") { "check" => cargo_check(&ctx, t.opt("package")?).await, - "expand" => cargo_expand(&ctx, t.req("item")?, t.opt("package")?).await, - "test" => cargo_test(&ctx, t.opt("package")?, t.opt("testname")?).await, + "expand" => cargo_expand(&ctx, t.req("item")?, t.opt("package")?) + .await + .map(Into::into), + "test" => cargo_test(&ctx, t.opt("package")?, t.opt("testname")?) + .await + .map(Into::into), _ => Err(format!("Unknown tool '{}'", t.name).into()), } } diff --git a/.config/jp/tools/src/cargo/check.rs b/.config/jp/tools/src/cargo/check.rs index d29266c..44e89ce 100644 --- a/.config/jp/tools/src/cargo/check.rs +++ b/.config/jp/tools/src/cargo/check.rs @@ -1,9 +1,9 @@ use duct::cmd; use jp_tool::Context; -use crate::Result; +use crate::util::{ToolResult, error}; -pub(crate) async fn cargo_check(ctx: &Context, package: Option) -> Result { +pub(crate) async fn cargo_check(ctx: &Context, package: Option) -> ToolResult { let package = package.map_or("--workspace".to_owned(), |v| format!("--package={v}")); let result = cmd!( "cargo", @@ -23,12 +23,11 @@ pub(crate) async fn cargo_check(ctx: &Context, package: Option) -> Resul let code = result.status.code().unwrap_or(0); if code != 0 { - return Err(format!( + return error(format!( "Cargo command failed ({}): {}", result.status.code().unwrap_or(1), String::from_utf8_lossy(&result.stderr) - ) - .into()); + )); } let content = String::from_utf8_lossy(&result.stderr); @@ -38,13 +37,14 @@ pub(crate) async fn cargo_check(ctx: &Context, package: Option) -> Resul let content = content.trim(); if content.is_empty() { - Ok("Check succeeded. No warnings or errors found.".to_owned()) + Ok("Check succeeded. No warnings or errors found.".into()) } else { Ok(indoc::formatdoc! {" ``` {content} ``` - "}) + "} + .into()) } } @@ -79,7 +79,7 @@ mod tests { let result = cargo_check(&ctx, None).await.unwrap(); - assert_eq!(result, indoc::indoc! {r#" + assert_eq!(result.into_content().unwrap(), indoc::indoc! {r#" ``` warning: unused `std::result::Result` that must be used --> src/main.rs:2:5 diff --git a/.config/jp/tools/src/fs/modify_file.rs b/.config/jp/tools/src/fs/modify_file.rs index fd2f9aa..11c102e 100644 --- a/.config/jp/tools/src/fs/modify_file.rs +++ b/.config/jp/tools/src/fs/modify_file.rs @@ -157,30 +157,15 @@ pub(crate) async fn fs_modify_file( let after = if replace_using_regex { contents.replace_using_regexp(&string_to_replace, &new_string)? } else { - let (start_byte, mut end_byte) = contents + let (start_byte, end_byte) = contents .find_pattern_range(&string_to_replace) .ok_or("Cannot find pattern to replace")?; - // Check if pattern is followed by a newline - let followed_by_newline = - end_byte < contents.len() && contents.as_bytes()[end_byte] == b'\n'; - - // If followed by newline, consume it - if followed_by_newline { - end_byte += 1; - } - // Replace the pattern with new string let mut new_content = String::new(); new_content.push_str(&contents[..start_byte]); new_content.push_str(&new_string); - // If we consumed a newline but replacement doesn't end with one, add it - // back - if followed_by_newline && !new_string.ends_with('\n') { - new_content.push('\n'); - } - new_content.push_str(&contents[end_byte..]); new_content }; @@ -519,6 +504,73 @@ mod tests { assert_eq!(&fs::read_to_string(&absolute_file_path).unwrap(), result,); } + #[tokio::test] + #[test_log::test] + async fn test_issue_with_newlines_at_end() { + let string_to_replace = "use crossterm::style::Stylize as _;\nuse inquire::Confirm;\nuse \ + jp_conversation::{Conversation, ConversationId, \ + ConversationStream};\nuse \ + jp_format::conversation::DetailsFmt;\nuse jp_printer::Printable \ + as _;\n\nuse crate::{Output, cmd::Success, ctx::Ctx};\n"; + + let new_string = "use crossterm::style::Stylize as _;\nuse inquire::Confirm;\nuse \ + jp_conversation::{Conversation, ConversationId, \ + ConversationStream};\nuse jp_format::conversation::DetailsFmt;\n\nuse \ + crate::{Output, cmd::Success, ctx::Ctx};\n"; + + let source = indoc! {" + use crossterm::style::Stylize as _; + use inquire::Confirm; + use jp_conversation::{Conversation, ConversationId, ConversationStream}; + use jp_format::conversation::DetailsFmt; + use jp_printer::Printable as _; + + use crate::{Output, cmd::Success, ctx::Ctx}; + + #[derive(Debug, clap::Args)] + pub(crate) struct Rm { + /// Conversation IDs to remove."}; + + let result = indoc! {" + use crossterm::style::Stylize as _; + use inquire::Confirm; + use jp_conversation::{Conversation, ConversationId, ConversationStream}; + use jp_format::conversation::DetailsFmt; + + use crate::{Output, cmd::Success, ctx::Ctx}; + + #[derive(Debug, clap::Args)] + pub(crate) struct Rm { + /// Conversation IDs to remove."}; + + // Create root directory. + let temp_dir = tempdir().unwrap(); + let root = temp_dir.path().to_path_buf(); + + // Create file to be modified. + let file_path = "test.txt"; + let absolute_file_path = root.join(file_path); + fs::write(&absolute_file_path, source).unwrap(); + + let ctx = Context { + root, + format_parameters: false, + }; + + let _actual = fs_modify_file( + ctx, + &Map::new(), + file_path.to_owned(), + string_to_replace.to_owned(), + new_string.to_owned(), + false, + ) + .await + .map_err(|e| e.to_string()); + + assert_eq!(&fs::read_to_string(&absolute_file_path).unwrap(), result); + } + #[tokio::test] #[test_log::test] async fn test_modify_file_replace_with_regexp() { diff --git a/.config/jp/tools/src/fs/read_file.rs b/.config/jp/tools/src/fs/read_file.rs index 7dff57f..dbf16a6 100644 --- a/.config/jp/tools/src/fs/read_file.rs +++ b/.config/jp/tools/src/fs/read_file.rs @@ -21,14 +21,37 @@ pub(crate) async fn fs_read_file( .unwrap_or_default(); let mut contents = std::fs::read_to_string(&absolute_path)?; + let lines = contents.split('\n').count(); + + if start_line.is_some_and(|v| v > end_line.unwrap_or(usize::MAX)) { + return error("`start_line` must be less than or equal to `end_line`."); + } else if start_line.is_some_and(|v| v == 0) { + return error("`start_line` must be greater than 0."); + } else if end_line.is_some_and(|v| v == 0) { + return error("`end_line` must be greater than 0."); + } else if start_line.is_some_and(|v| v > contents.lines().count()) { + return error(format!( + "`start_line` is greater than the number of lines in the file ({lines})." + )); + } if let Some(start_line) = start_line { - contents = contents.split('\n').skip(start_line).collect::(); + contents = contents + .split('\n') + .skip(start_line - 1) + .collect::>() + .join("\n"); + contents.insert_str(0, &format!("... (starting from line #{start_line}) ...\n")); } if let Some(end_line) = end_line { - contents = contents.split('\n').take(end_line).collect::(); + contents = contents + .split('\n') + .take(end_line) + .collect::>() + .join("\n"); + contents.push_str(&format!("\n... (truncated after line #{end_line}) ...")); } @@ -39,3 +62,82 @@ pub(crate) async fn fs_read_file( "} .into()) } + +#[cfg(test)] +mod tests { + use jp_tool::Outcome; + + use super::*; + + #[tokio::test] + async fn test_fs_read_file() { + struct TestCase { + file_contents: String, + start_line: Option, + end_line: Option, + expected: String, + } + + let cases = vec![ + ("all content", TestCase { + file_contents: "foo\nbar\nbaz\n".to_owned(), + start_line: None, + end_line: None, + expected: "```txt\nfoo\nbar\nbaz\n\n```\n".to_owned(), + }), + ("start line", TestCase { + file_contents: "foo\nbar\nbaz\n".to_owned(), + start_line: Some(2), + end_line: None, + expected: "```txt\n... (starting from line #2) ...\nbar\nbaz\n\n```\n".to_owned(), + }), + ("end line", TestCase { + file_contents: "foo\nbar\nbaz\n".to_owned(), + start_line: None, + end_line: Some(2), + expected: "```txt\nfoo\nbar\n... (truncated after line #2) ...\n```\n".to_owned(), + }), + ("start and end line", TestCase { + file_contents: "foo\nbar\nbaz\n\n".to_owned(), + start_line: Some(2), + end_line: Some(2), + expected: "```txt\n... (starting from line #2) ...\nbar\n... (truncated after \ + line #2) ...\n```\n" + .to_owned(), + }), + ]; + + for ( + name, + TestCase { + file_contents, + start_line, + end_line, + expected, + }, + ) in cases + { + let tmp = tempfile::tempdir().unwrap(); + let path = tmp.path().join("file.txt"); + + std::fs::write(&path, file_contents).unwrap(); + + let result = fs_read_file( + tmp.path().to_owned(), + "file.txt".to_owned(), + start_line, + end_line, + ) + .await + .unwrap(); + + let out = match result { + Outcome::Success { content } => content, + Outcome::Error { message, .. } => message, + Outcome::NeedsInput { .. } => String::new(), + }; + + assert_eq!(out, expected, "failed test case '{name}'"); + } + } +} diff --git a/.config/jp/tools/src/fs/snapshots/modify_file_replace_word@replace_first_line_with_multiple_lines.snap b/.config/jp/tools/src/fs/snapshots/modify_file_replace_word@replace_first_line_with_multiple_lines.snap index 49ce46b..a2e8802 100644 --- a/.config/jp/tools/src/fs/snapshots/modify_file_replace_word@replace_first_line_with_multiple_lines.snap +++ b/.config/jp/tools/src/fs/snapshots/modify_file_replace_word@replace_first_line_with_multiple_lines.snap @@ -6,8 +6,9 @@ File modified successfully: ```diff --- test.txt +++ test.txt -@@ -1 +1,2 @@ +@@ -1 +1,3 @@ -hello world +hello +world ++ ``` diff --git a/.config/jp/tools/src/lib.rs b/.config/jp/tools/src/lib.rs index cfa4e6a..a884268 100644 --- a/.config/jp/tools/src/lib.rs +++ b/.config/jp/tools/src/lib.rs @@ -15,7 +15,7 @@ type Result = std::result::Result; pub async fn run(ctx: Context, t: Tool) -> Result { match t.name.as_str() { - s if s.starts_with("cargo_") => cargo::run(ctx, t).await.map(Into::into), + s if s.starts_with("cargo_") => cargo::run(ctx, t).await, s if s.starts_with("github_") => github::run(ctx, t).await.map(Into::into), s if s.starts_with("fs_") => fs::run(ctx, t).await, s if s.starts_with("web_") => web::run(ctx, t).await.map(Into::into), diff --git a/deny.toml b/deny.toml index 6aeb249..928b9ea 100644 --- a/deny.toml +++ b/deny.toml @@ -1,6 +1,7 @@ [advisories] ignore = [ { id = "RUSTSEC-2024-0436", reason = "paste is unmaintained, but stable" }, + { id = "RUSTSEC-2025-0141", reason = "The team considers version 1.3.3 a complete version of bincode that is not in need of any updates." }, ] [licenses]