RFC: Add 'blame' feature

[lambdalisue]

I added 'blame' feature. I would like to hear comments before I settle the behaviors.

Syntax:

:Gina blame [+cmd] [++opt] [{options}] [{treeish}]

Examples:

:Gina blame :README.md
:Gina blame :autoload/gina/process.c -L10,20 -L26,30

Pros/Cons to vim-gita:

• Pros. Incredibely faster (tested on vim/vim/src/eval.c)
  • vim-gita: approx. 14 s
  • gina.vim: approx. 5 s
• Pros. Better option supports
  • E.g. Annotation limit (-L) was not supported in vim-gita
• Cons. Less visibility
  • vim-gita use pseudo lines to separate each chunks which makes easy to recognize a chunk region
  • gina.vim does not separate chunks for performance reason

Pros/Cons to vim-fugitive:

• Pros. Good looks
  • vim-fugitive shows a raw format
• Pros. Good navigation
  • to go the commit or the parent commit
  • to back
  • j/k to show detail
• Cons. A bit slower (tested on vim/vim/src/eval.c)
  • gina.vim: approx. 5 s
  • fugitive: approx. 3 s

[bimlas]

Very cool! Finally, I can drop Fugitive. :smile:

My opinion:

• Apply J/K map for Up/Down and <CR> to <C-J> too (alternatives of each other)
• An idea to map all the H/L & Left/Right to navigate in the blame (go back/go to the commit)
• Move user $hash line from the end of the hunk to the beginning - this is displayed for shorter hunks, additional contents (like commit message) should go below this, the date would be on the last line to mark the end of the hunk
• Instead of date (on 23 Dec, 2008) it displays (Invalid) (note that date format in my country is YYYY-MM-DD)
• In an earlier stage there was highlighting to separate the hunks visually - where did it go?

[prabirshrestha]

+1 for this feature.

Some suggestions.

• :Gina blame should work. Currently I need to always specificy the file with :Gina blame :%
• I would prefer the commit hash to be on start of the line instead of after the user name. I'm most likely to copy the hash than the user name.
• Might be have some option to see raw git style blame.

[lambdalisue]

1. Up/Down: Sure. I will
2. C-j: I know it is CR in terminal but it is not equal to CR in GUI so I think I shoul leave it and users can customize
3. H/L: Maybe not. I need to confirm if it's conflict with native mapping but I feel user can customize so it is a bit overkill
4. user/hash line: Make sence. I'll move it
5. Highlight: It was really slow on large file. I would like to highlight if there is a good way
6. Gina blame: That conflict with other command syntax . By the way, you can use Gina blame : instead (% is not necessary)
7. Hash/User: Sure. I will
8. Raw: No while it requires additional parse mech to create action candidates

[lambdalisue]

9. date: I'll check and I'll make it configureble

[lambdalisue]

@bimlas

I made it configurable. Could you check what kind of format should become a default while strftime depens on the platform so I've no idea what kind of format is available on Windows?

https://github.com/lambdalisue/gina.vim/pull/45/commits/fb2b66ffb8050969a66b8f8fbbcc5801df415086

let g:gina#command#blame#timestamper#format1 = '...'
let g:gina#command#blame#timestamper#format2 = '...'

[bimlas]

Date works as expected without changing anything.

I think the "default" is depends on language (locale?) instead of operating system.

[lambdalisue]

I ment "%Y" or so on. I don't know what atoms can be used for all

[mhartington]

Hi there! Just to leave my opinion.

I actually prefer the format that comes from fugitive

In this case, even though its just the raw output, it's tends to be easier to read and figure out than what gina is producing.

[bimlas]

@lambdalisue: Well, strftime('%d %b, %Y') producing 27 febr., 2017 so the format is the same as for Linux, but it prints the month in my language: for example echo strftime('%d %b, %Y', 1493669754) prints 01 máj., 2017 where máj. stands for május (May in Hungarian).

[lambdalisue]

@mhartington Why gina-blame window has line numbers? With setlocal number, the chunk is wrapped and difficult to distinguish the chunk.

@bimlas

Instead of date (on 23 Dec, 2008) it displays (Invalid) (note that date format in my country is YYYY-MM-DD)

Hum...? Then what did you mean with that?

[lambdalisue]

Note I pushed by -f

[lambdalisue]

Hum... The current interface has an issue for a large chunk.

I'll think about this but I may change the current interface to completely different one. If you have any idea, I'm welcome to hear.

[bimlas]

I was wrong: not always prints (Invalid), but when date is "absolute" (28 febr., 2017), XY ago works. Tried out on Linux where both form of dates are correct. I will try to fix it (you simply cannot test I think).

Found another issues when blaming this file:

• Line 43 is not shown in sidebar, but echoing the commit message
• Sidebar/echo seems to be "disconnected": SHAs in sidebar are absolutely different from echoed messages (line 13)

• Sidebar is shorter then content: content is 62 lines while sidebar is 42

Tested on Windows and Linux too!

[lambdalisue]

Wait, I may change the interface so do not try to fix for now

[lambdalisue]

I got another issue. I'll fix it before release :+1:

[bimlas]

It seems to be some kind of index-related bug because echoed message is changing in different line then sidebar. Example (on the file above, signing the lines where echoed message is changing):

87a555af          @BimbaLaszlo <- echoing [gina] uj telepitesi rendszer...
uj telepitesi rendszer        
4f594462          @BimbaLaszlo <- echoing [gina] update
update                        
|
|
|
|
|
|                 on (Invalid)
697c920c          @BimbaLaszlo
bin: Fix missing functions    
87a555af          @BimbaLaszlo <- echoing [gina] bin: Fix missing functions WHICH IS 2 LINES ABOVE
uj telepitesi rendszer        
|                 on (Invalid) <- [gina] uj telepitesi rendszer
ba90ea35          @BimbaLaszlo
!vim: Move minimal (debugging 
) settings to a standalone rc 
|                 on (Invalid)
87a555af          @BimbaLaszlo <- [gina] !vim: Move minimal (debugging) settings to a standalone rc 
uj telepitesi rendszer        
|
|
|
|
|                              <- [gina] uj telepitesi rendszer
|
|
|                 on (Invalid)
f85b86f7          @BimbaLaszlo
git: Rename '.git_template/'  
to '.gitconfig_files/'        
|                 on (Invalid)
223030d8          @BimbaLaszlo
f85b86f7          @BimbaLaszlo
git: Rename '.git_template/?/'
f85b86f7          @BimbaLaszlo
git: Rename '.git_template/'   <- [gina] git: Rename '.git_template/' to '.gitconfig_files/' SHOULD BE 8 LINES ABOVE
to '.gitconfig_files/'        
|
|
|
|
|
...

[bimlas]

If you have any idea, I'm welcome to hear.

I think the raw git blame output (like Fugitive's blame) + echoing the commit message is OK, maybe colorize the SHAs (same SHA = same color), the currently blamed SHA could be reversed (different background color).

EDIT Fugitive using the first 6 chars of SHA as the color which is a good idea I think.

[lambdalisue]

color with sha1 may make the word invisible. e.g. color for sha1 0123456789abcdef... makes really dark color and difficult to see

[bimlas]

I thought the same but Gina echoing the SHA too. I think the "grouped view" of commits is more important than some "invisible" SHAs.

Another solution would be to group by time (1/2/3/4 week ago, 1/2/3... month ago) but in this case a lot of commits would use the same color which makes difficult to separate visually.

EDIT

Or make the colors of SHA reverse. On the line of the actual (opened) commit make the whole line reverse.

EDIT

It's a silly idea, sorry... (reversing dark on the dark doesn't makes it more visible) :disappointed:

[bimlas]

Another idea: somehow use Vim's conceal feature to colorize a space character befor SHA, so SHA stays visible, but a colorized 'sign' before it can be used to group.

