BUG: Wrong separator used in get_synonyms on Windows

nymwo commented 1 month ago

Describe the bug

The Synonym feature doesn't work when hosted on Windows.

In wp-content\plugins\elasticpress\includes\classes\Feature\Search\Synonyms.php we can se that the synonyms are separated (explode) by PHP_EOL, which is \r\n on Windows:

    public function get_synonyms() {
        $synonyms_raw = $this->get_synonyms_raw();
        $synonyms     = array_values(
            array_filter(
                array_map( [ $this, 'validate_synonym' ], explode( PHP_EOL, $synonyms_raw ) )
            )
        );

        /**
         * Filter array of synonyms to add to a custom synonym filter.
         *
         * @hook ep_synonyms
         * @return  {array} The new array of search synonyms.
         */
        return apply_filters( 'ep_synonyms', $synonyms );
    }

But it looks like $this->get_synonyms_raw() always returns a string with synonyms separated by \n. On Linux, PHP_EOL is defined as \n, so this issue does not appear when running on that OS.

Changing PHP_EOL to "\n" resolves the issue on Windows.

Steps to Reproduce

Host on a Windows environment
Add synonyms
Try to use those synonyms

Screenshots, screen recording, code snippet

No response

Environment information

No response

WordPress and ElasticPress information

No response

Code of Conduct

[x] I agree to follow this project's Code of Conduct

felipeelia commented 1 month ago

@nymwo is your database also hosted on Windows? The synonyms content comes from the post_content of a CPT called ep-synonym, and is saved by the handle_update_synonyms method. Can you check exactly where that \r\n becomes \n?

github-actions[bot] commented 4 weeks ago

It has been 3 days since more information was requested from you in this issue and we have not heard back. This issue is now marked as stale and will be closed in 3 days, but if you have more information to add then please comment and the issue will stay open.

nymwo commented 3 weeks ago

Yes, the database is on Windows.

This is what is sent when i press "Save changes" on the synonym admin page:

{"mode":"simple","solr":"# Defined synonyms.\n\nTest, Try\n\n# Defined hyponyms.\n\nblue => blue, aqua, azure, cerulean, cyan, ultramarine\n\n# Defined replacements.\n\nsupposably => supposedly\nflustrated => flustered, frustrated\nintensive purposes => intents and purposes\n"}

Though minified, I can see that synonyms-script.js joins the lines with "\n" when constructing the "solr" parameter. Changing that to use PHP_EOL might fix this.

github-actions[bot] commented 3 weeks ago

It has been 3 days since more information was requested from you in this issue and we have not heard back. This issue is now marked as stale and will be closed in 3 days, but if you have more information to add then please comment and the issue will stay open.

felipeelia commented 2 weeks ago

@nymwo do you want to craft a PR with that change? Thanks!

nymwo commented 2 weeks ago

I'm not sure where it makes sense to make the change. If not passing PHP_EOL to the JS script, maybe there should be a line break fix here:

https://github.com/10up/ElasticPress/blob/b4326906e602754d989618bcb85b18e036245f33/includes/classes/REST/Synonyms.php#L94-L118

or here:

https://github.com/10up/ElasticPress/blob/b4326906e602754d989618bcb85b18e036245f33/includes/classes/Feature/Search/Synonyms.php#L859-L874

10up / ElasticPress