/mastodon

Add language detection #1772

Merged
merged 5 commits into from Apr 16, 2017

Conversation

Reviewers

@mjankowski mjankowski

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@Gargron @mjankowski @ineffyble
@Gargron
Member

Gargron commented Apr 14, 2017

  • Language detection through WhatLanguage gem
  • Stored as status metadata
  • Serialized in Atom feeds
  • Deserialized from Atom feeds

What else must be done? Expose language attribute in JSON API? Language filters for public timelines?
@Gargron

This comment has been minimized.

Show comment
Hide comment
@Gargron

Gargron Apr 14, 2017

Member

Hmm, surprised about PG::NotNullViolation tbh, I had the impression if a passed value is null and the column is not null, the default value would be used....
Member

Gargron commented Apr 14, 2017

Hmm, surprised about PG::NotNullViolation tbh, I had the impression if a passed value is null and the column is not null, the default value would be used....

@Gargron Gargron changed the title from [WIP] Add language detection to Add language detection Apr 15, 2017

app/services/process_feed_service.rb
@@ -230,6 +231,10 @@ def content(xml = @xml)
xml.at_xpath('./xmlns:content', xmlns: TagManager::XMLNS).content
end
+ def content_language(xml = @xml)
+ xml.at_xpath('./xmlns:content', xmlns: TagManager::XMLNS)['xml:lang']&.presence || 'en'

This comment has been minimized.

Show comment
Hide comment
@mjankowski

mjankowski Apr 16, 2017

Collaborator

Should that be hardcoded to en as default, or should it use whatever I18n.default_locale is?
@mjankowski

mjankowski Apr 16, 2017

Collaborator

Should that be hardcoded to en as default, or should it use whatever I18n.default_locale is?
db/migrate/20170414132105_add_language_to_statuses.rb
@@ -0,0 +1,5 @@
+class AddLanguageToStatuses < ActiveRecord::Migration[5.0]
+ def change
+ add_column :statuses, :language, :string, null: false, default: 'en'

This comment has been minimized.

Show comment
Hide comment
@mjankowski

mjankowski Apr 16, 2017

Collaborator

Will this make all historical statuses on an instance retroactively call themselves en ?
@mjankowski

mjankowski Apr 16, 2017

Collaborator

Will this make all historical statuses on an instance retroactively call themselves en ?

This comment has been minimized.

Show comment
Hide comment
@Gargron

Gargron Apr 16, 2017

Member

Yep, it will. That's the status quo - all current statuses are in the same pool
@Gargron

Gargron Apr 16, 2017

Member

Yep, it will. That's the status quo - all current statuses are in the same pool

This comment has been minimized.

Show comment
Hide comment
@mjankowski

mjankowski Apr 16, 2017

Collaborator

Is that true for a server running with a different default locale?

Like, if an admin has bothered to set a different default locale value, it might be a safe assumption that the statuses created on that instance are in that locale, and that this migration should use that value instead of hardcoding en?

When this is deployed it will be run on each instance separately, so it might be "more correct" to make the assumption based on that (ie, are the instances running in Japan, Hong Kong, etc - primarily using en or other locale?)
@mjankowski

mjankowski Apr 16, 2017

Collaborator

Is that true for a server running with a different default locale?

Like, if an admin has bothered to set a different default locale value, it might be a safe assumption that the statuses created on that instance are in that locale, and that this migration should use that value instead of hardcoding en?

When this is deployed it will be run on each instance separately, so it might be "more correct" to make the assumption based on that (ie, are the instances running in Japan, Hong Kong, etc - primarily using en or other locale?)
@mjankowski

This comment has been minimized.

Show comment
Hide comment
@mjankowski

mjankowski Apr 16, 2017

Collaborator

Side question to the actual changes here, is this change being made in future support of something like - #1094 ?
Collaborator

mjankowski commented Apr 16, 2017

Side question to the actual changes here, is this change being made in future support of something like - #1094 ?
@Gargron

This comment has been minimized.

Show comment
Hide comment
@Gargron

Gargron Apr 16, 2017

Member

Yeah, pretty much, although unsure which UI path I'll take. But getting the language attribute out to clients via the API is first step - would allow some to display a "translate" button conditionally, perhaps.
Member

Gargron commented Apr 16, 2017

Yeah, pretty much, although unsure which UI path I'll take. But getting the language attribute out to clients via the API is first step - would allow some to display a "translate" button conditionally, perhaps.

ineffyble added some commits Apr 16, 2017

@ineffyble
Merge branch 'master' into feature-language-detection
Loading status checks…
9b07a7d
@ineffyble
Re-add newline before 'react-rails' Gem to fix groupings 
Fixes Code Climate issue
Loading status checks…
ac16b92

@Gargron Gargron merged commit e4af489 into master Apr 16, 2017
3 checks passed

3 checks passed

codeclimate no new or fixed issues
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@Gargron Gargron deleted the feature-language-detection branch Apr 16, 2017

@d6rkaiz d6rkaiz referenced this pull request Apr 16, 2017

Closed

raise PG::NotNullViolation at statues table #1957

1 of 2 tasks complete

@tomouchuu tomouchuu referenced this pull request Apr 18, 2017

Closed

[Feature Request] Language Translation #883

1 of 1 task complete

Nyoho pushed a commit to Nyoho/mastodon that referenced this pull request Apr 25, 2017

@Gargron
Add language detection (#1772) 
* Add language detection via WhatLanguage and (de)serialization of it through Atom

* Fix default language in ProcessFeedService

* Re-add newline before 'react-rails' Gem to fix groupings

Fixes Code Climate issue
e61a12f

@ornithocoder ornithocoder referenced this pull request in tootsuite/documentation May 25, 2017

Merged

Adds language to Status #264

tearaikazuki pushed a commit to Craftodon/Craftodon that referenced this pull request Oct 15, 2017

@ornithocoder
Adds language to Status 
Based on:

* #1772
* #3209
0854f0f

@jorgesumle jorgesumle referenced this pull request in liberapay/salon Dec 14, 2017

Open

Language-specific social media accounts? #186

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment