From 07ab1c5850a71a3dc93da74dd2875284678008fc Mon Sep 17 00:00:00 2001 From: Nimrod Kor Date: Wed, 31 Jul 2024 10:32:49 +0300 Subject: [PATCH] Fix JSON output for created / deleted empty files (#4) * Fix JSON output for created / deleted empty files * Changed vs Unchanged * fmt * Remove println * Fix UT * fmt --- src/display/json.rs | 39 ++++++++++++++++++++++++- src/main.rs | 69 ++++++++++++++++++++++----------------------- src/options.rs | 11 ++++---- src/parse/syntax.rs | 2 ++ tests/cli.rs | 2 +- 5 files changed, 79 insertions(+), 44 deletions(-) diff --git a/src/display/json.rs b/src/display/json.rs index 50e557c5b9..f357b2059f 100644 --- a/src/display/json.rs +++ b/src/display/json.rs @@ -1,6 +1,10 @@ +use lazy_static::lazy_static; use line_numbers::LineNumber; +use regex::Regex; use serde::{ser::SerializeStruct, Serialize, Serializer}; +use crate::display::side_by_side::lines_with_novel; +use crate::parse::syntax::NON_EXISTENT_PERMISSIONS; use crate::{ display::{ context::{all_matched_lines_filled, opposite_positions}, @@ -10,7 +14,11 @@ use crate::{ parse::syntax::{self, MatchedPos, StringKind}, summary::{DiffResult, FileContent, FileFormat}, }; -use crate::display::side_by_side::lines_with_novel; + +lazy_static! { + static ref FILE_PERMS_RE: Regex = + Regex::new("^File permissions changed from (\\d+) to (\\d+).$").unwrap(); +} #[derive(Debug, Serialize, PartialEq)] #[serde(rename_all = "lowercase")] @@ -62,6 +70,23 @@ impl<'f> File<'f> { } } + fn extract_status(extra_info: &'f Option) -> Option<(String, String)> { + let temp = extra_info.clone().unwrap_or_default(); + let line = temp.lines().last(); + match line { + None => None, + Some(s) => { + if let Some(captures) = crate::display::json::FILE_PERMS_RE.captures(s) { + let from = captures[1].to_string(); + let to = captures[2].to_string(); + Some((from, to)) + } else { + None + } + } + } + } + fn with_status( language: &'f FileFormat, path: &'f str, @@ -99,6 +124,18 @@ impl<'f> From<&'f DiffResult> for File<'f> { if hunks.is_empty() { let status = if File::extract_old_path(&summary.extra_info).is_some() { Status::Renamed + } else if let Some((from, to)) = File::extract_status(&summary.extra_info) { + match (from.as_str(), to.as_str()) { + (NON_EXISTENT_PERMISSIONS, _) => Status::Created, + (_, NON_EXISTENT_PERMISSIONS) => Status::Deleted, + _ => { + if from == to { + Status::Unchanged + } else { + Status::Changed + } + } + } } else { Status::Unchanged }; diff --git a/src/main.rs b/src/main.rs index 086d5ed14b..ea899a4bbf 100644 --- a/src/main.rs +++ b/src/main.rs @@ -27,27 +27,21 @@ #![warn(clippy::todo)] #![warn(clippy::dbg_macro)] -mod conflicts; -mod constants; -mod diff; -mod display; -mod exit_codes; -mod files; -mod hash; -mod line_parser; -mod lines; -mod options; -mod parse; -mod summary; -mod version; -mod words; - #[macro_use] extern crate log; +extern crate pretty_env_logger; +use std::path::Path; +use std::{env, thread}; -use display::style::print_warning; +use humansize::{format_size, BINARY}; use log::info; use mimalloc::MiMalloc; +use owo_colors::OwoColorize; +use rayon::prelude::*; +use strum::IntoEnumIterator; +use typed_arena::Arena; + +use display::style::print_warning; use options::FilePermissions; use options::USAGE; @@ -55,6 +49,7 @@ use crate::conflicts::apply_conflict_markers; use crate::conflicts::START_LHS_MARKER; use crate::diff::changes::ChangeMap; use crate::diff::dijkstra::ExceededGraphLimit; +use crate::diff::sliders::fix_all_sliders; use crate::diff::{dijkstra, unchanged}; use crate::display::context::opposite_positions; use crate::display::hunks::{matched_pos_to_hunks, merge_adjacent}; @@ -64,28 +59,10 @@ use crate::files::{ guess_content, read_file_or_die, read_files_or_die, read_or_die, relative_paths_in_either, ProbableFileKind, }; +use crate::options::{DiffOptions, DisplayMode, DisplayOptions, FileArgument, Mode}; use crate::parse::guess_language::language_globs; use crate::parse::guess_language::{guess, language_name, Language, LanguageOverride}; use crate::parse::syntax; - -/// The global allocator used by difftastic. -/// -/// Diffing allocates a large amount of memory, and `MiMalloc` performs -/// better. -#[global_allocator] -static GLOBAL: MiMalloc = MiMalloc; - -use std::path::Path; -use std::{env, thread}; - -use humansize::{format_size, BINARY}; -use owo_colors::OwoColorize; -use rayon::prelude::*; -use strum::IntoEnumIterator; -use typed_arena::Arena; - -use crate::diff::sliders::fix_all_sliders; -use crate::options::{DiffOptions, DisplayMode, DisplayOptions, FileArgument, Mode}; use crate::summary::{DiffResult, FileContent, FileFormat}; use crate::syntax::init_next_prev; use crate::{ @@ -93,7 +70,27 @@ use crate::{ parse::tree_sitter_parser as tsp, }; -extern crate pretty_env_logger; +mod conflicts; +mod constants; +mod diff; +mod display; +mod exit_codes; +mod files; +mod hash; +mod line_parser; +mod lines; +mod options; +mod parse; +mod summary; +mod version; +mod words; + +/// The global allocator used by difftastic. +/// +/// Diffing allocates a large amount of memory, and `MiMalloc` performs +/// better. +#[global_allocator] +static GLOBAL: MiMalloc = MiMalloc; /// Terminate the process if we get SIGPIPE. #[cfg(unix)] diff --git a/src/options.rs b/src/options.rs index 774da35869..18f3baab04 100644 --- a/src/options.rs +++ b/src/options.rs @@ -12,6 +12,7 @@ use const_format::formatcp; use crossterm::tty::IsTty; use itertools::Itertools; +use crate::parse::syntax::NON_EXISTENT_PERMISSIONS; use crate::{ display::style::BackgroundColor, exit_codes::EXIT_BAD_ARGUMENTS, @@ -357,14 +358,12 @@ impl Display for FilePermissions { } } -impl TryFrom<&OsStr> for FilePermissions { - type Error = (); - - fn try_from(s: &OsStr) -> Result { +impl From<&OsStr> for FilePermissions { + fn from(s: &OsStr) -> Self { if s == "." { - Err(()) + Self(NON_EXISTENT_PERMISSIONS.to_owned()) } else { - Ok(Self(s.to_string_lossy().into_owned())) + Self(s.to_string_lossy().into_owned()) } } } diff --git a/src/parse/syntax.rs b/src/parse/syntax.rs index 12e190dced..8f4e3cc925 100644 --- a/src/parse/syntax.rs +++ b/src/parse/syntax.rs @@ -19,6 +19,8 @@ use crate::{ lines::is_all_whitespace, }; +pub const NON_EXISTENT_PERMISSIONS: &'static str = "0000000"; + /// A Debug implementation that does not recurse into the /// corresponding node mentioned for Unchanged. Otherwise we will /// infinitely loop on unchanged nodes, which both point to the other. diff --git a/tests/cli.rs b/tests/cli.rs index fa0ea1df92..b21ef1d347 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -212,7 +212,7 @@ fn git_style_arguments_new_file() { .arg("sample_files/simple_1.txt") .arg("abcdef1234") .arg("100644"); - let predicate_fn = predicate::str::contains("File permissions changed").not(); + let predicate_fn = predicate::str::contains("File permissions changed"); cmd.assert().stdout(predicate_fn); }