Skip to content

Commit 16b73ca

Browse files
committed
Warn about non-modified patches
Some patches do not modify an existing file in place. Instead they add, remove, or rename a file. This is a "non-modified patch", and will commute with everything in the stack. If all patches are non-modified, then no changes will be absorbed, so warn the user and stop processing. Even users that auto-stage changes will be warned, since they would've run git-absorb with the expectation that it would do something. Otherwise, if only some patches are non-modified, we can continue processing with a warning. In this case, do not warn users who auto-stage changes, since they may routinely keep untracked files in their working directory. Before this, 1. users who did not auto-stage were warned when nothing was staged. We have a more explicit warning and early exit above now. 2. there were staged changes (auto-staged or not) and all the patches were non-modified, we'd warn "Could not find a commit to fix up", which is accurate, but not as informative.
1 parent 535f660 commit 16b73ca

File tree

1 file changed

+32
-14
lines changed

1 file changed

+32
-14
lines changed

src/lib.rs

Lines changed: 32 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -130,19 +130,18 @@ fn run_with_repo(logger: &slog::Logger, config: &Config, repo: &git2::Repository
130130
let mut hunks_with_commit = vec![];
131131

132132
let mut modified_hunks_without_target = 0usize;
133-
let mut patches_considered = 0usize;
133+
let mut non_modified_patches = 0usize;
134134
'patch: for index_patch in index.iter() {
135135
let old_path = index_patch.new_path.as_slice();
136136
if index_patch.status != git2::Delta::Modified {
137-
debug!(logger, "skipped non-modified hunk";
137+
debug!(logger, "skipped non-modified patch";
138138
"path" => String::from_utf8_lossy(old_path).into_owned(),
139139
"status" => format!("{:?}", index_patch.status),
140140
);
141+
non_modified_patches += 1;
141142
continue 'patch;
142143
}
143144

144-
patches_considered += 1;
145-
146145
let mut preceding_hunks_offset = 0isize;
147146
let mut applied_hunks_offset = 0isize;
148147
'hunk: for index_hunk in &index_patch.hunks {
@@ -370,11 +369,25 @@ fn run_with_repo(logger: &slog::Logger, config: &Config, repo: &git2::Repository
370369
index.write()?;
371370
}
372371

373-
if patches_considered == 0 {
372+
if non_modified_patches == index.len() {
374373
warn!(
375374
logger,
376-
"Could not find a commit to fix up, use \
377-
--base to increase the search range."
375+
"No changes were in-place file modifications. \
376+
Added, removed, or renamed files cannot be automatically absorbed."
377+
);
378+
return Ok(());
379+
}
380+
381+
// So long as there was a patch that had the possibility of fixing up
382+
// a commit, warn about the presence of patches that will commute with
383+
// everything.
384+
// Users that auto-stage changes may be accustomed to having untracked files
385+
// in their workspace that are not absorbed, so don't warn them.
386+
if non_modified_patches > 0 && !we_added_everything_to_index {
387+
warn!(
388+
logger,
389+
"Some changes were not in-place file modifications. \
390+
Added, removed, or renamed files cannot be automatically absorbed."
378391
)
379392
}
380393

@@ -391,7 +404,7 @@ fn run_with_repo(logger: &slog::Logger, config: &Config, repo: &git2::Repository
391404
return Ok(());
392405
}
393406

394-
if config.and_rebase && patches_considered != 0 {
407+
if config.and_rebase && !hunks_with_commit.is_empty() {
395408
use std::process::Command;
396409
// unwrap() is safe here, as we exit early if the stack is empty
397410
let last_commit_in_stack = &stack.last().unwrap().0;
@@ -677,8 +690,8 @@ mod tests {
677690
}),
678691
&json!({
679692
"level": "WARN",
680-
"msg": "Could not find a commit to fix up, \
681-
use --base to increase the search range.",
693+
"msg": "No changes were in-place file modifications. \
694+
Added, removed, or renamed files cannot be automatically absorbed.",
682695
}),
683696
],
684697
);
@@ -721,6 +734,11 @@ mod tests {
721734
use --base or configure absorb.maxStack to override",
722735
"limit": config::MAX_STACK,
723736
}),
737+
&json!({
738+
"level": "WARN",
739+
"msg": "Some changes were not in-place file modifications. \
740+
Added, removed, or renamed files cannot be automatically absorbed.",
741+
}),
724742
&json!({
725743
"level": "WARN",
726744
"msg": "Could not find a commit to fix up, \
@@ -752,8 +770,8 @@ mod tests {
752770
capturing_logger.visible_logs(),
753771
vec![&json!({
754772
"level": "WARN",
755-
"msg": "Could not find a commit to fix up, \
756-
use --base to increase the search range.",
773+
"msg": "No changes were in-place file modifications. \
774+
Added, removed, or renamed files cannot be automatically absorbed."
757775
})],
758776
);
759777
}
@@ -1465,8 +1483,8 @@ mod tests {
14651483
capturing_logger.visible_logs(),
14661484
vec![&json!({
14671485
"level": "WARN",
1468-
"msg": "Could not find a commit to fix up, \
1469-
use --base to increase the search range."
1486+
"msg": "No changes were in-place file modifications. \
1487+
Added, removed, or renamed files cannot be automatically absorbed."
14701488
})],
14711489
);
14721490
}

0 commit comments

Comments
 (0)