Skip to content

Commit

Permalink
Try to be more helpful when git-absorb does nothing or stops early
Browse files Browse the repository at this point in the history
- Print "Please use --base to specify the base commit." when all the
  commits were "hidden" by other branches. (This makes the misbehavior
  described in tummychow#9 less confusing.)

- Instead of noting that "merge commit found", explain what it means to
  the user: "Will not fix up past the merge commit".

- Provide suggestions when "stack limit reached": "use --base or
  configure absorb.maxStack to override"

  - Don't honor absorb.maxStack with explicit --base

- Note if the stack is empty: "No commits available to fix up, exiting"

- Warn if "No additions staged, try adding something to the index."

- Print "Could not find a commit to fix up, use --base to increase the
  search range." instead of "could not find noncommutative commit", and
  display this message by default (without the verbose mode)
  • Loading branch information
nickolay committed Jul 14, 2019
1 parent bca8bce commit 0b72296
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 5 deletions.
17 changes: 15 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@ pub fn run(config: &Config) -> Result<(), failure::Error> {
let repo = git2::Repository::open_from_env()?;
debug!(config.logger, "repository found"; "path" => repo.path().to_str());

let stack = stack::working_stack(&repo, config.base, config.logger)?;
if stack.is_empty() {
crit!(config.logger, "No commits available to fix up, exiting");
return Ok(());
}

let mut diff_options = Some({
let mut ret = git2::DiffOptions::new();
ret.context_lines(0)
Expand All @@ -31,7 +37,6 @@ pub fn run(config: &Config) -> Result<(), failure::Error> {
});

let (stack, summary_counts): (Vec<_>, _) = {
let stack = stack::working_stack(&repo, config.base, config.logger)?;
let mut diffs = Vec::with_capacity(stack.len());
for commit in &stack {
let diff = owned::Diff::new(
Expand Down Expand Up @@ -73,6 +78,7 @@ pub fn run(config: &Config) -> Result<(), failure::Error> {
let signature = repo.signature()?;
let mut head_commit = repo.head()?.peel_to_commit()?;

let mut patches_considered = 0usize;
'patch: for index_patch in index.iter() {
let old_path = index_patch.new_path.as_slice();
if index_patch.status != git2::Delta::Modified {
Expand All @@ -83,6 +89,8 @@ pub fn run(config: &Config) -> Result<(), failure::Error> {
continue 'patch;
}

patches_considered += 1;

let mut preceding_hunks_offset = 0isize;
let mut applied_hunks_offset = 0isize;
'hunk: for index_hunk in &index_patch.hunks {
Expand Down Expand Up @@ -193,7 +201,8 @@ pub fn run(config: &Config) -> Result<(), failure::Error> {
// the hunk commutes with every commit in the stack,
// so there is no commit to absorb it into
None => {
debug!(config.logger, "could not find noncommutative commit");
warn!(config.logger, "Could not find a commit to fix up, use \
--base to increase the search range.");
continue 'hunk;
}
};
Expand Down Expand Up @@ -232,6 +241,10 @@ pub fn run(config: &Config) -> Result<(), failure::Error> {
}
}

if patches_considered == 0 {
warn!(config.logger, "No additions staged, try adding something to the index.");
}

Ok(())
}

Expand Down
16 changes: 13 additions & 3 deletions src/stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,13 @@ pub fn working_stack<'repo>(
}

let mut ret = Vec::new();
let mut commits_considered = 0usize;
let sig = repo.signature()?;
for rev in revwalk {
commits_considered += 1;
let commit = repo.find_commit(rev?)?;
if commit.parents().len() > 1 {
warn!(logger, "merge commit found"; "commit" => commit.id().to_string());
warn!(logger, "Will not fix up past the merge commit"; "commit" => commit.id().to_string());
break;
}
if commit.author().name_bytes() != sig.name_bytes()
Expand All @@ -74,13 +76,21 @@ pub fn working_stack<'repo>(
warn!(logger, "foreign author found"; "commit" => commit.id().to_string());
break;
}
if ret.len() == max_stack(repo) {
warn!(logger, "stack limit reached"; "limit" => ret.len());
if ret.len() == max_stack(repo) && user_provided_base.is_none() {
warn!(logger, "stack limit reached, use --base or configure absorb.maxStack to override";
"limit" => ret.len());
break;
}
debug!(logger, "commit pushed onto stack"; "commit" => commit.id().to_string());
ret.push(commit);
}
if commits_considered == 0 {
if user_provided_base.is_none() {
warn!(logger, "Please use --base to specify a base commit.");
} else {
warn!(logger, "Please try a different --base");
}
}
Ok(ret)
}

Expand Down

0 comments on commit 0b72296

Please sign in to comment.