Skip to content

Commit 1541024

Browse files
blairconradblairconrad
committed
Consider reason for stack end when crafting messages for user
1 parent 16b73ca commit 1541024

File tree

2 files changed

+232
-142
lines changed

2 files changed

+232
-142
lines changed

‎src/lib.rs‎

Lines changed: 138 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ fn run_with_repo(logger: &slog::Logger, config: &Config, repo: &git2::Repository
6969
}
7070
}
7171

72-
let stack = stack::working_stack(
72+
let (stack, stack_end_reason) = stack::working_stack(
7373
repo,
7474
config.base,
7575
config.force_author,
@@ -394,14 +394,63 @@ fn run_with_repo(logger: &slog::Logger, config: &Config, repo: &git2::Repository
394394
if modified_hunks_without_target > 0 {
395395
warn!(
396396
logger,
397-
"Could not find a commit to fix up, use \
398-
--base to increase the search range."
397+
"Some file modifications did not have an available commit to fix up. \
398+
You will have to manually create fixup commits."
399399
);
400-
}
401400

402-
if stack.is_empty() {
403-
crit!(logger, "No commits available to fix up, exiting");
404-
return Ok(());
401+
match stack_end_reason {
402+
stack::StackEndReason::ReachedRoot => {
403+
warn!(
404+
logger,
405+
"Cannot fix up past first commit in the repository.";
406+
);
407+
}
408+
stack::StackEndReason::ReachedMergeCommit => {
409+
warn!(
410+
logger,
411+
"Cannot fix up past a merge commit";
412+
"commit" => match stack.last() {
413+
Some(commit) => commit.0.id().to_string(),
414+
None => head_commit.id().to_string(),
415+
}
416+
);
417+
}
418+
stack::StackEndReason::ReachedAnotherAuthor => {
419+
warn!(
420+
logger,
421+
"Will not fix up past commits by another author. \
422+
Use --force-author to override";
423+
"commit" => match stack.last() {
424+
Some(commit) => commit.0.id().to_string(),
425+
None => head_commit.id().to_string(),
426+
}
427+
);
428+
}
429+
stack::StackEndReason::ReachedLimit => {
430+
warn!(
431+
logger,
432+
"Will not fix up past maximum stack limit. \
433+
Use --base or configure {} to override",
434+
config::MAX_STACK_CONFIG_NAME;
435+
"limit" => config::max_stack(repo),
436+
);
437+
}
438+
stack::StackEndReason::CommitsHiddenByBase => {
439+
warn!(
440+
logger,
441+
"Will not fix up past specified base commit. \
442+
Consider using --base to specify a different base commit";
443+
"base" => config.base.unwrap(),
444+
);
445+
}
446+
stack::StackEndReason::CommitsHiddenByBranches => {
447+
warn!(
448+
logger,
449+
"Will not fix up commits reachable by other branches. \
450+
Use --base to specify a base commit.";
451+
);
452+
}
453+
}
405454
}
406455

407456
if config.and_rebase && !hunks_with_commit.is_empty() {
@@ -637,14 +686,17 @@ mod tests {
637686
vec![
638687
&json!({
639688
"level": "WARN",
640-
"msg": "stack limit reached, \
641-
use --base or configure absorb.maxStack to override",
642-
"limit": config::MAX_STACK,
689+
"msg": "Some file modifications did not have an available commit to fix up. \
690+
You will have to manually create fixup commits.",
643691
}),
644692
&json!({
645693
"level": "WARN",
646-
"msg": "Could not find a commit to fix up, \
647-
use --base to increase the search range.",
694+
"msg": format!(
695+
"Will not fix up past maximum stack limit. \
696+
Use --base or configure {} to override",
697+
config::MAX_STACK_CONFIG_NAME
698+
),
699+
"limit": config::MAX_STACK,
648700
}),
649701
],
650702
);
@@ -681,19 +733,11 @@ mod tests {
681733

682734
log_utils::assert_log_messages_are(
683735
capturing_logger.visible_logs(),
684-
vec![
685-
&json!({
686-
"level": "WARN",
687-
"msg": "stack limit reached, \
688-
use --base or configure absorb.maxStack to override",
689-
"limit": config::MAX_STACK,
690-
}),
691-
&json!({
692-
"level": "WARN",
693-
"msg": "No changes were in-place file modifications. \
694-
Added, removed, or renamed files cannot be automatically absorbed.",
695-
}),
696-
],
736+
vec![&json!({
737+
"level": "WARN",
738+
"msg": "No changes were in-place file modifications. \
739+
Added, removed, or renamed files cannot be automatically absorbed.",
740+
})],
697741
);
698742
}
699743

@@ -730,19 +774,21 @@ mod tests {
730774
vec![
731775
&json!({
732776
"level": "WARN",
733-
"msg": "stack limit reached, \
734-
use --base or configure absorb.maxStack to override",
735-
"limit": config::MAX_STACK,
777+
"msg": "Some changes were not in-place file modifications. \
778+
Added, removed, or renamed files cannot be automatically absorbed.",
736779
}),
737780
&json!({
738781
"level": "WARN",
739-
"msg": "Some changes were not in-place file modifications. \
740-
Added, removed, or renamed files cannot be automatically absorbed.",
782+
"msg": "Some file modifications did not have an available commit to fix up. \
783+
You will have to manually create fixup commits.",
741784
}),
742785
&json!({
743786
"level": "WARN",
744-
"msg": "Could not find a commit to fix up, \
745-
use --base to increase the search range.",
787+
"msg": format!(
788+
"Will not fix up past maximum stack limit. \
789+
Use --base or configure {} to override",
790+
config::MAX_STACK_CONFIG_NAME
791+
),
746792
}),
747793
],
748794
);
@@ -800,16 +846,14 @@ mod tests {
800846
vec![
801847
&json!({
802848
"level": "WARN",
803-
"msg": "Please try a different --base",
849+
"msg": "Some file modifications did not have an available commit to fix up. \
850+
You will have to manually create fixup commits.",
804851
}),
805852
&json!({
806853
"level": "WARN",
807-
"msg": "Could not find a commit to fix up, \
808-
use --base to increase the search range.",
809-
}),
810-
&json!({
811-
"level": "CRIT",
812-
"msg": "No commits available to fix up, exiting",
854+
"msg": "Will not fix up past specified base commit. \
855+
Consider using --base to specify a different base commit",
856+
"base": "HEAD",
813857
}),
814858
],
815859
);
@@ -840,16 +884,12 @@ mod tests {
840884
vec![
841885
&json!({
842886
"level": "WARN",
843-
"msg": "Will not fix up past the merge commit",
887+
"msg": "Some file modifications did not have an available commit to fix up. \
888+
You will have to manually create fixup commits.",
844889
}),
845890
&json!({
846891
"level": "WARN",
847-
"msg": "Could not find a commit to fix up, \
848-
use --base to increase the search range.",
849-
}),
850-
&json!({
851-
"level": "CRIT",
852-
"msg": "No commits available to fix up, exiting",
892+
"msg": "Cannot fix up past a merge commit",
853893
}),
854894
],
855895
);
@@ -887,16 +927,10 @@ mod tests {
887927

888928
log_utils::assert_log_messages_are(
889929
capturing_logger.visible_logs(),
890-
vec![
891-
&json!({
892-
"level": "WARN",
893-
"msg": "Will not fix up past the merge commit",
894-
}),
895-
&json!({
896-
"level": "INFO",
897-
"msg": "committed",
898-
}),
899-
],
930+
vec![&json!({
931+
"level": "INFO",
932+
"msg": "committed",
933+
})],
900934
);
901935
}
902936

@@ -928,16 +962,22 @@ mod tests {
928962

929963
log_utils::assert_log_messages_are(
930964
capturing_logger.visible_logs(),
931-
vec![&json!({
932-
"level": "WARN",
933-
"msg": "Could not find a commit to fix up, \
934-
use --base to increase the search range.",
935-
})],
965+
vec![
966+
&json!({
967+
"level": "WARN",
968+
"msg": "Some file modifications did not have an available commit to fix up. \
969+
You will have to manually create fixup commits.",
970+
}),
971+
&json!({
972+
"level": "WARN",
973+
"msg": "Cannot fix up past a merge commit",
974+
}),
975+
],
936976
);
937977
}
938978

939979
#[test]
940-
fn first_hidden_commit_is_foreign_author() {
980+
fn first_hidden_commit_is_by_another_author() {
941981
let (ctx, file_path) = repo_utils::prepare_repo();
942982
let first_commit = ctx.repo.head().unwrap().peel_to_commit().unwrap();
943983
ctx.repo
@@ -960,11 +1000,18 @@ mod tests {
9601000

9611001
log_utils::assert_log_messages_are(
9621002
capturing_logger.visible_logs(),
963-
vec![&json!({
964-
"level": "WARN",
965-
"msg": "Could not find a commit to fix up, \
966-
use --base to increase the search range.",
967-
})],
1003+
vec![
1004+
&json!({
1005+
"level": "WARN",
1006+
"msg": "Some file modifications did not have an available commit to fix up. \
1007+
You will have to manually create fixup commits.",
1008+
}),
1009+
&json!({
1010+
"level": "WARN",
1011+
"msg": "Will not fix up past commits by another author. \
1012+
Use --force-author to override",
1013+
}),
1014+
],
9681015
);
9691016
}
9701017

@@ -991,11 +1038,18 @@ mod tests {
9911038

9921039
log_utils::assert_log_messages_are(
9931040
capturing_logger.visible_logs(),
994-
vec![&json!({
995-
"level": "WARN",
996-
"msg": "Could not find a commit to fix up, \
997-
use --base to increase the search range.",
998-
})],
1041+
vec![
1042+
&json!({
1043+
"level": "WARN",
1044+
"msg": "Some file modifications did not have an available commit to fix up. \
1045+
You will have to manually create fixup commits.",
1046+
}),
1047+
&json!({
1048+
"level": "WARN",
1049+
"msg": "Will not fix up commits reachable by other branches. \
1050+
Use --base to specify a base commit.",
1051+
}),
1052+
],
9991053
);
10001054
}
10011055

@@ -1024,7 +1078,7 @@ mod tests {
10241078
}
10251079

10261080
#[test]
1027-
fn foreign_author() {
1081+
fn another_author() {
10281082
let ctx = repo_utils::prepare_and_stage();
10291083

10301084
repo_utils::become_author(&ctx.repo, "nobody2", "[email protected]");
@@ -1044,21 +1098,20 @@ mod tests {
10441098
vec![
10451099
&json!({
10461100
"level": "WARN",
1047-
"msg": "Will not fix up past commits not authored by you, \
1048-
use --force-author to override",
1101+
"msg": "Some file modifications did not have an available commit to fix up. \
1102+
You will have to manually create fixup commits.",
10491103
}),
10501104
&json!({
10511105
"level": "WARN",
1052-
"msg": "Could not find a commit to fix up, \
1053-
use --base to increase the search range.",
1106+
"msg": "Will not fix up past commits by another author. \
1107+
Use --force-author to override"
10541108
}),
1055-
&json!({"level": "CRIT", "msg": "No commits available to fix up, exiting"}),
10561109
],
10571110
);
10581111
}
10591112

10601113
#[test]
1061-
fn foreign_author_with_force_author_flag() {
1114+
fn another_author_with_force_author_flag() {
10621115
let ctx = repo_utils::prepare_and_stage();
10631116

10641117
repo_utils::become_author(&ctx.repo, "nobody2", "[email protected]");
@@ -1087,7 +1140,7 @@ mod tests {
10871140
}
10881141

10891142
#[test]
1090-
fn foreign_author_with_force_author_config() {
1143+
fn another_author_with_force_author_config() {
10911144
let ctx = repo_utils::prepare_and_stage();
10921145

10931146
repo_utils::become_author(&ctx.repo, "nobody2", "[email protected]");
@@ -1157,18 +1210,17 @@ mod tests {
11571210
vec![
11581211
&json!({
11591212
"level": "WARN",
1160-
"msg": "HEAD is not a branch, but --force-detach used to continue.",
1161-
}),
1213+
"msg": "HEAD is not a branch, but --force-detach used to continue."}),
11621214
&json!({
11631215
"level": "WARN",
1164-
"msg": "Please use --base to specify a base commit.",
1216+
"msg": "Some file modifications did not have an available commit to fix up. \
1217+
You will have to manually create fixup commits.",
11651218
}),
11661219
&json!({
11671220
"level": "WARN",
1168-
"msg": "Could not find a commit to fix up, \
1169-
use --base to increase the search range.",
1221+
"msg": "Will not fix up commits reachable by other branches. \
1222+
Use --base to specify a base commit."
11701223
}),
1171-
&json!({"level": "CRIT", "msg": "No commits available to fix up, exiting"}),
11721224
],
11731225
);
11741226
}

0 commit comments

Comments
 (0)