Fix solr query types

[bix0r]

Fix for issue #37, attempt 3.

Previous: PR #38.

This time I manage to skip all BC problems, allowing both strings and integers.

[bix0r]

@Ingimarsson Here is the new attempt at the type fix.

[Ingimarsson]

@bix0r I compiled it on my machine and did some tests. Looks good.

I think it might be better to use the macros ZEND_PARSE_PARAMETERS_* to read parameters... although then it would be inconsistent with the rest of the code base.

[bix0r]

@Ingimarsson I wanted to use the new ZPP, but the macro we need in this case is Z_PARAM_STR_OR_LONG and that was only added in php-8.0.

[omars44]

@bix0r Welcome :)

I do think that the code base has too much legacy that has to do more with PHP5 > PHP7 compat, and now PHP 8.0 is reaching EOL, so I don't really think we should support any older versions than the active cycle of the PHP core, as this is an extension by definition. So 8.0 should be the oldest version to support IMHO.

I'm thinking about a major refactor, to get rid of all the legacy code that had to with the switch, and to have less things to worry about, especially BC of 5 is not really wise to think about even 7. Another topic would be to modernize the codebase, for the issue at hand, I think we should support the new macros, delegating the handling of method args to the Zend Engine would do us a great favor in terms of future support and stability.

So yea, if you would be interested to e-meet sometime next week to discuss the next steps, you're welcome to get in touch via email.

Best,
Omar

[bix0r]

Hi @omars44, nice to see you back here :)

I agree that we should freshen up the code base quite a bit, and I am interested in having an e-meet next week. I will email you.

However, I would love to get this fix into master and release a version with it, with all this backwards compatibility. Just so we leave that part in a bit better state (I know many still are using 7.x and some are still supporting it). That should be pretty simple and I think this PR should be ready to be merged.

[Jan-E]

I agree with @bix0r : let us have a final PHP 7 release (with this PR merged) before skipping support for 7.x.

[omars44]

@bix0r I believe these 2 signatures need the same parameter fixes:

public  **SolrDisMaxQuery::setPhraseSlop**(string  `$slop`): [SolrDisMaxQuery](https://www.php.net/manual/en/class.solrdismaxquery.php)

public  **SolrCollapseFunction::setSize**(int  `$size`):  [SolrCollapseFunction](https://www.php.net/manual/en/class.solrcollapsefunction.php)

[bix0r]

@omars44 I found few more methods:

• SolrDisMaxQuery::setQueryPhraseSlop()
• SolrDisMaxQuery::setBigramPhraseSlop()
• SolrDisMaxQuery::setTrigramPhraseSlop()

I've fixed all 5 of them.

[omars44]

@bix0r looks good, can u rebase master and add to NEWS the changelog entry

[bix0r]

@omars44 Perhaps you want to merge the optimize PR before I rebase?

[omars44]

@bix0r done, thanks

[bix0r]

@omars44 Rebased and NEWS updated