[bimlas]

Concept art:

(Imagine that the text is not colored)

[lambdalisue]

Aha, you just reached the same idea what I'm tring now (conceal box).

Well, additional that, I'm now considering to built unique color based on sha1 in HLS or HSV color space rather than RGB so that I can contral how dark or bright.

I totally agree with that chunk separation is the most important thing. The raw format is not good for this and it is what I don't like the raw format. So I'll try my best on that.

[lambdalisue]

Note. I noticed echoring via jk is slow on old computer (my Mac use Core2duo yet!) so I may drop off that and use CursorHold autocmd instead

[bimlas]

What about HSLuv? It cannotbe too dark/light.

It's OK (CursorHold), if a user is interested in the commit, then stops moving.

[lambdalisue]

What about HSLuv? It cannotbe too dark/light.

Thanks. I didn't know that. But actually I noticed that I cannot make good color for term/cterm with HSL or so on and the calculation cost a bit so I ended up to make a GOOD color.

Instead, now I'm trying to make uniqueness with three colors combination like

lambdalisue 2 days ago xxxxxxxxx
lambdalisue 2 days ago xxxyyyxxx

|
v

lambdalisue 2 days ago ▌███   -- The first half block indicate the current, and 3 blocks indicate the sha1
lambdalisue 2 days ago  ███

1. In 16 colors cterm, use the first letter to choose color. e.g. FxxFxxFxx is color16, color16, color16
2. In 256 colors cterm, use the first two letters to choose color. e.g. FFxFFxFFx is color255, color255, color255
3. In gui, use the three letters to chose color. e.g. FFFFFFFFF is #f0f0f0, #f0f0f0, #f0f0f0

Additionally, I think I don't need to put SHA1 chars while gina echo as you said. So I directly conceal the SHA1 into three boxes.

[bimlas]

Since conceal is used, the sidebar can contain the full echoed message, j/k could be mapped to nnoremap j :p<CR>j or similar.

[bimlas]

Sorry for the late recognition (didn't had time), but I'm not sure that the tricolor is a good idea. Raw blame is useless because it cannot be recognize connection between the hunks associated to the same commit because the letters/digits have to be compared one by one. If colors are used instead of SHA then it's a bit easier, but it needs the same comparing procedure of the brain. For example here's a concept art about what you wrote above.

IMAGE should be here, but nothing works for me (imgur, photobox, drag-n-drop to this comment, NOTHING :angry: )

The highlighted three lines are easier to seperate via colorized SHAs than tricolored lines.

[lambdalisue]

It is WIP but I'm trying something like

[lambdalisue]

It seems image service is down. I used dropbox instead.

[bimlas]

Oh... I though only the bars will be on the sidebar. With the commit messages it's easier to pair the hunks. Great work! :clap:

[lambdalisue]

May be I should use flexible sha1 indicator.

1. Prepare N colors
2. If there are N or less revisions in blame, use a single line
3. If there are N×N or less, use two lines
4. If there are N×N×N or less, use three lines

[bimlas]

It would be the best (and simpler), but seems to be resource intensive.

[bimlas]

(sorry, have to go, will be back tomorrow)

[lambdalisue]

I'm satisfied with this new UI.

When blame shows revisions less than 16 (single column sha1 indicator)

When blame shows revisions more than 16x16 and less than 16x16x16 (three columns sha1 indicator)

Any comments for the new UI design? (or behaviour which I have not fixed yet)

[lambdalisue]

Note that I omitted

• Author name
• SHA1

While I think when user blame, there are two step

1. Find a chunk which you may need to investigate
2. See what was happen on that chunk (revision)

And (I assumed) the author name or SHA1 is not required for the first step. They are required for the second step but users can hit <Tab> to choose echo action or show:commit:above to investigate.

Additionally, I omitted auto-echo as well while it affected to the performance on my old mac. Users can repeat actions by . so

1. Select chunk
2. Hit <Tab> and type echo then hit <CR>
3. Repeat with .

seems enough. And also, if users still want some auto-echo, they can define custom mappings.

[bimlas]

The new design seems to be cool and intuitive. :+1:

There are issues for me:

s:CURRENT_MARK = '▌'

I think it's not a good idea to use non-ascii character since not all of the fonts can display it. For example FixedSys (Windows' default monospace) shows a ?, DejaVu Sans Mono 'reversing' it (the right side is colored instead of the left). Set it to | or similar, or at least make it configurable.

