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

feat: separate segment matcher configurable #961

Closed

Conversation

miurahr
Copy link
Member

@miurahr miurahr commented Feb 16, 2024

Pull request type

  • Bugs
  • Feature enhancement -> [enhancement]

Which ticket is resolved?

What does this PR change?

  • Add preference to enable separate segment matcher
  • Add panalty value for separate segment matcher
  • Make penalty configurable for separate segment matcher
  • UI: add checkbox and spinner on TMMatcher preference dialog
  • Add TMX path as "from_separate_segment_matcher" when match it.

Other information

- Add preference to enable separate segment matcher
- Add panalty value for separate segment matcher
- Make penalty configurable for separate segment matcher
- UI: add checkbox and spinner on TMMatcher preference dialog
- Add TMX path as "from_separate_segment_matcher" when match it.

Signed-off-by: Hiroshi Miura <[email protected]>
Copy link

❌ Run Gradle test failed:

- fix indent
- Update supressions.xml

Signed-off-by: Hiroshi Miura <[email protected]>
@miurahr miurahr added the bug label Feb 16, 2024
@brandelune
Copy link
Contributor

We don't need to rush a modification. Now that we understand where the issue comes from we need to discuss it and see if it is worth modifying. We can consider that as a undocumented feature and not as a bug since that's the way Alex intended to implement it.

Also, it seems to appear only in very specific cases. Here, the separate string was (1) and the issue did not appear with longer strings.

Maybe the feature implementation is not correct (obviously I should not have a 100% match here, or it should be displayed differently). But I don't think adding another preference is the solution.

@miurahr miurahr marked this pull request as draft February 16, 2024 08:23
@miurahr
Copy link
Member Author

miurahr commented Feb 17, 2024

There is successor #963

@miurahr miurahr closed this Feb 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants