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

Rebase --onto: Display only the current branch to make selection easier #9686

Merged

Conversation

pmiossec
Copy link
Member

Proposed changes

Because a commit should be selected in the history of the current branch, display only this current branch to make the selection easier.

Screenshots

Before

image

After

image

Test methodology

  • Manual

Test environment(s)

  • Git Extensions 33.33.33
  • Build 9f8c00c (Dirty)
  • Git 2.28.0.windows.1 (recommended: 2.30.0 or later)
  • Microsoft Windows NT 10.0.19042.0
  • .NET 5.0.11
  • DPI 192dpi (200% scaling)

Merge strategy

I agree that the maintainer squash merge this PR (if the commit message is clear).


✒️ I contribute this code under The Developer Certificate of Origin.

@ghost ghost assigned pmiossec Oct 25, 2021
Copy link
Member

@gerhardol gerhardol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1
(I had to view the code to realize this functionality, this could have been described with a screenshot...)

@pmiossec
Copy link
Member Author

(I had to view the code to realize this functionality, this could have been described with a screenshot...)

I don't understand, there are 2 screenshots...

@gerhardol
Copy link
Member

I don't understand, there are 2 screenshots...

image

Copy link
Member

@RussKie RussKie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please describe the flow to understand the context of this change better?

Comment on lines 340 to 365
var previousValueBranchFilterEnabled = AppSettings.BranchFilterEnabled;
var previousValueShowCurrentBranchOnly = AppSettings.ShowCurrentBranchOnly;
var previousValueShowReflogReferences = AppSettings.ShowReflogReferences;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use explicit types for intrinsic data types

Suggested change
var previousValueBranchFilterEnabled = AppSettings.BranchFilterEnabled;
var previousValueShowCurrentBranchOnly = AppSettings.ShowCurrentBranchOnly;
var previousValueShowReflogReferences = AppSettings.ShowReflogReferences;
bool previousValueBranchFilterEnabled = AppSettings.BranchFilterEnabled;
bool previousValueShowCurrentBranchOnly = AppSettings.ShowCurrentBranchOnly;
bool previousValueShowReflogReferences = AppSettings.ShowReflogReferences;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest this is changed, then I will approve and I am supposedly controlling the merge

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

@RussKie RussKie added 📭 needs: author feedback More info/confirmation awaited from OP; issues typically get closed after 30 days of inactivity hacktoberfest-accepted labels Oct 26, 2021
@gerhardol
Copy link
Member

Can you please describe the flow to understand the context of this change better?

That was what I struggled with as well - I never use the feature this way.
The onto in the subject throw me off too.
This is only about the commits to include in the rebase.

I normally always rebase interactively, removing the commits I do not want.
This is tweaking the Git internal handling allowing Git to only include a range of commits.
Of course, specify something else than in the current branch to rebase is not possible.
(I guess the first commit to list could be set too.)

@pmiossec
Copy link
Member Author

Sorry, I was not aware that you couldn't know it.
I use it really often so it was kind of normal to call it "Rebase onto" because that's in GitExtensions the way we use the git rebase --onto feature. where you are allowed to define the scope of the commits that will be rebased somewhere else.

Here is a gif showing the feature:
rebase_onto

This is only about the commits to include in the rebase.

Yes, with a normal rebase, commits to rebase are defined starting from the common ancestor commit between the tip of the branch you want to rebase and the commit where you want to rebase.

The git rebase --onto feature allows you to define a subset of these commit by selecting a commit of the branch you are rebasing between the common ancestor and the tip of the branch you rebase.

So, displaying other commits than the one of the branch your rebase makes no sense.
Thus, this is the goal of this PR to remove the unneeded visual noise by deplaying only the history of the current branch.

In this example:

Before                                    After
A---B---C---F---G (branch)                A---B---C---F---G (branch)
         \                                             \
          D---E---H---I (HEAD my-branch)                E'---H'---I' (HEAD my-branch)

D is the commit that we will select to exclude it (and also previous ones if there were more) to only rebase the subset of commits: E, H and I.

I normally always rebase interactively, removing the commits I do not want.

Yes, I know that very often propose you different ways to achieve the same result but this feature is quicker and in some rare cases make commit selection much more easier to do ;)

@ghost ghost removed the 📭 needs: author feedback More info/confirmation awaited from OP; issues typically get closed after 30 days of inactivity label Oct 26, 2021
@pmiossec pmiossec changed the title Rebase onto: Display only the current branch to make selection easier Rebase --onto: Display only the current branch to make selection easier Oct 26, 2021
@pmiossec pmiossec force-pushed the better_rebase_onto_selected_commit branch from 3fa48a2 to a016bc4 Compare October 27, 2021 07:46
@RussKie
Copy link
Member

RussKie commented Oct 27, 2021

@msftbot merge if @gerhardol approves

@ghost ghost added the status: auto merge label Oct 27, 2021
@ghost
Copy link

ghost commented Oct 27, 2021

Hello @RussKie!

Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:

  • I'll only merge this pull request if it's approved by @gerhardol

If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you".

@pmiossec pmiossec force-pushed the better_rebase_onto_selected_commit branch from a016bc4 to 2e0b3b7 Compare October 30, 2021 05:34
@gerhardol gerhardol merged commit 478fb1e into gitextensions:master Oct 30, 2021
@ghost ghost added this to the vNext milestone Oct 30, 2021
@pmiossec pmiossec deleted the better_rebase_onto_selected_commit branch November 7, 2021 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants