Pass document by reference.

[tmpfs]

I think it makes more sense to pass a reference to the document when adding to the index as it is only a reference that is passed to the extraction function for each field.

Also I have a use case where I need to store the original documents and this prevents an unnecessary clone when adding to the index.

Thanks 🙏

[marcus-pousette]

Thanks for the PR!

This looks good. Not sure why I passed documents by value in the first place when creating this.

Before merging:

• Could you make sure that the cargo fmt check works as expected? See the failing pipeline step.

• Could you make sure cargo bench works? There is a compile error in test_benchmark.rs

[tmpfs]

Sure @marcus-pousette, those are fixed now 👍

[tmpfs]

Technically this is a breaking change, do you want to bump the major version, maybe 2.0.0-alpha-1?

Also, I think a better API for Filter would be to return &str and Tokenizer could return Vec<&str> and you can take ownership in add_document_to_index() where appropriate, what do you think?

[marcus-pousette]

A major version bump sounds good. Especially if we could have the improvements in the Filter and Tokenizer as you mentioned. I think the updates you mentioned sound great!

Additional things:

• I realized the readme has to be changed with this PR See the example where add_document_to_index is invoked. Should be reference now on the document in stead of values.

• cargo clippy is complaining, see failiing pipeline

If you are motivated to make the Tokenizer, and Filter update, feel free to include those in this PR or in another PR

[tmpfs]

Thanks @marcus-pousette, lets see if we can get the Tokenizer and Filter changes into this PR while I am at it, should get to it in the next day or so 👍

[tmpfs]

@marcus-pousette, I think this is good now, please review 👍

The benchmarks show a little performance improvement 😁

[marcus-pousette]

The changes you have done look good.

I included some suggestions for additional improvements that are now possible because of your changes, if you like to apply them (for even more performance gains) feel free to :). Otherwise I will merge this!

[tmpfs]

Good catch @marcus-pousette , I have updated with those changes 👍

[marcus-pousette]

Well done! Thanks 👍