RishabhRD / popfix

Neovim lua API for highly extensible popup window
83 stars 3 forks source link

libfzy-bsd-x86_64.so missing on FreeBSD #5

Closed xanderio closed 3 years ago

xanderio commented 3 years ago

When loading this plugin fzy-native.lua tries to load libfzy-bsd-x86_64.so. After some digging I noticed that this shard object in not include in this repository.

This seems to be linked to #3

I also doubt that the same so file will work on all BSD Platforms (FreeBSD, NetBSD, OpenBSD, DragonflyBSD).

RishabhRD commented 3 years ago

The fzy native algorithm implementation is based on fzy-lua-native. The linked repository doesn't include configuration of BSD platforms.

I need to look into it, because I guess we can support unix based platforms easily as these have very similar architecture.

Maybe we can do some sort of dynamic compilation.

Looking into the best way to support all unix based platforms easily. :smiley:

Thanks for reporting

xanderio commented 3 years ago

I just build that project under FreeBSD, it produces a libfzy-freebsd-amd64.so shared object.

Dynamic compilation should be doable. This would also remove the need for .so file in git.

RishabhRD commented 3 years ago

That's good! Then we can just give a makefile and users can use it generate suitable file for their platform. I guess even this can be automated for most of plugin managers. I would try push the suitable makefile today itself.

Thanks for such a good help. :smile:

xanderio commented 3 years ago

I would suggest doing something like nvim-treesitter/nvim-treesitter, they compile there .so via the :TSInstall command. A separate commend would probably be very overkill for the project. So why not compile the .so file if it is not found.

RishabhRD commented 3 years ago

Now, the dynamic compilation is possible. The shared object can be built automatically with the use of the plugin manager post-update hook. (README updated)

I have not pushed the .so files because it can be a little less robust if I feel some need to change C files in the future.

I guess this should work with most operating systems(Just windows may be problematic. As I don't have any windows machine.)

ruifm commented 3 years ago

I think you added a submodule (neovim) without specifying it in a .gitmodules. Now I get errors when trying to update/reinstall.

Also, why not bring the fzy-lua-native as a git submodule instead of just copying its source?

RishabhRD commented 3 years ago

@ruifm I accidentally pushed the testing module being used in pull request #4. I was a little bit careless while pushing.

Thanks for pointing it out too fast. :smile:

The reason I am not adding fzy-lua-native as git submodule is we don't need many of its files like benchmark.lua, etc. However, I would check the lua source code organization a little bit. If it just simplifies the current scenario then why not, it would be great. Thanks for the suggestion. :smiley:

ruifm commented 3 years ago

The reason I am not adding fzy-lua-native as git submodule is we don't need many of its files like benchmark.lua, etc. However, I would check the lua source code organization a little bit. If it just simplifies the current scenario then why not, it would be great. Thanks for the suggestion. smiley

I personally think it's easier to update changes from upstream (recently I submitted a patch to support aarch64) and also more transparent (as in, one can see where the code actually came from and authorship is properly credited). The extra files should not really an issue. Most vim plugins that live in repos have a lot of functionally useless files (like READMEs) and no one ever complained.

RishabhRD commented 3 years ago

@ruifm Yeah you are right. I agree with you. I would push the submodule based architecture tomorrow. It would be a great improvement. A big thanks :smile:

RishabhRD commented 3 years ago

@xanderio We have added native fzy sorter as a separate git submodule so that it is not a hard dependency for the project.

install_native_fzy_sorter script is included for the dynamic compilation thing.

Pull request #7 does the job. This is not only good for Unix based platforms but also helps for windows as well. If windows is supported by the author then it would be automatically be supported by popfix. (I guess they support windows with some special criterion.)

I guess this solves the issue.

Thanks for reporting. :smile: