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

Fix language detection taking mentions and URLs into account #33700

Merged
merged 1 commit into from
Jan 23, 2025

Conversation

ClearlyClaire
Copy link
Contributor

@ClearlyClaire ClearlyClaire commented Jan 23, 2025

Fixes #33694

This should significantly improve detection accuracy.

There is still an issue with the way the function is called: it's debounced, but its result is used synchronously. I'm surprised lodash's debounce even allows you to return a value at all. The correct way is likely to use the debounced function in an effect hook and have it change the state, but it will be more work.

@ClearlyClaire ClearlyClaire requested a review from a team January 23, 2025 09:31
Copy link
Contributor

@oneiros oneiros left a comment

Choose a reason for hiding this comment

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

I am a bit surprised that the mention regexp does not already exist somewhere in the JS code (similar to the url one).

Maybe the one from twitter-text could be used instead, but I am not sure this is worth the hassle.

@ClearlyClaire ClearlyClaire added this pull request to the merge queue Jan 23, 2025
@ClearlyClaire
Copy link
Contributor Author

It exists in the counter code, I copied it from there. Maybe it would help if it were refactored.

Merged via the queue into main with commit 3178acc Jan 23, 2025
28 checks passed
@ClearlyClaire ClearlyClaire deleted the fixes/language-detection-mentions-urls branch January 23, 2025 10:35
@mcclure
Copy link

mcclure commented Jan 23, 2025

This patch is NOT in v4.4.0-nightly.2025-01-22 , correct ?

@ClearlyClaire
Copy link
Contributor Author

But it is in v4.4.0-alpha.2+pr-33702-7bd434c

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.

Language detection should not consider @mastodon@handles
3 participants