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

Select multiple revisions using Left Panel #8056

Merged
4 commits merged into from May 3, 2020

Conversation

mstv
Copy link
Member

@mstv mstv commented May 1, 2020

Proposed changes

Improve revision selection Left Panel vs. RevisionGrid:

  • Minor fixup in NativeTreeViewExplorerNavigationDecorator:
    Use the given lambda instead of DateTime.Now.
  • Reset RevisionGridControl.InitialObjectId after use. (Might be related to Refreshing is changing selected commit #6057.)
  • Fix up the check for the last visible row in RevisionGridControl.SetSelectedIndex:
    • factor out a new local function EnsureRowVisible
    • call _gridView.Select() first (was omitted by too early return from my previous PR)
  • Toggle the RevisionGrid selection if Ctrl is pressed especially if triggered from left panel.

Screenshots

Before

result of Ctrl+Click

grafik

After

grafik

Test methodology

  • manual

Test environment(s)

  • Git Extensions 3.3.0
  • Build e3ee536
  • Git 2.26.0.windows.1
  • Microsoft Windows NT 6.2.9200.0
  • .NET Framework 4.8.4121.0
  • DPI 96dpi (no scaling)

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

@codecov
Copy link

codecov bot commented May 1, 2020

Codecov Report

Merging #8056 into master will increase coverage by 0.01%.
The diff coverage is 67.64%.

@@            Coverage Diff             @@
##           master    #8056      +/-   ##
==========================================
+ Coverage   49.54%   49.56%   +0.01%     
==========================================
  Files         846      846              
  Lines       60878    60895      +17     
  Branches    10915    10918       +3     
==========================================
+ Hits        30164    30182      +18     
- Misses      28488    28493       +5     
+ Partials     2226     2220       -6     
Flag Coverage Δ
#production 37.37% <67.64%> (+0.02%) ⬆️
#tests 94.87% <ø> (+<0.01%) ⬆️

@mstv mstv force-pushed the feature/rot_multiselect branch from e3ee536 to c267614 Compare May 2, 2020 06:37
@gerhardol
Copy link
Member

Implements part of #6180

Great feature, simplifies selecting branches to compare.
(To review it, I will have to dig deeper into the revision grid, but the changes are well motivated. Especially the #6057 related change is interesting.)

@mstv mstv force-pushed the feature/rot_multiselect branch from c267614 to 03da69a Compare May 3, 2020 13:02
mstv added 4 commits May 3, 2020 15:05
- factor out local function EnsureRowVisible
- call _gridView.Select() first (was omitted by too early return)
especially if triggered from left panel
@mstv mstv force-pushed the feature/rot_multiselect branch from 03da69a to 927b317 Compare May 3, 2020 13:07
@mstv
Copy link
Member Author

mstv commented May 3, 2020

@msftbot merge in 8 hours

@ghost ghost added the status: auto merge label May 3, 2020
@ghost
Copy link

ghost commented May 3, 2020

Hello @mstv!

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 won't merge this pull request until after the UTC date Sun, 03 May 2020 22:00:22 GMT, which is in 8 hours

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".

@RussKie RussKie closed this May 3, 2020
@RussKie RussKie reopened this May 3, 2020
@gitextensions gitextensions deleted a comment May 3, 2020
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.

Seem and works OK for me, No formal review, but supporting merging

@ghost ghost merged commit 68c368f into gitextensions:master May 3, 2020
@ghost ghost added this to the 3.4 milestone May 3, 2020
@gerhardol
Copy link
Member

927b317 causes a regression
If Ctrl-P/N is used to move around in the grid, the existing selected commit is retained

@mstv
Copy link
Member Author

mstv commented May 4, 2020

If Ctrl-P/N is used to move around in the grid, the existing selected commit is retained

Oops, fixing.

This pull request was closed.
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