-
-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add branch name to payload #27
base: master
Are you sure you want to change the base?
Conversation
Still learning vimscript. @ErikBjare @johan-bjareholt , let me know if I can improve anything. |
plugin/activitywatch.vim
Outdated
@@ -105,6 +107,20 @@ function! s:CreateBucket() | |||
call HTTPPostJson(s:bucket_apiurl, l:body) | |||
endfunc | |||
|
|||
function! s:GetGitBranch(localtime) | |||
if a:localtime - s:last_branch_update > 5 | |||
let l:cmd_result = systemlist('git branch --show-current')[0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This call is still synchronous. So while it's an improvement, there's still a risk of VIM freezing when doing s blocking operation like systemlist. It would be optimal if we did something similar as in HTTPPostJson with jobstart.
@johan-bjareholt I think this will work. I update the branch async, but the branch is still sent with the rest of the data. I think this is preferred over sending it in a separate heartbeat? |
@johan-bjareholt , do you want to have something added to the README about how to access the data through the query-api?
|
@FilipHarald That would be nice :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Would like a review from @johan-bjareholt too though.
plugin/activitywatch.vim
Outdated
function! s:RefreshGitBranch(localtime) | ||
if a:localtime - s:last_branch_update > 5 | ||
let s:last_branch_update = a:localtime | ||
let l:cmd_result = systemlist('git branch --show-current')[0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like I said before, systemlist is a synchronous operation and needs to be replaced with something asynchronous such as jobstart/job_start.
@FilipHarald This is pretty stale already but I'd love to get this merged, any chance you would continue with this ? |
@varac it helps a little bit now that I know that someone needs it :) I will see what I can do. No promises though! |
@johan-bjareholt I made a second try now. Also, instead of doing |
A second try at adding branch name to payload #19 . This time with 5-second cache to reduce calls to
systemlist
, hopefully resolving #26 .