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

WIP Improve branch detection for merge message without "into" #6237

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mstv
Copy link
Member

@mstv mstv commented Feb 10, 2019

Fixes #6236

Proposed changes

  • try to get more info if merge commit lacks "into"
  • use Environment.NewLine instead of `\n'

Screenshots

Before

before

After

after

Test methodology

  • TODO: NUnit test cases

Test environment(s)

  • Git Extensions 3.1.0
  • Build 5c41338 (Dirty)
  • Git 2.20.1.windows.1
  • Microsoft Windows NT 6.1.7601 Service Pack 1
  • .NET Framework 4.7.2117.0
  • DPI 96dpi (no scaling)

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

@mstv
Copy link
Member Author

mstv commented Feb 10, 2019

The sort order of child nodes does not match the requirements for this feature. I would expect the master branch to be the first child, i.e. node.Children.Last() (.Last() because the collection is a stack: ImmutableStack<RevisionGraphRevision>).

Can I rely on the sorting of Parents only, @spdr870? I.e. the first parent is the branch merged into.

bad

Here, it works as expected:

good

@codecov
Copy link

codecov bot commented Feb 10, 2019

Codecov Report

Merging #6237 into master will increase coverage by 0.35%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #6237      +/-   ##
==========================================
+ Coverage   45.44%   45.79%   +0.35%     
==========================================
  Files         669      669              
  Lines       50383    50396      +13     
  Branches     6650     6645       -5     
==========================================
+ Hits        22897    23080     +183     
+ Misses      26275    26040     -235     
- Partials     1211     1276      +65
Flag Coverage Δ
#production 35.32% <0%> (+0.44%) ⬆️
#tests 97.6% <ø> (-0.01%) ⬇️

@mstv mstv force-pushed the feature/6236_branch_tooltip_if_not_really_master branch from 12bfd4e to 2607383 Compare March 3, 2019 22:40
@mstv
Copy link
Member Author

mstv commented Mar 3, 2019

This works much more like expected - though still not completely:
grafik

Do you have objections against this approach, @spdr870, @RussKie, others?

@spdr870: How can I get a really sorted list of segments?

public RevisionGraphRevision GetLeftmostChild(RevisionGraphRevision node)
{
    int rowIndex = _revisionGraphRowProvider.GetRowForNode(node);
    var segments = _revisionGraphRowProvider.GetSegmentsForRow(rowIndex);
    return segments?.Segments?.FirstOrDefault(segment => segment.Parent == node)?.Child;
}

@GintasS
Copy link
Collaborator

GintasS commented Aug 6, 2021

This is an old PR and should be closed or set to a draft. @gerhardol

@mstv mstv marked this pull request as draft August 6, 2021 19:20
@ghost ghost assigned mstv Aug 6, 2021
@mstv
Copy link
Member Author

mstv commented Aug 6, 2021

I must follow up here. The detection does not work well - especially with squash-merges.

@GintasS, "WIP" is the former marker for "draft".

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.

Improve branch detection for merge message without "into"
3 participants