Blame previous revision without function

[mstv]

Current behaviour

try to show the blame for a previous revision
nothing happens

Expected behaviour

blame of previous revision is shown

Steps to reproduce

• open blame for GitUI/CommandsDialogs/SubmodulesDialog/FormAddSubmodule.cs
• goto line 75
• chose Blame previous revision from the context menu

Screenshots

Did this work in previous version of GitExtensions

at least not in 3.00.00.4433

Environment

• Git Extensions 3.1.0.5877
• Build bb1687f
• Git 2.21.0.windows.1
• Microsoft Windows NT 10.0.17763.0
• .NET Framework 4.7.3362.0
• DPI 96dpi (no scaling)

[pmiossec]

I had to blame absolutely a file this friday so I played a little with it and I think I understood what it is doing (and that I find strange)...

I propose you to test this version: https://github.com/pmiossec/gitextensions/tree/improve_blame

Now, do the same thing and it will do something 😉
In the version you have, it is doing nothing because you don't have all the history ( always this problem of rename -- that is quick fixed in my branch--).

And that's where I personnaly find that it is doing a strange thing.
I would expect that this feature select the first commit in the list before the one introducing the lines.
In fact, it tries to select the commit introducing the lines before the one introduced by the commit. In your case a commit of 2009, not in the commit listed by the broken actual version.

For me the behavior is not the one expected because:

• it does not match to the label of the feature
• I don't understand the logic that make GE to go to the commit of the first line replaced even if that replace 3 lines from different commits.

I think the selected commit should be either the one in the list just before the one corresponding to this lines (easier to do and to use, I think).
Or the newest among all the lines replaced (but that's hard to determine).

[pmiossec]

nothing happens

To be clearer, I think that in your version, it is looking in the list for a commit that is not present, so it does nothing...

[mstv]

I would expect that this feature selects the first commit in the list before the one introducing the lines.

Exactly! Thank you for the analysis.

Regarding your commit 095d915 "Blame: Display avatars":

• The height of the avatars seems to be less than the texts's.
• The avatars should be displayed only if they are enabled for the revision grid.

my settings:

[pmiossec]

Thanks for the quick review.

The height of the avatars seems to be less than the texts's.

That's a big blocking problem. I spent a lot of time trying to solve it, without success 😕 I didn't find a way to get the real height of a line which seems not to be an integer value... If I get the height from the font, the images are slightly too small and if I add 1 pixel, they are too big! So, I don't know how to be pixel perfect.

…

[mstv]

I would try to access TextView.FontHeight etc. or if these properties cannot be accessed easily (because it is a submodule), the same calculation could be used (it works!):

        private static int GetFontHeight(Font font)
        {
            var max = Math.Max(
                TextRenderer.MeasureText("_", font).Height,
                (int)Math.Ceiling(font.GetHeight()));

            return max + 1;
        }

[RussKie]

Do we really want avatars in the blame window?

[gerhardol]

Avatars use less space than names so probably better

[mstv]

WIP: try to solve "Blame to previous revision"

🎉 👍 In a short test, this seems to work as I expect it.

[RussKie]

Do we need to patch 3.1.1 with the fix?

[pmiossec]

Do we need to patch 3.1.1 with the fix?

No, for 2 reasons...

That is a not a bug but a strange behavior, there from the beginning, so for more than 6 years.

That is a quick try and it is far from being finished.
only the normal case is managed and commits with no parents (easy) or more than one (I don't know how to handle it at the moment) are not handled.

Also trying to display the blame at the good line (like done before) is not still done.

[RussKie]

Thank you 👍

…

On Thu, May 30, 2019, 6:14 PM Philippe Miossec ***@***.***> wrote: Do we need to patch 3.1.1 with the fix? No, for 2 reasons... That is a not a bug but a strange behavior, there from the beginning, so for more than 6 years. That is a quick try and it is far from being finished. only the normal case is managed and commits with no parents (easy) or more than one (I don't know how to handle it at the moment) are not handled. Also trying to display the blame at the good line (like done before) is not still done. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#6605?email_source=notifications&email_token=ABBTEXVT6V4XOTILJWMK74TPX6EETA5CNFSM4HN5LESKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWRV7WQ#issuecomment-497246170>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABBTEXROFIVNLRZ2KWOELI3PX6EETANCNFSM4HN5LESA> .

[mstv]

only the normal case is managed and commits with no parents (easy) or more than one (I don't know how to handle it at the moment) are not handled.

I suggest to use the first parent. IMO each choice is correct. One can see in the revision tree that it is a parent of a merge. If one wants to see the other(s), it's only a click away.

(Hint: Don't try to rely too much on the sequential order of the parent branches. Henk Westhuis has prepared some improved variants of the revision graph #5782, #5783, unfortunately unfinished yet.)

[pmiossec]

I suggest to use the first parent.

That's what I already did as a quick fix.
But,you're right, it's probably enough as a first step.

Don't try to rely too much on the sequential order of the parent branches.

I don't 😁

There is still the fact to better select the good lines to display to do...