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.

Sign up for GitHub

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

Jump to bottom

Focus commits view after moving custom patch into new commit #3200

Closed
jesseduffield opened this issue Jan 5, 2024 · 3 comments · Fixed by #3577
Closed

Focus commits view after moving custom patch into new commit #3200

jesseduffield opened this issue Jan 5, 2024 · 3 comments · Fixed by #3577
Assignees
@AzraelSec
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@jesseduffield
Copy link
Owner

Is your feature request related to a problem? Please describe.
When a custom patch is moved out into a new commit, we stay in the commit files view but end up looking at the new commit's files. This is a bit jarring: the important context we want to show the user is where this new commit is situated compared to the existing ones. The user already knows what changes the new commit contains because they just selected the changes themselves, and if they want to double-check they can easily press enter to view them.

Describe the solution you'd like
Focus the commits view after moving a custom patch out into a new commit.
@jesseduffield jesseduffield added enhancement New feature or request good first issue Good for newcomers labels Jan 5, 2024
@jesseduffield jesseduffield mentioned this issue Jan 9, 2024
Closed
@AzraelSec
Copy link
Contributor

Hey @jesseduffield - I started working on this, but I need clarification on the best path.

I was trying to intercept the error coming from CheckMergeOrRebase to avoid focusing the local commit's context in case of an error here:

lazygit/pkg/gui/controllers/custom_patch_options_menu_action.go

Line 219 in 49df908

return self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(err)

The problem is that the isMergeConflictErr method does not propagate the original error and returns the one coming from the ErrorMsg method:

lazygit/pkg/gui/controllers/helpers/merge_and_rebase_helper.go

Line 173 in 49df908

return self.c.ErrorMsg(result.Error())

What do you think is the best solution here? Thanks!

P.S. Feel free to assign this issue to me :)

@AzraelSec AzraelSec mentioned this issue May 15, 2024
Closed
@jesseduffield
Copy link
Owner Author

@AzraelSec sorry for the late reply: Let me know if this isn't what you asked, but if we get a merge conflict, then I would still want the commit view to be focused because that's what the user would want to see.

@AzraelSec AzraelSec mentioned this issue May 19, 2024
Merged
6 tasks
@AzraelSec
Copy link
Contributor

@jesseduffield thanks for responding. I just restarted from a blank branch a couple of days ago and am trying to remember exactly what my comment's context was. I created a PR for this; let's see if my code looks what you'd expect it to be 👍🏻

stefanhaller added a commit that referenced this issue May 20, 2024
@stefanhaller
Focus on local commits view after moving code into new commit (#3577)
6343fb5 
- **PR Description**

This PR forces lazygit to focus to the `Commits` view after moving a
custom (partial) patch in a new commit.

This closes #3200.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@AzraelSec AzraelSec

Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Focus on local commits view after moving code into new commit AzraelSec/lazygit
2 participants
@AzraelSec @jesseduffield