feature blame popup

[tablatronix]

Description

optional, button in popup to goto blame, or inline blame , or view on github.

https://github.com/repo/repo/blame/branch/file.ext#L385

[deathaxe]

How about the Git blame package? It allows to show the blame of each line via phantoms.

Nevertheless adding the inline blame to the diff popup might be a useful feature.

[tablatronix]

Ill check that out, I am assuming it will not cross branch compare so it will blame the working branch, which is not as helpful though.

[deathaxe]

Not sure what you expect but as you always see the content of the working tree's file in your editor git blame always shows the most recent commit that changed the file. I found arguments to specify the amount of time to go back looking for changes but not a certain branch or path of branches to compare against.

A blame popup in GitGutter would just show the result of git blame -Lx,y -- <filename>.

The issue with creating a popup for it is the question how to trigger the popup? Showing the popup when hovering the gutter area of a line will most likely result in the popup fighting against other plugins. The currently used methods to avoid it (protected_regions) would not work then as the blame popup would occupy each line which git blame returns a value for.

[tablatronix]

Yeah thats the thing, I was manually comparing across branches, then blaming the compare against file:line to get the commit of that change ( usually latest was good enough ), then looking at that entire commit with all the change in context, and decide if I could cherry pick it. So even getting just the commit for that line in the compare might be good enough.

I started thinking, I could do alot more I could just get the commit of this change right in front of me.

Is there a feature discussion on adding callouts? user callouts might be cool with tokens for %filename %line etc.

I would not implement this entire feature inline, or use another plugin, I would just add a blame button to the popup itself and have it either show in the popup the info, or always blame and show it in the popup at the bottom footer style or something.

( better if it could link to github, but that is a use case specific request, other svn users would not benefit )

Just the output of git blame, is good, but why would it need to be another popup ??

[r-stein]

The problem with integrating git blame into the popup is that the popup only appears on changed lines. Hence you would change the line to enable the popup and then press the git blame button to open the blame. This doesn't sound like a reasonable behavior.

[tablatronix]

Still not sure I see the problem, what do you mean "change the line" ?

The gutter already shows the change, the popup already exists, I want the blame on the compare against source , not the current file.

[r-stein]

Let's say you want to blame on line 34, but you didn't change the line. Then the git gutter popup won't appear on the line. Hence you need to change it to enable the popup and press the button.

[tablatronix]

I don't want to blame line 34. I want to blame the line that shows a change, and blame the other line... If that makes any sense.

I guess ill make a gif or mockup

I guess this only makes sense on changes, not deletions or insertions

[r-stein]

If you only want to blame lines you have changed the button makes sense, but git blame is usually not only used on changed lines.

[tablatronix]

ah, Ok I should qualify that then

Blame the current line/change in the diff source, to get the commit of that diff in the source, source being the compare against source, which may depend on context of source (branch, commit, origin) for blame capability, but most should work.

[tablatronix]

Not sure this is even possible, now that I think about it... Git blame probably only works on working tree..

doh,

EDIT , you can!

[deathaxe]

Do I understand correct?

You want to see the blame output of the content which is displayed within diff popup? So you can see who changed the line within the committed file you are currently comparing against?

[tablatronix]

Yeah I think that sounds about right.

This is just an example

[deathaxe]

If I read the git blame documentation correctly, specifying a branch name like "esp32" in your case just limits the range git blame walks back in history to look for changes. It does not specify the "esp32" to be the commit to take the source file to blame from.

With other words: Blame works for the working file only.

So what's possible is to call git blame against the changed lines of the working file (which are highlighted in the gutter area) to reveal the most recent commit the changes were introduced in. Your compare target might point to a commit which is much older and therefore changes are displayed which belong to different commits.

From this point of view some kind of "status line" at the bottom of the popup with the author, date and commit of that change might make sense. As a result a button could be added to set the compare target exactly to that commit.

[tablatronix]

hmm, yeah I was trying to figure that out also, was thinking maybe it used work tree when <rev> was provided..

[jbrooksuk]

It could also be done the way GitLens does it in VSCode, so we'd use Phantoms.

[Spaxe]

