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

Fuzzy translations #42

Merged
merged 7 commits into from
Sep 24, 2015
Merged

Fuzzy translations #42

merged 7 commits into from
Sep 24, 2015

Conversation

whatyouhide
Copy link
Contributor

Hey @josevalim, can you have a look at this when you have some time? It's just the foundations of how fuzzy matching would work. It's very simple, but not configurable (I guess we should pass things like --fuzzy/--no-fuzzy and other options to gettext.merge).

Wdyt?

If the translations are not of the same type (e.g., a translation and a plural
translation), `-1` is returned.
"""
@spec jaro_distance(Gettext.PO.translation, Gettext.PO.translation) :: 0..1 | -1
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see where -1 would come from.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whooops, documented the spec but forgot to implement that. :)

@whatyouhide
Copy link
Contributor Author

@josevalim pushed a couple of new commits and left a bunch of comments around the code. Let me know what you think :)

@josevalim
Copy link
Contributor

This looks great. Now we just need how to make the fuzzy configurable as well as the threshold. We can probably do something like this:

match(old, new, fuzzy_matcher)

Where fuzzy match receives two translations and returns {:match, pos_integer} and :nomatch. Then we could call it like this:

match(old, new, Gettext.Fuzzy.matcher(0.8))

The matcher will return an anonymous function as defined above and will return {:match, _} only if it is above the threshold.

Thoughts?

@whatyouhide
Copy link
Contributor Author

Got it, will start working on it right now :)

@whatyouhide
Copy link
Contributor Author

@josevalim implemented Gettext.Fuzzy.matcher/1. Wdty?

@josevalim
Copy link
Contributor

Beautiful. Now let's make use of it in gettext.merge, provide the proper options, and ship it! :D

This module takes care of

* finding how similar two translations are
* fuzzy-merging two translations
This function is used to return a matcher that tells if two translations
are a fuzzy match.
@whatyouhide
Copy link
Contributor Author

@josevalim pushed the --fuzzy and --fuzzy-threshold options, wdyt about the API?

alias Gettext.Merger

def run(args) do
_ = Mix.Project.get!

case OptionParser.parse(args, strict: [locale: :string]) do
parse_switches = [locale: :string, fuzzy: :boolean, fuzzy_threshold: :float]
case OptionParser.parse(args, strict: parse_switches) do
{opts, [arg1, arg2], _} ->
Copy link
Contributor

Choose a reason for hiding this comment

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

The third element of the tuple are option parsing errors. Maybe we should check they are not empty and print them somehow.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe something like this:

 {_, _, [_|_] = errors} ->
  for {key, _} <- errors, do: Mix.shell.error "#{key} is invalid"
  Mix.raise "mix gettext.merge aborted."

The following options are forwarded to the `gettext.merge` task, which is
called internally by this task:

* `--fuzzy`
Copy link
Contributor

Choose a reason for hiding this comment

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

--no-fuzzy. I would actually reword the paragraph to say that it supports the same option as gettext.merge in those cases and just show an example. :)

@whatyouhide
Copy link
Contributor Author

@josevalim pushed the fixes to the docs, I'm merging this PR. I'll polish the docs a little bit and then we can release 0.6.0 🎉

@whatyouhide whatyouhide changed the title [WIP] Fuzzy translations Fuzzy translations Sep 24, 2015
whatyouhide added a commit that referenced this pull request Sep 24, 2015
@whatyouhide whatyouhide merged commit 3763a3f into master Sep 24, 2015
@whatyouhide whatyouhide deleted the fuzzy-matching branch September 24, 2015 07:11
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.

None yet

2 participants