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

Change label for subsegments #1224

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

t-cordonnier
Copy link
Contributor

Actually a subsegment is identified as an empty string, which can make confusion with segments coming from project

Pull request type

Feature enhancement -> [enhancement]

What does the PR change

Display first entry as file name instead of empty string

Adds new variables for "comes from":

  • ${matchSource} contains a string like 'From TM'
  • ${matchSource-dingbat} contains a symbol, better than ${matchSource} when added in a very long line
  • ${matchSource-emoji} contains an emoji, better than ${matchSource} but may be unavailable in some fonts

Which ticket is resolved?

RFE#1251

@t-cordonnier t-cordonnier force-pushed the subsegment-label branch 2 times, most recently from 1ad1c6c to ab6ccc7 Compare December 19, 2024 07:48
@miurahr
Copy link
Member

miurahr commented Dec 21, 2024

Please adjust for master and update with feedbacks from #1220

@t-cordonnier
Copy link
Contributor Author

t-cordonnier commented Dec 21, 2024

Please adjust for master and update with feedbacks from #1220

Just done but each time I do a commit I receive alerts from checkstyle.
Seems linked to the size of the method, but it was long even before I modified it...

@miurahr
Copy link
Member

miurahr commented Dec 22, 2024

Please adjust for master and update with feedbacks from #1220

Just done but each time I do a commit I receive alerts from checkstyle. Seems linked to the size of the method, but it was long even before I modified it...

Please check config/checkstyle/suppressions.xml
I specified a suppression with an exact line number, to notify developers who modify the target source the problem.

@miurahr
Copy link
Member

miurahr commented Dec 22, 2024

The change here affects OmegaT UI faced by users. It is mandatory to update the manual page to explain marks and its meanings.

@brandelune do you have any suggestions about the manual?

@brandelune
Copy link
Contributor

The change here affects OmegaT UI faced by users. It is mandatory to update the manual page to explain marks and its meanings.

@brandelune do you have any suggestions about the manual?

@t-cordonnier could you please identify in the manual the parts that need updating and suggest a draft ?

@t-cordonnier
Copy link
Contributor Author

t-cordonnier commented Dec 22, 2024

Please check config/checkstyle/suppressions.xml I specified a suppression with an exact line number, to notify developers who modify the target source the problem.

I see nothing about FindMatches.java in my local copy. Are you saying it is intentional? Do you expect us to try to make this method smaller?

@miurahr
Copy link
Member

miurahr commented Dec 22, 2024

Please check config/checkstyle/suppressions.xml I specified a suppression with an exact line number, to notify developers who modify the target source the problem.

I see nothing about FindMatches.java in my local copy. Are you saying it is intentional? Do you expect us to try to make this method smaller?

If it is so and checkstyle had reported the error for FindMatches#search, it means FindMatches#search was under 150 lines in master branch, and addition of your code brings the method over 150 lines. You need to consider a way to reduce lines, because there are too match in the one method.

The errors which the checkstyle reported in CI seems against NearString. You may want to look for a line to suppress warning for NearSting instead of FindMatches.

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.

3 participants