Skip to content
New issue

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

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

Already on GitHub? # to your account

Section headers for rebase todos and actual commits #4463

Open
wants to merge 4 commits into
base: revert-range-selection-of-commits
Choose a base branch
from

Conversation

stefanhaller
Copy link
Collaborator

  • PR Description

During interactive rebase, or when stopping with conflicts in a cherry-pick or revert, show section headers for the todo items and the actual commits. This makes it much clearer where the current head commit is. Get rid of the "<-- YOU ARE HERE" marker that we would previously show; it is no longer needed. (We still show the "<-- CONFLICT" marker though.)

For example:

╭─[4]─Commits - Reflog───────────────────────────╮
│--- Pending rebase todos ---                    │
│6562f903 pick  CI commit 03                     │
│--- Commits ---                                 │
│56a04448       CI ◯ commit 02                   │
│6d0c0e41       CI ◯ commit 01                   │
  • Please check if the PR fulfills these requirements
  • Cheatsheets are up-to-date (run go generate ./...)
  • Code has been formatted (see here)
  • Tests have been added/updated (see here for the integration test guide)
  • Text is internationalised (see here)
  • If a new UserConfig entry was added, make sure it can be hot-reloaded (see here)
  • Docs have been updated if necessary
  • You've read through your own file changes for silly mistakes etc

@stefanhaller stefanhaller added the enhancement New feature or request label Apr 5, 2025
@stefanhaller
Copy link
Collaborator Author

Marking as a draft because two integration tests are failing.

Copy link

codacy-production bot commented Apr 6, 2025

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
+0.03% 98.21%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (c711aa4) 54918 47670 86.80%
Head commit (027f33c) 55072 (+154) 47819 (+149) 86.83% (+0.03%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#4463) 223 219 98.21%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

@stefanhaller
Copy link
Collaborator Author

I added a fix for the failing tests; we can discuss if the fix is good enough. Opening for review.

@stefanhaller stefanhaller marked this pull request as ready for review April 6, 2025 08:15
@ruudk
Copy link
Contributor

ruudk commented Apr 6, 2025

Nice, could we also use these markers to indicatie where a branch branched of the default branch (main)? That's something I miss in Lazygit.

@stefanhaller
Copy link
Collaborator Author

Nice, could we also use these markers to indicatie where a branch branched of the default branch (main)? That's something I miss in Lazygit.

You can tell by the colors of the hashes; main branch hashes are green, the ones in your branch are either yellow or red, depending on whether they are pushed. I do realize that 10% of the population is color blind in some way, but I feel that section headers would be a bit too strong in this case.

@stefanhaller stefanhaller force-pushed the section-headers-for-rebase-todos-and-actual-commits branch from 3a84492 to 25fa371 Compare April 6, 2025 11:55
@stefanhaller
Copy link
Collaborator Author

I found a better fix for the failing integration tests (1c107fd), so I dropped the previous band-aid fix (for reference: it was 3a84492).

@stefanhaller stefanhaller force-pushed the revert-range-selection-of-commits branch from 9a5e245 to 8aaacef Compare April 7, 2025 12:43
@stefanhaller stefanhaller force-pushed the section-headers-for-rebase-todos-and-actual-commits branch from 25fa371 to d1b8318 Compare April 7, 2025 12:43
…isn't focused

When rerendering a view at the end of a refresh, we call HandleFocus only if the
view has the focus. This is so that we rerender the main view for the new
selection.

What was missing here is to update the view selection from the list selection if
the view doesn't have the focus, so that the selection is painted properly.
Normally this is not relevant because you don't see the selection if another
side panel has the focus; however, you do see it as an inactive selection when
e.g. a popup is shown, in which case it does matter.

This will become more important when we introduce section headers for commits,
because in that case the view selection needs to change when the working copy
state changes from normal to rebasing or vice versa, even if the list selection
stays the same.

The changed test submodule/reset.go shows how this was wrong before: when
entering the submodule again after resetting, there is a refresh which keeps the
same branch selected as before (master); however, since the branches panel is
not focused, the view didn't notice and kept thinking that the detached head is
selected (which it isn't, you can tell by running the test in sandbox mode and
focusing the branches panel at the end: you'll see that master is selected). So
the change in this commit fixes that.
This is needed because we want to show different section headers for rebase
todos and cherry-pick/revert todos.
Now that we have sections, it is no longer needed.

Keep the "<-- CONFLICTS" marker though.
@stefanhaller stefanhaller force-pushed the revert-range-selection-of-commits branch from 8aaacef to c711aa4 Compare April 10, 2025 09:53
@stefanhaller stefanhaller force-pushed the section-headers-for-rebase-todos-and-actual-commits branch from d1b8318 to 027f33c Compare April 10, 2025 09:54
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants