From 903f82a8a3a7dcdd210b0180ab8b612a127f8c37 Mon Sep 17 00:00:00 2001 From: pm100 Date: Tue, 13 Oct 2020 11:44:38 -0700 Subject: [PATCH 1/9] test --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 12600a80e5..fda3763ad5 100644 --- a/README.md +++ b/README.md @@ -35,7 +35,7 @@ - Scalable terminal UI layout - Async [input polling](assets/perf_compare.jpg) - Async git API for fluid control - +a # Benchmarks For a [RustBerlin meetup presentation](https://youtu.be/rpilJV-eIVw?t=5334) ([slides](https://github.com/extrawurst/gitui-presentation)) I compared `lazygit`,`tig` and `gitui` by parsing the entire Linux git repository (which contains over 900k commits): From 7fa35c851f08a2dd93a4ef7d487172f62e14e284 Mon Sep 17 00:00:00 2001 From: pm100 Date: Tue, 27 Oct 2020 15:54:25 -0700 Subject: [PATCH 2/9] test --- asyncgit/src/sync/hooks.rs | 16 ++++++++++++++++ asyncgit/src/sync/mod.rs | 2 +- src/components/commit.rs | 10 ++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/asyncgit/src/sync/hooks.rs b/asyncgit/src/sync/hooks.rs index 50f2ed6ee1..2d094a7bac 100644 --- a/asyncgit/src/sync/hooks.rs +++ b/asyncgit/src/sync/hooks.rs @@ -9,6 +9,7 @@ use std::{ }; const HOOK_POST_COMMIT: &str = ".git/hooks/post-commit"; +const HOOK_PRE_COMMIT: &str = ".git/hooks/pre-commit"; const HOOK_COMMIT_MSG: &str = ".git/hooks/commit-msg"; const HOOK_COMMIT_MSG_TEMP_FILE: &str = ".git/COMMIT_EDITMSG"; @@ -45,6 +46,21 @@ pub fn hooks_commit_msg( } } +/// this hook is documented here https://git-scm.com/docs/githooks#_pre_commit +/// +pub fn hooks_pre_commit(repo_path: &str) -> Result { + scope_time!("hooks_pre_commit"); + + let work_dir = work_dir_as_string(repo_path)?; + + if hook_runable(work_dir.as_str(), HOOK_PRE_COMMIT) { + let res = run_hook(work_dir.as_str(), HOOK_PRE_COMMIT, &[]); + + Ok(res) + } else { + Ok(HookResult::Ok) + } +} /// pub fn hooks_post_commit(repo_path: &str) -> Result { scope_time!("hooks_post_commit"); diff --git a/asyncgit/src/sync/mod.rs b/asyncgit/src/sync/mod.rs index 0425381fef..ca6216ca2d 100644 --- a/asyncgit/src/sync/mod.rs +++ b/asyncgit/src/sync/mod.rs @@ -31,7 +31,7 @@ pub use commit_details::{ pub use commit_files::get_commit_files; pub use commits_info::{get_commits_info, CommitId, CommitInfo}; pub use diff::get_diff_commit; -pub use hooks::{hooks_commit_msg, hooks_post_commit, HookResult}; +pub use hooks::{hooks_commit_msg, hooks_post_commit, hooks_pre_commit, HookResult}; pub use hunks::{reset_hunk, stage_hunk, unstage_hunk}; pub use ignore::add_to_ignore; pub use logwalker::LogWalker; diff --git a/src/components/commit.rs b/src/components/commit.rs index 650f28d484..99a01189fc 100644 --- a/src/components/commit.rs +++ b/src/components/commit.rs @@ -194,6 +194,16 @@ impl CommitComponent { } fn commit_msg(&mut self, msg: String) -> Result<()> { + if let HookResult::NotOk(e) = sync::hooks_pre_commit(CWD)? { + log::error!("pre-commit hook error: {}", e); + self.queue.borrow_mut().push_back( + InternalEvent::ShowErrorMsg(format!( + "commit-msg hook error:\n{}", + e + )), + ); + return Ok(()); + } let mut msg = msg; if let HookResult::NotOk(e) = sync::hooks_commit_msg(CWD, &mut msg)? From ca28939b465d6150f9874a802cd48a0f67bbe399 Mon Sep 17 00:00:00 2001 From: pm100 Date: Tue, 27 Oct 2020 17:27:10 -0700 Subject: [PATCH 3/9] test new run --- asyncgit/src/sync/hooks.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/asyncgit/src/sync/hooks.rs b/asyncgit/src/sync/hooks.rs index 2d094a7bac..405b482554 100644 --- a/asyncgit/src/sync/hooks.rs +++ b/asyncgit/src/sync/hooks.rs @@ -110,7 +110,11 @@ fn run_hook( hook_script: &str, args: &[&str], ) -> HookResult { - let mut bash_args = vec![hook_script.to_string()]; + + let path = Path::new(path); + let script = path.join(hook_script); + let mut bash_args = vec![]; +// let mut bash_args = vec![hook_script.to_string()]; bash_args.extend_from_slice( &args .iter() @@ -118,7 +122,7 @@ fn run_hook( .collect::>(), ); - let output = Command::new("bash") + let output = Command::new(script) .args(bash_args) .current_dir(path) // This call forces Command to handle the Path environment correctly on windows, From f843ecf0dd7eb3b725aa6c8b76d1685fb6f8105c Mon Sep 17 00:00:00 2001 From: pm100 Date: Wed, 28 Oct 2020 16:44:26 -0700 Subject: [PATCH 4/9] fix hooks for linux --- asyncgit/src/sync/hooks.rs | 9 +++------ asyncgit/src/sync/mod.rs | 4 +++- src/components/msg.rs | 28 ++++++++++++++++++---------- 3 files changed, 24 insertions(+), 17 deletions(-) diff --git a/asyncgit/src/sync/hooks.rs b/asyncgit/src/sync/hooks.rs index 405b482554..8c3a89dbac 100644 --- a/asyncgit/src/sync/hooks.rs +++ b/asyncgit/src/sync/hooks.rs @@ -110,11 +110,8 @@ fn run_hook( hook_script: &str, args: &[&str], ) -> HookResult { - - let path = Path::new(path); - let script = path.join(hook_script); - let mut bash_args = vec![]; -// let mut bash_args = vec![hook_script.to_string()]; + let mut bash_args = + vec!["-c".to_string(), hook_script.to_string()]; bash_args.extend_from_slice( &args .iter() @@ -122,7 +119,7 @@ fn run_hook( .collect::>(), ); - let output = Command::new(script) + let output = Command::new("bash") .args(bash_args) .current_dir(path) // This call forces Command to handle the Path environment correctly on windows, diff --git a/asyncgit/src/sync/mod.rs b/asyncgit/src/sync/mod.rs index ca6216ca2d..f9baea63f1 100644 --- a/asyncgit/src/sync/mod.rs +++ b/asyncgit/src/sync/mod.rs @@ -31,7 +31,9 @@ pub use commit_details::{ pub use commit_files::get_commit_files; pub use commits_info::{get_commits_info, CommitId, CommitInfo}; pub use diff::get_diff_commit; -pub use hooks::{hooks_commit_msg, hooks_post_commit, hooks_pre_commit, HookResult}; +pub use hooks::{ + hooks_commit_msg, hooks_post_commit, hooks_pre_commit, HookResult, +}; pub use hunks::{reset_hunk, stage_hunk, unstage_hunk}; pub use ignore::add_to_ignore; pub use logwalker::LogWalker; diff --git a/src/components/msg.rs b/src/components/msg.rs index 9f40436459..421295137d 100644 --- a/src/components/msg.rs +++ b/src/components/msg.rs @@ -7,12 +7,11 @@ use crossterm::event::Event; use tui::{ backend::Backend, layout::{Alignment, Rect}, - text::{Span, Spans}, + text::Span, widgets::{Block, BorderType, Borders, Clear, Paragraph, Wrap}, Frame, }; use ui::style::SharedTheme; - pub struct MsgComponent { title: String, msg: String, @@ -32,17 +31,26 @@ impl DrawableComponent for MsgComponent { if !self.visible { return Ok(()); } - let txt = Spans::from( - self.msg - .split('\n') - .map(|string| Span::raw::(string.to_string())) - .collect::>(), - ); - let area = ui::centered_rect_absolute(65, 25, f.size()); + // determine the maximum width of text block + let lens = self + .msg + .split('\n') + .map(|string| string.len()) + .collect::>(); + let max = lens.iter().max().unwrap(); + let mut width = (*max + 2) as u16; + // dont overflow screen, and dont get too narrow + if width > f.size().width { + width = f.size().width + } else if width < 60 { + width = 60 + } + + let area = ui::centered_rect_absolute(width, 25, f.size()); f.render_widget(Clear, area); f.render_widget( - Paragraph::new(txt) + Paragraph::new(self.msg.clone()) .block( Block::default() .title(Span::styled( From ab6362c135e3110d49edbe0e579f2738a5113233 Mon Sep 17 00:00:00 2001 From: pm100 Date: Wed, 28 Oct 2020 17:40:51 -0700 Subject: [PATCH 5/9] correct running of scripts with args set --- asyncgit/src/sync/hooks.rs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/asyncgit/src/sync/hooks.rs b/asyncgit/src/sync/hooks.rs index 8c3a89dbac..cbfa764dcd 100644 --- a/asyncgit/src/sync/hooks.rs +++ b/asyncgit/src/sync/hooks.rs @@ -110,14 +110,8 @@ fn run_hook( hook_script: &str, args: &[&str], ) -> HookResult { - let mut bash_args = - vec!["-c".to_string(), hook_script.to_string()]; - bash_args.extend_from_slice( - &args - .iter() - .map(|x| (*x).to_string()) - .collect::>(), - ); + let arg_str = format!("{} {}", hook_script, args.join(" ")); + let bash_args = vec!["-c".to_string(), arg_str]; let output = Command::new("bash") .args(bash_args) From 6ac6fbafe2ba72b2126206b786b4591ff67fbf34 Mon Sep 17 00:00:00 2001 From: pm100 Date: Wed, 28 Oct 2020 18:03:45 -0700 Subject: [PATCH 6/9] fix clippy pedantic --- src/components/msg.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/components/msg.rs b/src/components/msg.rs index 421295137d..ea062f1c53 100644 --- a/src/components/msg.rs +++ b/src/components/msg.rs @@ -4,6 +4,7 @@ use super::{ }; use crate::{keys::SharedKeyConfig, strings, ui}; use crossterm::event::Event; +use std::convert::TryFrom; use tui::{ backend::Backend, layout::{Alignment, Rect}, @@ -36,10 +37,10 @@ impl DrawableComponent for MsgComponent { let lens = self .msg .split('\n') - .map(|string| string.len()) + .map(str::len) .collect::>(); - let max = lens.iter().max().unwrap(); - let mut width = (*max + 2) as u16; + let max = lens.iter().max().expect("max"); + let mut width = u16::try_from(*max + 2).expect("cant fail"); // dont overflow screen, and dont get too narrow if width > f.size().width { width = f.size().width From 3039f3a5936a44c45559b9d331c8044ae3cdaa5c Mon Sep 17 00:00:00 2001 From: pm100 Date: Thu, 29 Oct 2020 20:52:27 -0700 Subject: [PATCH 7/9] feedback fixes, add tests --- README.md | 2 +- asyncgit/src/sync/hooks.rs | 46 ++++++++++++++++++++++++++++++++++++++ src/components/commit.rs | 2 +- src/components/msg.rs | 8 +++++-- 4 files changed, 54 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index cdc45c2a2b..3af8d1837d 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,7 @@ - Scalable terminal UI layout - Async [input polling](assets/perf_compare.jpg) - Async git API for fluid control -a + # Benchmarks For a [RustBerlin meetup presentation](https://youtu.be/rpilJV-eIVw?t=5334) ([slides](https://github.com/extrawurst/gitui-presentation)) I compared `lazygit`,`tig` and `gitui` by parsing the entire Linux git repository (which contains over 900k commits): diff --git a/asyncgit/src/sync/hooks.rs b/asyncgit/src/sync/hooks.rs index cbfa764dcd..20702087b1 100644 --- a/asyncgit/src/sync/hooks.rs +++ b/asyncgit/src/sync/hooks.rs @@ -215,6 +215,52 @@ exit 0 assert_eq!(msg, String::from("test")); } + #[test] + fn test_pre_commit_py() { + let (_td, repo) = repo_init().unwrap(); + let root = repo.path().parent().unwrap(); + let repo_path = root.as_os_str().to_str().unwrap(); + + // mirror how python pre-commmit sets itself up + #[cfg(not(windows))] + let hook = b"#!/bin/env python +import sys +sys.exit(0) + "; + #[cfg(windows)] + let hook = b"#!/bin/env python.exe +import sys +sys.exit(0) + "; + + create_hook(root, HOOK_PRE_COMMIT, hook); + let res = hooks_pre_commit(repo_path).unwrap(); + assert_eq!(res, HookResult::Ok); + } + + #[test] + fn test_pre_commit_fail_py() { + let (_td, repo) = repo_init().unwrap(); + let root = repo.path().parent().unwrap(); + let repo_path = root.as_os_str().to_str().unwrap(); + + // mirror how python pre-commmit sets itself up + #[cfg(not(windows))] + let hook = b"#!/bin/env python +import sys +sys.exit(1) + "; + #[cfg(windows)] + let hook = b"#!/bin/env python.exe +import sys +sys.exit(1) + "; + + create_hook(root, HOOK_PRE_COMMIT, hook); + let res = hooks_pre_commit(repo_path).unwrap(); + assert!(res != HookResult::Ok); + } + #[test] fn test_hooks_commit_msg_reject() { let (_td, repo) = repo_init().unwrap(); diff --git a/src/components/commit.rs b/src/components/commit.rs index 99a01189fc..708aeb1c40 100644 --- a/src/components/commit.rs +++ b/src/components/commit.rs @@ -198,7 +198,7 @@ impl CommitComponent { log::error!("pre-commit hook error: {}", e); self.queue.borrow_mut().push_back( InternalEvent::ShowErrorMsg(format!( - "commit-msg hook error:\n{}", + "pre-commit hook error:\n{}", e )), ); diff --git a/src/components/msg.rs b/src/components/msg.rs index ea062f1c53..3a50327d05 100644 --- a/src/components/msg.rs +++ b/src/components/msg.rs @@ -39,8 +39,12 @@ impl DrawableComponent for MsgComponent { .split('\n') .map(str::len) .collect::>(); - let max = lens.iter().max().expect("max"); - let mut width = u16::try_from(*max + 2).expect("cant fail"); + let mut max = lens.iter().max().expect("max") + 2; + if max > std::u16::MAX as usize { + max = std::u16::MAX as usize; + } + let mut width = + u16::try_from(max).expect("cant fail due to check above"); // dont overflow screen, and dont get too narrow if width > f.size().width { width = f.size().width From 38fa51358158873882048cefd839768d7dc5f4dc Mon Sep 17 00:00:00 2001 From: pm100 Date: Thu, 29 Oct 2020 21:32:14 -0700 Subject: [PATCH 8/9] fix linux/osx test failure --- asyncgit/src/sync/hooks.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/asyncgit/src/sync/hooks.rs b/asyncgit/src/sync/hooks.rs index 20702087b1..0f71ee4fed 100644 --- a/asyncgit/src/sync/hooks.rs +++ b/asyncgit/src/sync/hooks.rs @@ -223,7 +223,7 @@ exit 0 // mirror how python pre-commmit sets itself up #[cfg(not(windows))] - let hook = b"#!/bin/env python + let hook = b"#!/usr/bin/env python import sys sys.exit(0) "; @@ -246,7 +246,7 @@ sys.exit(0) // mirror how python pre-commmit sets itself up #[cfg(not(windows))] - let hook = b"#!/bin/env python + let hook = b"#!/usr/bin/env python import sys sys.exit(1) "; From e2e5e7e0d5b53e24c5f0204a42556df8a7ad4b7b Mon Sep 17 00:00:00 2001 From: pm100 Date: Sat, 31 Oct 2020 10:59:45 -0700 Subject: [PATCH 9/9] added shell based test --- asyncgit/src/sync/hooks.rs | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/asyncgit/src/sync/hooks.rs b/asyncgit/src/sync/hooks.rs index 0f71ee4fed..021fd2573a 100644 --- a/asyncgit/src/sync/hooks.rs +++ b/asyncgit/src/sync/hooks.rs @@ -215,6 +215,37 @@ exit 0 assert_eq!(msg, String::from("test")); } + #[test] + fn test_pre_commit_sh() { + let (_td, repo) = repo_init().unwrap(); + let root = repo.path().parent().unwrap(); + let repo_path = root.as_os_str().to_str().unwrap(); + + let hook = b"#!/bin/sh +exit 0 + "; + + create_hook(root, HOOK_PRE_COMMIT, hook); + let res = hooks_pre_commit(repo_path).unwrap(); + assert_eq!(res, HookResult::Ok); + } + + #[test] + fn test_pre_commit_fail_sh() { + let (_td, repo) = repo_init().unwrap(); + let root = repo.path().parent().unwrap(); + let repo_path = root.as_os_str().to_str().unwrap(); + + let hook = b"#!/bin/sh +echo 'rejected' +exit 1 + "; + + create_hook(root, HOOK_PRE_COMMIT, hook); + let res = hooks_pre_commit(repo_path).unwrap(); + assert!(res != HookResult::Ok); + } + #[test] fn test_pre_commit_py() { let (_td, repo) = repo_init().unwrap();