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
Fixup GetAllBranchesWhichContainGivenCommit #10836
Fixup GetAllBranchesWhichContainGivenCommit #10836
Conversation
@@ -3141,12 +3141,12 @@ public IReadOnlyList<string> GetAllBranchesWhichContainGivenCommit(ObjectId obje | |||
return Array.Empty<string>(); | |||
} | |||
|
|||
var result = exec.StandardOutput.Split(new[] { '\r', '\n', '*', '+' }, StringSplitOptions.RemoveEmptyEntries); | |||
string[] result = exec.StandardOutput.Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good to add a comment explaining what information git may provide and what transformation we do.
IIRC, *
denotes the current branch, but I can't remember what +
may be used for...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@RussKie that would be a link to Git documentation
Documentation is poor, does not statate that the line starts with two chars before branch
So something should be added below (this line is fine).
https://git-scm.com/docs/git-branch#_description
+
is used for linked work trees, similar to *
@@ -3141,12 +3141,12 @@ public IReadOnlyList<string> GetAllBranchesWhichContainGivenCommit(ObjectId obje | |||
return Array.Empty<string>(); | |||
} | |||
|
|||
var result = exec.StandardOutput.Split(new[] { '\r', '\n', '*', '+' }, StringSplitOptions.RemoveEmptyEntries); | |||
string[] result = exec.StandardOutput.Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@RussKie that would be a link to Git documentation
Documentation is poor, does not statate that the line starts with two chars before branch
So something should be added below (this line is fine).
https://git-scm.com/docs/git-branch#_description
+
is used for linked work trees, similar to *
Fixes #10819
Proposed changes
Screenshots
n/a
Test methodology
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.