show:commit shows wrong commit

For example blaming autoload/gina/core/writer.vim: echo shows the info about the right commit, but show:commit shows 803f052 on most of the lines which is absolutely irrevelant to this file (tested on Windows & Linux, without ~/.gitconfig (renamed it)). Note that the file is moved at 778963c.

autoload/gina/core/console.vim producing the same (and it's not renamed).

[bimlas]

Just found the solution of (Invalid) date but it's a modification of Vital's DateTime:

The bias (offset from UTC) is stored in a 32bit signed number (REG_DWORD type) in Windows registry even on 64 bit system. From this thread it seems that Vim itself falls short. Patch:

Fix date parsing on Windows: (Invalid) date on blame                    

The blame interface shows date like `1 month ago` well, but fails on    
`10 Feb, 2017` format (showing `(Invalid)` instead).                    

The commit fixes it, but it seems that offset has issues too. For       
example if the commit date (and author date) is
`2015-10-25 22:07:41 +0100`, then `26. Oct, 2015` shown.                                     

diff --git a/autoload/vital/_gina/DateTime.vim b/autoload/vital/_gina/DateTime.vim
index 9a56574..fa0135e 100644
--- a/autoload/vital/_gina/DateTime.vim
+++ b/autoload/vital/_gina/DateTime.vim
@@ -43,8 +43,15 @@ function! s:_vital_loaded(V) abort
   if s:Prelude.is_windows()
     let key = 'HKLM\System\CurrentControlSet\Control\TimeZoneInformation'
     let regs = s:Process.system(printf('reg query "%s" /v Bias', key))
+    " REG_DWORD is 32 bit number.
     let time = matchstr(regs, 'REG_DWORD\s*\zs0x\x\+')
-    let s:win_tz = empty(time) ? 0 : time / -s:NUM_MINUTES
+    " If integers are stored on 64 bit in Vim, then it have to be filled up to
+    " get the real value of negative numbers. See `:h limits` -> Range of a
+    " Number variable.
+    if (time =~ '^0xf') && (len(printf('%x', -1)) == 16)
+      let time = or(time, 0xffffffff00000000)
+    endif
+    let s:win_tz = empty(time) ? 0 : str2nr(time, 16) / -s:NUM_MINUTES
   endif

   " default values

Please review this and if you agree, I will open a PR on Vital (which I never used yet :smile: ).

[lambdalisue]

It's great idea to send PR to vital.vim! Please do it. But note that there are a lot of undocumented rules so you will be asked to follow the rules when you send PR like commit message shall start from a module name (e.g. DateTime: Fix bug on 64bit Vim on Windows). Sorry about this.

[lambdalisue]

And prob, you will be asked to make a test for that as well

[pocari]

It's cool! but I have a problem. When blame window was resized, redraw_content were not called. so, window had become following state.

Currently, I use resizing vim window as workarround( because redraw_content is called on VimResized) Is There way to redraw like <Plug>(gina-blame-xxxx(redraw?))

[lambdalisue]

That's good idea. I'll make it and prob assign it to C-l in default

[lambdalisue]

I thought extend ascii is widely availaule but anway i'll make a mark configurable thanks.

i'll investigate the issue thanks!

[bimlas]

How can I :Vitalize .? It says Module Vim.Buffer.Anchor is not found. I installed Vital.vim - Gina has other dependecies too?

[lambdalisue]

Yes.

https://github.com/lambdalisue/vital-System-Job https://github.com/lambdalisue/vital-Vim-Buffer-Anchor https://github.com/lambdalisue/vital-Vim-Console https://github.com/lambdalisue/vital-Vim-Buffer-Opener https://github.com/lambdalisue/vital-Vim-Window https://github.com/lambdalisue/vital-Vim-Buffer-ANSI https://github.com/lambdalisue/vital-Config

But as long as you are fixing DateTime module, the above modules are not required. The required steps are

1. Checkout vim-jp/vital.vim
2. Write test which fail on test/DateTime.vimspec
3. Execute tests by themis --target DateTime (while vital.vim has a lot of test)
4. Fix and confirm with the command above

Good luck 👍

[lambdalisue]

Fixed

• redraw_content is called on WinLeave, WinEnter, and VimResized when winwidth has changed
• User can force redraw_content by <Plug>(gina-blame-redraw) or <Plug>(gina-blame-C-L) (<C-L> in default)
• User can change current mark with g:gina#command#blame#formatter#current_mark
• rev mis-match should be fixed

Unfixed

• DateTime issue (will be fixed by https://github.com/vim-jp/vital.vim/pull/485)

@bimlas @pocari could you check?

[bimlas]

:+1: rev mis-match fixed

Some new issues:

• nomodeline should be set for sidebar: vim: Commit message drops error
• nofoldenable should be set for file buffer: do not close folds

Would be good:

• Blame on the currently blamed content (<CR> on the marked line) should set cursor's position to the same line
• Same for first blame: Gina blame should set the cursor to the same line where it was

[pocari]

@lambdalisue

redraw_content is called on WinLeave, WinEnter, and VimResized when winwidth has changed User can force redraw_content by (gina-blame-redraw) or (gina-blame-C-L) ( in default)

it works 👍

[lambdalisue]

nomodeline ...

I recognized that issue on other commands as well. So I'll fix these on a different PR.

nofoldenable

True. I'll add that

Blame on the currently blamed content...

Probably no. blame:enter action call Gina blame with

1. A commit of the chunk if the commit of the blame is different from the commit of the chunk
2. A previous commit of the chunk (namely xxxxx^ or a parent commit) if the commit of the blame is equal to the commit of the chunk

In case of 1, gina can find an original line of the current line while git detected a content movement as a chunk. So blame:enter jump to an original line of the chunk. In case of 2, gina never know what was the original line of the current line because the content did not exist on a previous commit. So currently gina reset the line and use the first line in this case.

Same for first blame

It is a bit different topic and I would like to make that but it is a bit complicated.

[bimlas]

For 2.: isn't it possible to store the current line number before blame and restore after it? I mean the cursor position, not line number of hunk. The hunk is added in the currently blamed commit, so the previous commit's same line is where the hunk will appear. It would be easier to overview the changes of the lines around the hunk if cursor's position would be restored.

[lambdalisue]

The hunk is added in the currently blamed commit, so the previous commit's same line is where the hunk will appear.

Not always. For example

Current        Previous
============   ============
 1 aaaaaaaaa   1 aaaaaaaaaa
 2 FOOFOOFOO
 3 FOOFOOFOO
 4 FOOFOOFOO
 5 bbbbbbbbb   2 bbbbbbbbbb
 6 ccccccccc   3 cccccccccc
 7 BARBARBAR
 8 BARBARBAR
 9 BARBARBAR
10 ddddddddd   4 dddddddddd
11 eeeeeeeee   5 eeeeeeeeee

In this case

1. Line 2-4 on Current should jumps to Line 2 on Previous. It is what you said
2. Line 7-9 on Current should jumps to Line 4 on Previous. It is NOT what you said (your method leads to Line 7 and it is not what we want)

The example is quite simplified so in this case we can calculate where the line comes from but in real world, there are a lot of additions/deletions/modifications so it would be quite difficult. So I just ended up to line 1 because we will quickly lost where we are with Line 7-9 to Line 7 jump in above example