Add 'g:lightline#gitdiff#show_empty_indicators' option

[gblock0]

I like being able to see A: 0 D: 0 M: 0, so this adds the g:lightline#gitdiff#show_empty_indicators option to show 0 for the indicators if they would otherwise be empty.

This is my first Vim script related PR, so let me know if you'd prefer things any changes.

[niklaas]

Thanks a lot for your contribution. I'll take a look at it.

[niklaas]

Thanks again for your changes. Please address the following suggestions (more detailed explanation below):

• Check for new option in lightline#gitdiff#write_calculation_to_cache() (to support all algorithms)
• Always return added, deleted and modified lines in lightline#gitdiff#algorithms#word_diff_porcelain#calculate
• Add unit tests for show_empty_indicators

You only adapted algorithm word_diff_porcelain. There is another one called numstat in the same directory here:

lightline-gitdiff/autoload/lightline/gitdiff/algorithms/numstat.vim

Line 4 in 0679231

function! lightline#gitdiff#algorithms#numstat#calculate(buffer) abort

So instead of checking for the option in word_diff_porcelain only, I would suggest checking between the following two expressions:

lightline-gitdiff/autoload/lightline/gitdiff.vim

Lines 40 to 42 in 0679231

	let l:Calculation = get(g:, 'lightline#gitdiff#algorithm',
	\ { buffer -> lightline#gitdiff#algorithms#word_diff_porcelain#calculate(buffer) })
	let g:lightline#gitdiff#cache[a:buffer] = l:Calculation(a:buffer)

This way we can guarantee that all current and all future algorithms will be supported.

This also requires changing word_diff_porcelain to always return added, deleted and modified lines -- even if there are none -- here

lightline-gitdiff/autoload/lightline/gitdiff/algorithms/word_diff_porcelain.vim

Lines 21 to 31 in 0679231

	if l:lines_added > 0
	let l:ret['A'] = l:lines_added
	endif
	if l:lines_deleted > 0
	let l:ret['D'] = l:lines_deleted
	endif
	if l:lines_modified > 0
	let l:ret['M'] = l:lines_modified
	endif

Please add additional unit tests that ensure that show_empty_indicators works as expected.

Would you mind adding these suggestions? This is quite a lot and I'm already very thankful for your contribution. Please tell me if you need any help.

[gblock0]

Thanks for the feedback! I’ll take a look tonight and make the changes.

[gblock0]

I'll add tests for show_empty_indicators later tonight

[gblock0]

Tests added and code updated! Let me know your thoughts!

These were my first Vader tests, so let me know if you'd like them structured differently.

[niklaas]

I'm sorry, this needs further changes. I you want to, you can open your branch so I can commit there directly. Otherwise feel free to apply and change the provided diffs.

Don't hesitate to ask further questions. At the moment I don't have a lot of time to spend on this plugin but I will answer eventually. 🙂

[niklaas]

And we would also need these additional changes to the other algorithm that is provided:

diff --git a/autoload/lightline/gitdiff/algorithms/numstat.vim b/autoload/lightline/gitdiff/algorithms/numstat.vim
index 4fed527..ae665c3 100644
--- a/autoload/lightline/gitdiff/algorithms/numstat.vim
+++ b/autoload/lightline/gitdiff/algorithms/numstat.vim
@@ -17,17 +17,5 @@ function! lightline#gitdiff#algorithms#numstat#calculate(buffer) abort
     return {}
   endif
 
-  let l:ret = {}
-
-  " lines added
-  if l:stats[0] !=# '0'
-    let l:ret['A'] = l:stats[0]
-  endif
-
-  " lines deleted
-  if l:stats[1] !=# '0'
-    let l:ret['D'] = l:stats[1]
-  endif
-
-  return l:ret
+  return { 'A': l:stats[0], 'D': l:stats[1] }
 endfunction

[gblock0]

No worries at all! I really appreciate all the great feedback!

• I added a lot of tests based on your feedback. I think I covered all the permutations, but let me know if I missed any
• I renamed lightline#gitdiff#algorithm to LightlineGitDiffAlgorithm (I updated the README and comments to reflect this as well)
• numstat.vim now always returns an object with A and D
• I kept the change in word_diff_porcelain.vim the same to let write_calculation_to_cache process which indicators should be shown, but happy to change it if you still want me to

I've checked Allow edits by maintainers if you'd like to make any more changes, but I'm also happy to make any other necessary changes!

[niklaas]

I really appreciate your contributions and thoughts. I’ll take a look at these throughout the day.

[niklaas]

g:LightlineGitDiffAlgorithm makes much more sense. And thanks a lot for providing the unit tests. Your contribution greatly improves the quality of the plugin!

[niklaas]

P.S.: Since the plugin seems to get a bit more attention, what do you think of #16 ? I'm wondering whether I should spend some time on that.

[niklaas]

🎉 thanks again!

[gblock0]

Happy to contribute! I'll take a look at #16 and add some thoughts.