Skip to content

Commit 2fe5f28

Browse files
committed
feat: implement option for hook timeout and abort the commit
1 parent 4f8b5ba commit 2fe5f28

File tree

4 files changed

+130
-35
lines changed

4 files changed

+130
-35
lines changed

src/app.rs

+1
Original file line numberDiff line numberDiff line change
@@ -846,6 +846,7 @@ impl App {
846846
| AppOption::DiffInterhunkLines => {
847847
self.status_tab.update_diff()?;
848848
}
849+
AppOption::HookTimeout => {}
849850
}
850851

851852
flags.insert(NeedsUpdate::ALL);

src/options.rs

+13
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ use std::{
1414
io::{Read, Write},
1515
path::PathBuf,
1616
rc::Rc,
17+
time::Duration,
1718
};
1819

1920
#[derive(Default, Clone, Serialize, Deserialize)]
@@ -22,6 +23,7 @@ struct OptionsData {
2223
pub diff: DiffOptions,
2324
pub status_show_untracked: Option<ShowUntrackedFilesConfig>,
2425
pub commit_msgs: Vec<String>,
26+
pub hook_timeout: Option<Duration>,
2527
}
2628

2729
const COMMIT_MSG_HISTORY_LENGTH: usize = 20;
@@ -66,6 +68,11 @@ impl Options {
6668
self.data.diff
6769
}
6870

71+
#[allow(unused)]
72+
pub const fn hook_timeout(&self) -> Option<Duration> {
73+
self.data.hook_timeout
74+
}
75+
6976
pub const fn status_show_untracked(
7077
&self,
7178
) -> Option<ShowUntrackedFilesConfig> {
@@ -137,6 +144,12 @@ impl Options {
137144
}
138145
}
139146

147+
#[allow(unused)]
148+
pub fn set_hook_timeout(&mut self, timeout: Option<Duration>) {
149+
self.data.hook_timeout = timeout;
150+
self.save();
151+
}
152+
140153
fn save(&self) {
141154
if let Err(e) = self.save_failable() {
142155
log::error!("options save error: {}", e);

src/popups/commit.rs

+69-33
Original file line numberDiff line numberDiff line change
@@ -238,16 +238,28 @@ impl CommitPopup {
238238

239239
if verify {
240240
// run pre commit hook - can reject commit
241-
if let HookResult::NotOk(e) =
242-
sync::hooks_pre_commit_with_timeout(
243-
&self.repo.borrow(),
244-
self.get_hook_timeout(),
245-
)? {
246-
log::error!("pre-commit hook error: {}", e);
247-
self.queue.push(InternalEvent::ShowErrorMsg(
248-
format!("pre-commit hook error:\n{e}"),
249-
));
250-
return Ok(CommitResult::Aborted);
241+
match sync::hooks_pre_commit_with_timeout(
242+
&self.repo.borrow(),
243+
self.get_hook_timeout(),
244+
)? {
245+
HookResult::NotOk(e) => {
246+
log::error!("pre-commit hook error: {}", e);
247+
self.queue.push(InternalEvent::ShowErrorMsg(
248+
format!("pre-commit hook error:\n{e}"),
249+
));
250+
return Ok(CommitResult::Aborted);
251+
}
252+
HookResult::TimedOut => {
253+
log::error!("pre-commit hook timed out");
254+
self.queue.push(InternalEvent::ShowErrorMsg(
255+
format!(
256+
"pre-commit hook timed out after {} seconds",
257+
self.get_hook_timeout().as_secs()
258+
),
259+
));
260+
return Ok(CommitResult::Aborted);
261+
}
262+
HookResult::Ok => {}
251263
}
252264
}
253265

@@ -256,30 +268,53 @@ impl CommitPopup {
256268

257269
if verify {
258270
// run commit message check hook - can reject commit
259-
if let HookResult::NotOk(e) =
260-
sync::hooks_commit_msg_with_timeout(
261-
&self.repo.borrow(),
262-
&mut msg,
263-
self.get_hook_timeout(),
264-
)? {
265-
log::error!("commit-msg hook error: {}", e);
266-
self.queue.push(InternalEvent::ShowErrorMsg(
267-
format!("commit-msg hook error:\n{e}"),
268-
));
269-
return Ok(CommitResult::Aborted);
271+
match sync::hooks_commit_msg_with_timeout(
272+
&self.repo.borrow(),
273+
&mut msg,
274+
self.get_hook_timeout(),
275+
)? {
276+
HookResult::NotOk(e) => {
277+
log::error!("commit-msg hook error: {}", e);
278+
self.queue.push(InternalEvent::ShowErrorMsg(
279+
format!("commit-msg hook error:\n{e}"),
280+
));
281+
return Ok(CommitResult::Aborted);
282+
}
283+
HookResult::TimedOut => {
284+
log::error!("commit-msg hook timed out");
285+
self.queue.push(InternalEvent::ShowErrorMsg(
286+
format!(
287+
"commit-msg hook timed out after {} seconds",
288+
self.get_hook_timeout().as_secs()
289+
),
290+
));
291+
return Ok(CommitResult::Aborted);
292+
}
293+
HookResult::Ok => {}
270294
}
271295
}
272296
self.do_commit(&msg)?;
273297

274-
if let HookResult::NotOk(e) =
275-
sync::hooks_post_commit_with_timeout(
276-
&self.repo.borrow(),
277-
self.get_hook_timeout(),
278-
)? {
279-
log::error!("post-commit hook error: {}", e);
280-
self.queue.push(InternalEvent::ShowErrorMsg(format!(
281-
"post-commit hook error:\n{e}"
282-
)));
298+
match sync::hooks_post_commit_with_timeout(
299+
&self.repo.borrow(),
300+
self.get_hook_timeout(),
301+
)? {
302+
HookResult::NotOk(e) => {
303+
log::error!("post-commit hook error: {}", e);
304+
self.queue.push(InternalEvent::ShowErrorMsg(
305+
format!("post-commit hook error:\n{e}"),
306+
));
307+
}
308+
HookResult::TimedOut => {
309+
log::error!("post-commit hook timed out");
310+
self.queue.push(InternalEvent::ShowErrorMsg(
311+
format!(
312+
"post-commit hook timed out after {} seconds",
313+
self.get_hook_timeout().as_secs()
314+
),
315+
));
316+
}
317+
HookResult::Ok => {}
283318
}
284319

285320
Ok(CommitResult::CommitDone)
@@ -488,10 +523,11 @@ impl CommitPopup {
488523
Ok(msg)
489524
}
490525

491-
// TODO - Configurable timeout
492-
#[allow(clippy::unused_self, clippy::missing_const_for_fn)]
493526
fn get_hook_timeout(&self) -> Duration {
494-
Duration::from_secs(5)
527+
self.options
528+
.borrow()
529+
.hook_timeout()
530+
.unwrap_or(Duration::ZERO)
495531
}
496532
}
497533

src/popups/options.rs

+47-2
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
use std::time::Duration;
2+
13
use crate::{
24
app::Environment,
35
components::{
@@ -27,6 +29,7 @@ pub enum AppOption {
2729
DiffIgnoreWhitespaces,
2830
DiffContextLines,
2931
DiffInterhunkLines,
32+
HookTimeout,
3033
}
3134

3235
pub struct OptionsPopup {
@@ -99,6 +102,19 @@ impl OptionsPopup {
99102
&diff.interhunk_lines.to_string(),
100103
self.is_select(AppOption::DiffInterhunkLines),
101104
);
105+
Self::add_header(txt, "");
106+
107+
Self::add_header(txt, "Hooks");
108+
self.add_entry(
109+
txt,
110+
width,
111+
"Timeout",
112+
&self.options.borrow().hook_timeout().map_or_else(
113+
|| "None".to_string(),
114+
|d| format!("{d:?}"),
115+
),
116+
self.is_select(AppOption::HookTimeout),
117+
);
102118
}
103119

104120
fn is_select(&self, kind: AppOption) -> bool {
@@ -138,7 +154,7 @@ impl OptionsPopup {
138154
if up {
139155
self.selection = match self.selection {
140156
AppOption::StatusShowUntracked => {
141-
AppOption::DiffInterhunkLines
157+
AppOption::HookTimeout
142158
}
143159
AppOption::DiffIgnoreWhitespaces => {
144160
AppOption::StatusShowUntracked
@@ -149,6 +165,9 @@ impl OptionsPopup {
149165
AppOption::DiffInterhunkLines => {
150166
AppOption::DiffContextLines
151167
}
168+
AppOption::HookTimeout => {
169+
AppOption::DiffInterhunkLines
170+
}
152171
};
153172
} else {
154173
self.selection = match self.selection {
@@ -162,6 +181,9 @@ impl OptionsPopup {
162181
AppOption::DiffInterhunkLines
163182
}
164183
AppOption::DiffInterhunkLines => {
184+
AppOption::HookTimeout
185+
}
186+
AppOption::HookTimeout => {
165187
AppOption::StatusShowUntracked
166188
}
167189
};
@@ -207,6 +229,14 @@ impl OptionsPopup {
207229
.borrow_mut()
208230
.diff_hunk_lines_change(true);
209231
}
232+
AppOption::HookTimeout => {
233+
let current =
234+
self.options.borrow().hook_timeout();
235+
let inc = Duration::from_secs(1);
236+
let new = current.map(|d| d + inc).or(Some(inc));
237+
238+
self.options.borrow_mut().set_hook_timeout(new);
239+
}
210240
}
211241
} else {
212242
match self.selection {
@@ -246,6 +276,21 @@ impl OptionsPopup {
246276
.borrow_mut()
247277
.diff_hunk_lines_change(false);
248278
}
279+
AppOption::HookTimeout => {
280+
let current =
281+
self.options.borrow().hook_timeout();
282+
let dec = Duration::from_secs(1);
283+
284+
let new = current.and_then(|d| {
285+
if d > dec {
286+
Some(d - dec)
287+
} else {
288+
None
289+
}
290+
});
291+
292+
self.options.borrow_mut().set_hook_timeout(new);
293+
}
249294
}
250295
}
251296

@@ -257,7 +302,7 @@ impl OptionsPopup {
257302
impl DrawableComponent for OptionsPopup {
258303
fn draw(&self, f: &mut Frame, area: Rect) -> Result<()> {
259304
if self.is_visible() {
260-
const SIZE: (u16, u16) = (50, 10);
305+
const SIZE: (u16, u16) = (50, 12);
261306
let area =
262307
ui::centered_rect_absolute(SIZE.0, SIZE.1, area);
263308

0 commit comments

Comments
 (0)