You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This would greatly alleviate startuptime costs when jobs are available, at the penalty of not syncing path with the results until after the job completes.
to the end of the function would need some changes; when we don't have jobs, we proceed as normal. When we do, we want to recreate this logic with a job call-back which also assigns 'path' on the correct buffer. The original function would probably have to return &l:path unmodified
This would greatly alleviate startuptime costs when jobs are available, at the penalty of not syncing path with the results until after the job completes.
Changing the behavior from deterministic to nondeterministic is a massive penalty. I could maybe get behind using the stale cached version while the new one is computed (though that only mitigates the nondeterminism) and/or providing an opt-in async API to give users enough rope to hang themselves.
When we do, we want to recreate this logic with a job call-back which also assigns 'path' on the correct buffer. The original function would probably have to return &l:path unmodified
Am I reading this correctly that you want to provide backwards compatibility by returning a bogus value? I do not approve. My initial inclination is to add an optional callback argument to classpath#detect(), but depending on how things shake out a new function may be more appropriate. Don't sweat this part; compared to jobstart()/job_start()/system() trifecta that needs to be implemented, ironing out the API will be a piece of cake. Feel free to start with an async only backwards incompatible implementation and we can decide from there how to proceed. I'm also not opposed to dropping Vim 7 support entirely.
I could maybe get behind using the stale cached version while the new one is computed (though that only mitigates the nondeterminism) and/or providing an opt-in async API to give users enough rope to hang themselves.
Agreed, fair enough. Not sure what the "opt-in async API" should look like—a variable that says "prefer aysnc over sync"?
Don't sweat this part; compared to jobstart()/job_start()/system() trifecta that needs to be implemented, ironing out the API will be a piece of cake. Feel free to start with an async only backwards incompatible implementation and we can decide from there how to proceed. I'm also not opposed to dropping Vim 7 support entirely.
Well, that certainly simplifies making a prototype :)
I do agree that doing something bogus is not good. One option you mentioned is the stale cache—but what happens when this doesn't exist? Probably for a prototype I would just leave path alone until the job gives us what we need.
jobstart()/job_start()
Argh, I forget about neovim everytime... I take it compatibility is a goal?
Agreed, fair enough. Not sure what the "opt-in async API" should look like—a variable that says "prefer aysnc over sync"?
I was thinking exposing a lower level function that the user could invoke directly, but we'd still need an option to disable the built-in behavior. Cross that bridge when we come to it.
Don't sweat this part; compared to jobstart()/job_start()/system() trifecta that needs to be implemented, ironing out the API will be a piece of cake. Feel free to start with an async only backwards incompatible implementation and we can decide from there how to proceed. I'm also not opposed to dropping Vim 7 support entirely.
Well, that certainly simplifies making a prototype :)
I do agree that doing something bogus is not good. One option you mentioned is the stale cache—but what happens when this doesn't exist? Probably for a prototype I would just leave path alone until the job gives us what we need.
The first time should always block, probably.
jobstart()/job_start()
Argh, I forget about neovim everytime... I take it compatibility is a goal?
Story of my life. Neovim does need to be supported. You could keep the system() version around for that but the end result would probably going be more complicated than just supporting both job APIs.
Activity
benknoble commentedon Oct 1, 2020
I think from about here
vim-classpath/autoload/classpath.vim
Line 125 in 88a7e8a
'path'on the correct buffer. The original function would probably have to return&l:pathunmodifiedtpope commentedon Oct 2, 2020
Changing the behavior from deterministic to nondeterministic is a massive penalty. I could maybe get behind using the stale cached version while the new one is computed (though that only mitigates the nondeterminism) and/or providing an opt-in async API to give users enough rope to hang themselves.
Am I reading this correctly that you want to provide backwards compatibility by returning a bogus value? I do not approve. My initial inclination is to add an optional callback argument to
classpath#detect(), but depending on how things shake out a new function may be more appropriate. Don't sweat this part; compared tojobstart()/job_start()/system()trifecta that needs to be implemented, ironing out the API will be a piece of cake. Feel free to start with an async only backwards incompatible implementation and we can decide from there how to proceed. I'm also not opposed to dropping Vim 7 support entirely.benknoble commentedon Oct 2, 2020
Agreed, fair enough. Not sure what the "opt-in async API" should look like—a variable that says "prefer aysnc over sync"?
Well, that certainly simplifies making a prototype :)
I do agree that doing something bogus is not good. One option you mentioned is the stale cache—but what happens when this doesn't exist? Probably for a prototype I would just leave
pathalone until the job gives us what we need.Argh, I forget about neovim everytime... I take it compatibility is a goal?
tpope commentedon Oct 2, 2020
I was thinking exposing a lower level function that the user could invoke directly, but we'd still need an option to disable the built-in behavior. Cross that bridge when we come to it.
The first time should always block, probably.
Story of my life. Neovim does need to be supported. You could keep the
system()version around for that but the end result would probably going be more complicated than just supporting both job APIs.benknoble commentedon Oct 3, 2020
Alright, I'll at least mock-up a function that does one of the three, and we can go from there?
Not sure I follow: are you saying if there's no cache, do a blocking load (
system) no matter what?