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

[BUGS#1251] FindMatches always search separate segment match without penalty: add regression test, fix it, make configurable, show where comes from in FuzzyMatch pane, and refactor #963

Draft
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

miurahr
Copy link
Member

@miurahr miurahr commented Feb 17, 2024

Pull request type

Please mark GitHub LABEL of the type of change your PR introduces:

  • Bug fix → [bug]

Which ticket is resolved?

What does this PR change?

  • Add regression test for BUGS#1248 and 1251 against both FindMatches and FindMatchesThread classes, that is failed by the bug reported.
  • Add internal FindMatches#search method which change behavior by argument flag, segmentation support, and remove boolean allowSeparateSegmentMatch argument from constructor.
  • make the feature "paragraph from separated segment matches on sentence level TMX" configurable
  • Update manual document to add explanation of preference > TM matches - paragraph from separated segment
  • Show fuzzy match result comes from "Project/Sub-segment match/External TM" on the fuzzy match pane

Other information

@miurahr miurahr changed the title [BUGS#1248] feat: add regression test [BUGS#1248,1251] feat: add regression test Feb 17, 2024
@miurahr miurahr added the bug label Feb 17, 2024
@miurahr miurahr force-pushed the topic/miurahr/matches/fix-separate-segmented branch from bc9c2cd to e42b6d3 Compare February 17, 2024 00:43
@miurahr miurahr changed the title [BUGS#1248,1251] feat: add regression test [BUGS#1248,1251] add regression test and refactor FindMatches class Feb 17, 2024

This comment was marked as outdated.

2 similar comments
Copy link

❌ Run Gradle test failed:

Copy link

❌ Run Gradle test failed:

@miurahr miurahr force-pushed the topic/miurahr/matches/fix-separate-segmented branch 2 times, most recently from bc92bea to 4665a8e Compare February 17, 2024 07:23
@omegat-org omegat-org deleted a comment from github-actions bot Feb 17, 2024
@omegat-org omegat-org deleted a comment from github-actions bot Feb 17, 2024
@omegat-org omegat-org deleted a comment from github-actions bot Feb 17, 2024
@omegat-org omegat-org deleted a comment from github-actions bot Feb 17, 2024
@omegat-org omegat-org deleted a comment from github-actions bot Feb 17, 2024
@omegat-org omegat-org deleted a comment from github-actions bot Feb 17, 2024
@omegat-org omegat-org deleted a comment from github-actions bot Feb 17, 2024
@miurahr
Copy link
Member Author

miurahr commented Mar 9, 2024

A topic branch here waiting a realization of GUI behavior acceptance test #964 is accepted.

@miurahr miurahr marked this pull request as ready for review July 28, 2024 01:19
@miurahr
Copy link
Member Author

miurahr commented Aug 14, 2024

A topic branch here waiting a realization of GUI behavior acceptance test #964 is accepted.

GUI testing is not stable to be integrated. So we should merge it now.

@miurahr miurahr changed the title [BUGS#1248,1251] add regression test and refactor FindMatches class [BUGS#1251] add regression test and refactor FindMatches class Nov 4, 2024
@miurahr miurahr added this to the 6.1.0 (Require Java 11) milestone Nov 4, 2024
@miurahr
Copy link
Member Author

miurahr commented Nov 4, 2024

#1172 enables acceptance test.

- Add FindMatchesThreadTest to reproduce BUGS#1251
- Add a test case to test FindMatches with the case of BUGS#1251.

Signed-off-by: Hiroshi Miura <[email protected]>
- add internal search method to handle normal and segmented search conditions, also use for test purpose
- drop threshold arguments for CalcMatchStatistics usage
- FindMatchesThread.finderSearch to take threshold argument for testing.
- update search() callers accordingly.

Signed-off-by: Hiroshi Miura <[email protected]>
@miurahr miurahr force-pushed the topic/miurahr/matches/fix-separate-segmented branch from 1e4afdb to f049239 Compare November 26, 2024 04:08

This comment was marked as resolved.

@miurahr miurahr changed the title [BUGS#1251] FindMatches always search separate segment match without penalty: add regression test, fix it, and refactor [BUGS#1251] FindMatches always search separate segment match without penalty: add regression test, fix it, make configurable, and refactor Nov 29, 2024
@miurahr miurahr marked this pull request as ready for review November 29, 2024 03:57
- update expectation not affected by OS path delimiter

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

This comment was marked as outdated.

- Change default fuzzy match pane template with matchSource
- Add method MatchesVarExpansion#expandMatchSource
- Extend NearString.MATCH_SOURCE to have TM_SUBSEG
- Update test expectations of MatchesTextAreaTest, and FindMatchesTest
- FindMatches to handle FuzzyMark of segmented match
- FindMatches to store tmx file path of segmented matches
- Fix MatchesVarExpansionTest which causes NPE
- Add human-readable names of MATCH_SOURCE in Bundle.properties

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

❌ Quality checks failed.

Please look a Gradle Scan page for details:
https://gradle.com/s/5jugeswutt4eq

@miurahr miurahr requested a review from t-cordonnier November 30, 2024 02:56
@miurahr miurahr changed the title [BUGS#1251] FindMatches always search separate segment match without penalty: add regression test, fix it, make configurable, and refactor [BUGS#1251] FindMatches always search separate segment match without penalty: add regression test, fix it, make configurable, show where comes from in FuzzyMatch pane, and refactor Nov 30, 2024
- Improve javadoc
- Improve switch syntax style
- Improve indentation

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

This comment was marked as resolved.

This comment was marked as resolved.

@miurahr miurahr force-pushed the topic/miurahr/matches/fix-separate-segmented branch from ebb7da2 to 0878d1b Compare November 30, 2024 12:43
@miurahr
Copy link
Member Author

miurahr commented Dec 5, 2024

@t-cordonnier now plans to propose further improvements, I would like to merge here then go to his proposal when ready.

Copy link
Contributor

@brandelune brandelune left a comment

Choose a reason for hiding this comment

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

Documentation modifications need review.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Kazephil we need your review in the 3 documentation files below. Thank you in advance.

/** From files */
FILES,
/** From sub-segmented match */
TM_SUBSEG;
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
TM_SUBSEG;
TM_SUBSEG

no need semicolon

@miurahr
Copy link
Member Author

miurahr commented Dec 18, 2024

As discussed in dev-ML, I split changes to a handful of PRs.
@Kazephil @brandelune pls review #1217 and #1218 for documentation and UI labels.

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