search

skywind3000 / asyncrun.vim

:rocket: Run Async Shell Commands in Vim 8.0 / NeoVim and Output to the Quickfix Window !!
https://www.vim.org/scripts/script.php?script_id=5431
MIT License
1.84k stars 109 forks source link

fix: use qf id to prevent interleaving output #239

Closed kaihowl closed 2 years ago

kaihowl commented 2 years ago

Fixes #73

This is RFC. Since the id functionality for vim only landed in 8.0.1023 and nvim v0.6.1, there should probably be some checks added for this functionality? Or should it silently fail as the dictionary key is simply ignored?

One caveat to this (probably more important to add to documentation than to fix it) is that when there are more quickfix lists than 10, the least recently updated quickfix list is freed. If the output of the command is frequent enough (more frequent than we add quickfix lists) this is not an issue. If there is a long running command with no output, though, the quickfix list will silently be freed and the output of the command effectively goes into void.

KaiHoewelmeyer-TomTom commented 2 years ago

@skywind3000, please have a look. :)

skywind3000 commented 2 years ago

Sorry, busy on my daily job, will review tommorrow.

KaiHoewelmeyer-TomTom commented 2 years ago

@skywind3000, no urgency at all. I only wanted to ping you as I wasn't sure if you are looking at "draft" state reviews. This is an unfinished proposal. Before fixing up all remaining caveats, I want to get an indication of go/no-go.

skywind3000 commented 2 years ago

OK, what I care about is compatibility with old vim, asyncrun is promised to run under patch-7.4.1829,

Does this patch can handle both situation for new & old vims ?

kaihowl commented 2 years ago

@skywind3000, thank you for the feedback. I will brush up the patch such that it works with 7.4.1829 as well. As initially said: I definitely need to add feature checking for the availability of the quickfix-ID.

kaihowl commented 2 years ago

@skywind3000 this is now ready for review. I tested on nvim 0.6.1, vim 7.4.1829, and vim 8.1.3489. Testing included checking strip/append and raw modes.

If there is anything else I should check or change, please let me know. I am happy to follow up.

As before: There is no urgency to reviewing this.

Thank you for your time reviewing this and thank for for maintaining this great plugin.

kaihowl commented 2 years ago

@skywind3000 I renamed the variable to your proposed, shorter name. Moreover, the readme now documents the setting as well. I used google translate to add the chinese readme documentation as well. I am almost certain that the wording has to be improved or changed entirely. As I don't speak any Chinese, please have a close look at it.

skywind3000 commented 2 years ago

Google Chinese translation is decent and can be directly used in the README-cn.md.

I will merge this at the first place, and made some minor modifications myself.

Thanks.