-
Notifications
You must be signed in to change notification settings - Fork 73
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
Unbundle file engine binaries from plugin #282
base: master
Are you sure you want to change the base?
Conversation
Additional minor improvements:
|
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.
Very sensible improvement; thanks for the good work!
autoload/ctrlspace/context.vim
Outdated
@@ -94,6 +94,12 @@ function! s:init() abort | |||
|
|||
if s:conf.FileEngine ==# "auto" | |||
let s:conf.FileEngine = s:detectEngine() | |||
else |
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.
See above proviso.
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.
I been meaning to submit an issue outlining a couple of my main medium to long-term plans/ideas for the project going forward, you'll know it when you see it.
Just as a preview though, one major point I want to raise is that, for an eventual major release we should remove some unnecessary options (which ones are deemed so is ofc up for discussion and should be agreed upon before the changes are committed).
@Konfekt I added a MD5 checksum check, and added some more details to the README. Lmk what you think; I think it's probably more or less good to be merged. Although I'm not too sure if we should just remove those 4 binaries from tracking immediately. Since if that's done, next time people pull the update, it'll be deleted from their plugin, but if the script didn't work correctly, then they might get confused. But without removing the binaries from git tracking, then it defeats the whole point of this PR. What you think? |
819eb27
to
d08a81b
Compare
Well, this change is abrupt, but for the better. The spell download vimscript file https://github.com/vim/vim/blob/master/runtime/autoload/spellfile.vim serves as a template to implement it without any need for vim-plug on every OS. |
* add post-install shell script to auto fetch file engine bin * use MD5 to prevent unnecessary re-downloading * update 'Installation' section of README * remove existing bins from git tracking (don't delete bins for now)
Instead making users download 4 file engine binaries (3 of which they will never use or even all 4 if their OS+arch doesn't match any of them), it makes more sense to use a post install script to download only the binary applicable to their system.