The automatic showing at the end of line is very distracting. I've turned it off in my preferences (thanks for adding a toggle!)

{
  "show_line_annotation": "false"
}

The same information of git blame is repeated in the status bar anyway, so this is not a loss.

[tablatronix]

I agree default on popup is a bit annoying, it would be better if it was inside the actual gitgutter dialog popups also.

EDIT, ooh line annotation supports templates! that IS cool.

[tablatronix]

I would make 2 changes

well 3

• [ ] disable line annotation by default in stable releases
• [ ] add annotation to diff popup option ( somewhere in the popup, titlebar, or a footer maybe ), ( so it can be hidden always unless you inquire in popup, less obtrusive )
• [ ] only show line annotation on diffs ( if gitgutter icon present based on compare against )

and maybe

• [ ] custom scope class for the annotation (style color )

[deathaxe]

1. Disabling the annotation by default is considerable but will most likely cause nobody to realize that feature. It is enabled in GitLens for VS Code by default as well. There are thousands of settings all over ST and its plugins to customize nearly everything. IMHO the effort to disable that feature if someone really doesn't like it is acceptable.

2. Displaying the blame info in the diff popup for changed lines wouldn't make sense as you'd only see You (just now) if compared against HEAD. It might make sense if comparing against older commits/branches/... if the changed line was not modified in the working tree / editor. Furthermore the blame information is not related with the diff. It is an information worth to be displayed for any line, which makes the usage of a popup for that purpose hard without conflicting with other plugins occupying the gutter.

3. Makes no sense as described in 2.

4. The scope for is comment.line.annotation.git_gutter in order to default to comment style. The line_annotation_scope setting was finally removed as it would result in requests to add custom diff scope settings as well. With that said a syntax definition or plugin defines a scope, which is a key do be addressed by color schemes to provide suitable colors for. This is how ST works. Any option to define fixed styles would work well only with one color scheme.

Basically the phantom being used for line annotation is just a peace of HTML with the id gitgutter-line-annotation which a color scheme could even provide a custom embedded CSS for.

I also thought about making use of mdpopups to render the phantom, which would have enabled us to use the diff_popup.css, but in the end I found it all too bloated in order to just provide the ability to define a fixed color, which wouldn't match any color scheme well anyway.

[tablatronix]

I know the diff is not related to the blame, but arguing for no. 2 or no. 3 serves more as a filter to make it less noisy when you want to see if for something specific, I guess that was my point, and also my original intent, inspect blame for a diff, not all.

I agree with 1. it is often a good way to expose large new features for people who do not read release notes to just force them on. But if it leads to endless support questions it might be solvable by docs or changing the default, etc.

[alexlouden]

+1 on disabling line annotation by default. I discovered it today, didn't find it useful at all, so spent 20 minutes figuring out which package it came from. Happened upon this post, and this comment which already has 10 votes. I'm guessing there's a big % of users who won't bother finding the feature flag and will just disable the package?

I think it's okay for new features to be enabled by default for discovery, but you need to be receptive to user feedback too. Thanks, really appreciate all your work on GitGutter!

[deathaxe]

User feedback is respected, but with 400..600 downloads per day 10 votes at all are just a very minor amount. Unfortunately people only open issues or vote if the dislike something. Searching ST forum or other sources might also reveal as much posts of people liking the feature. So the current number of votes is just not enough to disable it by default.

[tablatronix]

notes Line annotation interferes with mouse selection(if cursor is over its indented whitespace or anywhere neat the annotation ), and the text also gets included in keyboard selection ( highlighted only, not copied or pasted )

[cjreeve]

Although I agree the git blame line annotation option is a little distracting, it is very useful for debugging and is worth keeping on for me. I wasted a bit of time working out how to enable it again since upgrading ST3. I had not realised it was part of GitGutter :). I think it should be enabled again by default otherwise people may not discover this useful feature.

[tablatronix]

Does anyone know how to debug this ? Every file in a project I have says the same wrong commit for line blame.

[deathaxe]

GitGutter won't receive any further blame related feature beyond the current line annotation. Blame is a task better done via Sublime Merge.