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.

Sign up for GitHub

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

Jump to bottom

Interactive rebase: Highlight 4 byte sha #8648

Merged
merged 2 commits into from
Nov 30, 2020
Merged

Interactive rebase: Highlight 4 byte sha #8648

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
68a4c79
Interactive rebase: Highlight 4 byte sha
gerhardol Oct 31, 2020
067e22d
Gitmodule compiled
gerhardol Oct 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion GitCommands/Git/GitModule.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3017,7 +3017,7 @@ public IReadOnlyList<IGitRef> ParseRefs([NotNull] string refList)
//
// Lines may also use \t as a column delimiter, such as output of "ls-remote --heads origin".

var regex = new Regex(@"^(?<objectid>[0-9a-f]{40})[ \t](?<refname>.+)$", RegexOptions.Multiline);
var regex = new Regex(@"^(?<objectid>[0-9a-f]{40})[ \t](?<refname>.+)$", RegexOptions.Multiline | RegexOptions.Compiled);
Copy link
Member

Choose a reason for hiding this comment

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

If compiling the Regex, it should ideally be cached so that it's not compiled again the next time refList is called.

Copy link
Member

Choose a reason for hiding this comment

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

Regex compilation is fairly slow. It does give slightly faster run time, but the main benefit is a reduction in memory allocation while matching, in my experience. If it's being recompiled each time then my hunch is the overhead likely outweighs the benefit.

Copy link
Member Author

Choose a reason for hiding this comment

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

I did not try to measure (I expect it to be hard to see the difference), but this situation seem to be parsing a larger amount of data and I assume it to be faster.

In most other situations where Compiled is used, it should be changed.


var matches = regex.Matches(refList);

Expand Down
4 changes: 2 additions & 2 deletions GitUI/Editor/RebaseTodoHighlightingStrategy.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ private static bool TryHighlightInteractiveRebaseCommand(IDocument document, Lin
{
var idLength = index - idStartIndex;

if (idLength <= 5)
if (idLength < 4)
Copy link
Member

Choose a reason for hiding this comment

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

This will highlight a 3 byte SHA too. Is that intentional?

Copy link
Member Author

Choose a reason for hiding this comment

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

The minimum length is 4 so 3 should have been excluded. But it makes not much difference, should be changed if updating again

Copy link
Member Author

Choose a reason for hiding this comment

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

This is correct, a sha with 4 chars is highlighted, 3 is not

{
return false;
}
Expand Down Expand Up @@ -159,4 +159,4 @@ private static bool TryHighlightInteractiveRebaseCommand(IDocument document, Lin
bool IsHexChar() => char.IsDigit(c) || (c >= 'a' && c <= 'f');
}
}
}
}