use job for s:system() on vim8

[mattn]

Vim dispose jobs in main thread. So vim stop job operations while calling system(). This use job in s:system().

Left is without this patch. Right is with this patch.

[junegunn]

Thanks, it looks interesting. But there are places we refer to v:shell_error after s:system call to see if the call was successful but job_start, understandably, doesn't update the value. And I believe that's the reason the test cases are failing.

e.g. :call system('xxx') | PlugUpdate.

Also I wonder why bumping up maxfuncdepth is necessary. I can confirm that if it's not increased Vim gives errors when running :PlugUpdate99, but I still don't see why Vim complains when there are no recursive calls.

[mattn]

I will add code for v:shell_error. About maxfuncdepth, job_status catch-up another running jobs. So when another job make the error, exception will be raised at the line at job_status. The effect of this change, muliple jobs can be spawned in same time. So we need to grow-up maxfuncdepth.

[junegunn]

I think we should stop using v:shell_error. We can make s:system() return [result, error] pair.

[mattn]

yes, we have better to separate s:system and s:systm_with_error? as far as i see, s:system is used in many part like let foo = s:some_func(s:system(cmd)).

[mattn]

added s:system_with_error which return two values.

[junegunn]

A few test cases are still failing on Vim 8:

https://s3.amazonaws.com/archive.travis-ci.org/jobs/204097914/log.txt

Looks like the lines in the output of the new system function are not properly separated by newline characters ( ... new_branch_nameHEAD is now at ...), any idea?

(109/134) [EXECUTE] Commit hash support > ['Updated. Elapsed time: 0.256267 sec.', '[=x]', '', '- Finishing ... Done!', 'x goyo.vim:', ' error: pathspec ''ffffffff'' did not match any file(s) known to git.', '- vim-emoji: Note: checking out ''9db7fcfee0d90dafdbcb7a32090c0a9085eb054a''.You are in ''detached HEAD'' state. You can look around, make experimentalchanges and commit them, and you can discard any commits you make in thisstate without impacting any branches by performing another checkout.If you want to create a new branch to retain commits you create, you maydo so (now or later) by using -b with the checkout command again. Example: git checkout -b new_branch_nameHEAD is now at 9db7fcf... MIT license (close #2)']

[mattn]

Sorry, It's difficult to read the test result for me.

[junegunn]

I'll look into those errors when I get some time. By the way, I tested the patched version on my machine.

1. I can notice the difference when I do PlugUpdate99 that Vim becomes responsive earlier, I can move the cursor before every plugin is queued. However, the total time for the command is more or less the same; 8~10 seconds for 62 plugins with or without the patch. Maybe the difference is more noticeable on Windows?

2. A subtle issue I noticed is that when Vim becomes responsive, my cursor ends up on a random line. Before the patch, it's always on line 2.

3. For testing/debugging the issue I mentioned above, I tried commenting out redraw line. Then I noticed that some random errors occur during the update as shown below.