Skip to content
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

Move fzy algo from rff crate into local file and some other small tweaks to Rust code #180

Merged
merged 3 commits into from
Dec 26, 2019

Conversation

ImmemorConsultrixContrarie
Copy link
Contributor

Original (MIT licensed) "rff" crate has terminal module which utilizes std::os::unix thus it doesn't compile on non-unix OS, and could not be "cargo check"ed and tested.
If it is not accepted, all tweaks that don't depend on moving fzy algorithm are done in "Less allocations, more laziness" commit.

If `find_start_at()` returns `None`, the function instantly returns `None` too. So instead of lowercasing the whole string and then splitting it into words, split it and then lowercase given words. Less lowercasing if `find_start_at()` will exit early.

Also removed vector of indices. Those could be created in place, so no need for allocation of the whole vector. Besides, `vec.extend_from_slice(slice)` desugars into something like this:

for element in slice.iter().cloned() {
    vec.push(element);
}
The biggest change is fzy algo being moved from "rff" crate into one of the files in the directory.
Some very little changes done to the algorithm while moving.
Now Rust code could be checked and tested not on unix systems only.

Also added ".lock" to gitignore for libs.
The reason is explained here:
https://doc.rust-lang.org/cargo/faq.html#why-do-binaries-have-cargolock-in-version-control-but-not-libraries
@@ -0,0 +1,3 @@
/target
!.cargo
*.lock
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the faq applies to our situation. Although it's a library, it's unpublished and actually used for vim-clap exclusively. No others depend on it, and we need to ensure people are always able to compile this Rust extension on their own. Keeping Cargo.lock is better for our case.

@liuchengxu
Copy link
Owner

Overall looks good to me, except ignoring the Cargo.lock I'm not against moving the fzy algo impl into vim-clap if that helps non-unix users. We can use a dedicated fzy library once stewart/rff#2 is resolved, but rff is seemingly not much active now :(.

@ImmemorConsultrixContrarie
Copy link
Contributor Author

Okay, will revert gitignoring .lock files and add my .locks to the PR, once I'm back at home.

Removed *.lock from .gitignore for libs, added my (Windows) .locks to the pushed files.
@ImmemorConsultrixContrarie
Copy link
Contributor Author

Ready to merge now, I guess?
@liuchengxu

@liuchengxu liuchengxu merged commit 997f60a into liuchengxu:master Dec 26, 2019
@liuchengxu
Copy link
Owner

Merged, thanks!

liuchengxu pushed a commit that referenced this pull request Aug 17, 2021
* GamePower nft-collectibles-wallet Milestone 1

* updating docs for wallet pallet

* updating docs for wallet example node

* updating repos README to provide a better description of the project
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants