Skip to content

Commit b932754

Browse files
DaRacciDaRacci
committed
feat: implement option for hook timeout and abort the commit
1 parent 63acc53 commit b932754

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
@@ -236,16 +236,28 @@ impl CommitPopup {
236236

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

@@ -254,30 +266,53 @@ impl CommitPopup {
254266

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

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

283318
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)