Feature/git blame layer #2469
Feature/git blame layer #2469
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2469 +/- ##
=========================================
+ Coverage 43.83% 44.2% +0.36%
=========================================
Files 343 344 +1
Lines 13716 13886 +170
Branches 1800 1829 +29
=========================================
+ Hits 6013 6138 +125
- Misses 7464 7508 +44
- Partials 239 240 +1
Continue to review full report at Codecov.
|
|
Very cool! 😄 |
|
@bryphe thanks 👍 the layer api is really nice, making layers really fun to create, hence my various layers, its such a cool way to add extra functionality 😍 |
This reverts commit 97f0c61.
ken tooltip color
add ability to run in manual mode require a binding to be hit to show the blame
add comments to explain, also fix out of bounds function
pass in font-famil
query only a single line at a time and update state with next props
use words to calculate if truncation should happen append only if truncated, use proper truncation symbol
98729d4 to
7c10e8c
Compare
| @@ -69,6 +69,13 @@ export class BufferLayerManager { | |||
| } | |||
| } | |||
|
|
|||
| const getInstance = (() => { | |||
There was a problem hiding this comment.
Just curious - why is this wrapped in an IIFE? Is there a functionaility reason for it?
There was a problem hiding this comment.
It's to encapsulate the instance inside of the function and just return a function that provides access to it,
So rather than
let instance = new BufferLayerManager() // global variable
export getInstance = () => instance
const getInstance = (() => {
const instance = new Manager() // scoped to the IIFE and accessed via the closure
return () => {
return instance
}Its a very small distinction I've just been bitten by global vars previously and try to always find ways around them.
| } | ||
|
|
||
| public getLastEmptyLine() { | ||
| const { cursorLine, visibleLines, topBufferLine } = this.props |
There was a problem hiding this comment.
Nice work getting all this info from the buffer layer API, it's another great example 💯
| @@ -42,6 +63,7 @@ export interface VersionControlProvider { | |||
| canHandleWorkspace(dir?: string): Promise<boolean> | |||
| getStatus(): Promise<StatusResult | void> | |||
| getRoot(): Promise<string | void> | |||
| getBlame(args: BlameArgs): Promise<Blame> | |||
There was a problem hiding this comment.
Cool! I like that this is abstracted out and that the layer itself doesn't need to know about Git - just about the Blame / BlameArgs.
|
Change looks good to me! Great work @akin909 - really like the way you architected this functionality. Minor feedback on the look - I'm wondering if it makes sense to right-align the blame text? But I'm fine if that comes in separately, we can always tweak the styling later 👍 |
|
@bryphe thanks for reviewing 👍, re the look I was basically copying vscodes' GitLens plugin Having caught a peek of it looking at a colleagues setup, and also because I though another popup would be too cluttered but am open to suggestions (I don't have any alternate ideas at all myself) |
|
I enabled this a few days ago and it's working wonderfully. I really like how it finds nearby whitespace to render into - much less invasive than a pop-up that obscures code, or a split that reduces available window width. My one issue is that I use Oni with two vertical splits. This doesn't leave much room, and most of the space available is taken up with the author and time, when for me the message is more useful. Perhaps a future tweak would be some configuration as to the ordering of the fields? Maybe that can already be done with "experimental.vcs.blame.mode" config option? I haven't had chance to dive into the code to see what that does, though. |
Given the functionality was almost there I decided to try my hands at a git blame layer. The idea being to emulate a little the vscode
GitLensgit blame functionality. What this adds is a git blame per line, when the user is resting on the line like the hover tool tip.It renders inline if there is space otherwise it renders above the line in a tooltip like container.
EDIT: Removed the popup as this was in an awkward position (competing with the hover tooltip) instead the layer now renders the highlight on the given line if the space is too small it begins to truncate the blame summary if still too large it renders the summary in the earliest previous empty space (possibly overkill to have so many layers to try and preserve rendering on depending on screen space but as someone who works on a laptop it seemed worth the effort)
Outstanding: