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

BUG: Wrong separator used in get_synonyms on Windows #3984

Open
1 task done
nymwo opened this issue Oct 14, 2024 · 6 comments
Open
1 task done

BUG: Wrong separator used in get_synonyms on Windows #3984

nymwo opened this issue Oct 14, 2024 · 6 comments
Labels
bug Something isn't working
Milestone

Comments

@nymwo
Copy link

nymwo commented Oct 14, 2024

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

  1. Host on a Windows environment
  2. Add synonyms
  3. 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

  • I agree to follow this project's Code of Conduct
@nymwo nymwo added the bug Something isn't working label Oct 14, 2024
@felipeelia
Copy link
Member

@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?

Copy link

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.

@github-actions github-actions bot added the stale label Oct 29, 2024
@nymwo
Copy link
Author

nymwo commented Oct 31, 2024

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 github-actions bot removed the stale label Nov 1, 2024
Copy link

github-actions bot commented Nov 5, 2024

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
Copy link
Member

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

@felipeelia felipeelia added this to the 5.2.0 milestone Nov 7, 2024
@nymwo
Copy link
Author

nymwo commented Nov 13, 2024

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:

public function update_synonyms( \WP_REST_Request $request ) {
$feature = Features::factory()->get_registered_feature( 'search' )->synonyms;
$mode = $request->get_param( 'mode' );
$solr = $request->get_param( 'solr' );
$post_id = $feature->update_synonym_post( $solr );
if ( ! $post_id || is_wp_error( $post_id ) ) {
return new \WP_Error( 'error-update-post' );
}
$updated = $feature->update_synonyms();
if ( ! $updated ) {
return new \WP_Error( 'error-update-index' );
}
$feature->save_editor_mode( $mode );
return [
'data' => $solr,
'success' => true,
];
}

or here:

public function update_synonym_post( $content ) {
$synonym_post_id = $this->get_synonym_post_id();
if ( ! $synonym_post_id ) {
return $synonym_post_id;
}
return wp_insert_post(
[
'ID' => $synonym_post_id,
'post_content' => $content,
'post_type' => self::POST_TYPE_NAME,
],
true
);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants