Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fallback to file-based approach for finding appropriate commit to amend #3466

Open
jesseduffield opened this issue Apr 1, 2024 · 2 comments
Labels
enhancement New feature or request

Comments

@jesseduffield
Copy link
Owner

Is your feature request related to a problem? Please describe.
I just had a situation where I had two commits on a branch (i.e. two ahead of the main branch) and one staged file which was included in the first of the two commits. The change to that file was in a separate part of the file (it was updating an import statement at the top of the file).

Upon trying to find the appropriate commit, I was told that multiple base commits were found, neither of which were my two branch commits.

Describe the solution you'd like
What I would like in this case is for lazygit to fall back to just looking to see if there is a single commit on the current branch which changed the staged file(s). If there are multiple such commits, we can just return an error, and if there is one such commit, we can show a confirmation popup saying something like 'We couldn't do a perfect match but we did find one commit which changed the staged files, jump to commit?' (I personally don't think the confirmation is necessary but I suspect you would want this @stefanhaller ).

Additional context
@stefanhaller we have definitely talked about this in the past, and we considered having a separate keybinding for taking the file-based approach, but in this case I didn't know ahead of time whether the patch-based approach would work or not (I assumed that it would), so now I think we should just extend the logic of the existing keybinding.

@jesseduffield jesseduffield added the enhancement New feature or request label Apr 1, 2024
@stefanhaller
Copy link
Collaborator

A few thoughts:

  • I agree that it should be included in the logic of the existing keybinding, not be a separate command.
  • While the proposed logic would have solved it for the case you described, I can also imagine cases where it would pick the wrong commit (especially related to import statements, that's a good example.) Example where it would guess wrong: you rename a header file in C++, and forget to adapt all the #include statements. I have to say that I'm totally unsure how much more likely it is to guess right than guess wrong, or how to find out.
  • I would make this only the second fallback. As a first fallback, I'd suggest a heuristic similar to what git-absorb does, which is a tool that tries to solve the same problem as our <c-f> command does, but using a very different approach. The approach is very interesting (I was only recently made aware of it), and it has the advantage that it works for the case where you add a comment for a new function. We might also consider switching to that approach altogether, although it is slower than our current one, and seems to be more complicated to implement, though I haven't looked into that very much yet.

@jesseduffield
Copy link
Owner Author

While the proposed logic would have solved it for the case you described, I can also imagine cases where it would pick the wrong commit (especially related to import statements, that's a good example.) Example where it would guess wrong: you rename a header file in C++, and forget to adapt all the #include statements. I have to say that I'm totally unsure how much more likely it is to guess right than guess wrong, or how to find out.

Good point, though I'm not worried by that if we're clear that we found a match based on matching file.

I would make this only the second fallback.

Interesting: git-absorb's approach seems reasonable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants