omegat icon indicating copy to clipboard operation
omegat copied to clipboard

[RFE#1736] feat: Alternative translation highlight

Open chelobaka opened this issue 1 year ago • 9 comments

Pull request type

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

  • Feature enhancement -> [enhancement]

Which ticket is resolved?

  • Alternative translation highlight
    • https://sourceforge.net/p/omegat/feature-requests/1736/

What does this PR change?

  • This PR implements new highlighter for marking segments with an alternative translation.
  • GUI bits one might expect for a highlighter like changing color in settings and default color values for light/dark themes.

Other information

chelobaka avatar Feb 06 '24 20:02 chelobaka

OmegaT marker plugins implementations and marker API are incomplete. When we want to extend something related to marker API, it is chance to refactor and enhance marker API. One of places we should improve is marker preference. We should allow marker plugin to extend preference without changing IEditorSettings class and also extending UI parts. IMarker should provide a way to give preference API that marker implementation just return a name of marker, preference key and default values.

miurahr avatar Feb 07 '24 00:02 miurahr

About marking alternative translations, let's remember that I had another proposal: https://github.com/t-cordonnier/omegat/tree/mark-tra-alternative

In this proposal, alternative translations are marked two ways:

  • as a context variable in translation info => advantage: the mark is visible not only for current segment, but also other ones
  • as an extra character in the segment marker (only because I know some people wanted it)

The problem with using a marker is that you have to think about accumulation: what if the segment is alternative + coming from tm/auto?

t-cordonnier avatar Feb 07 '24 07:02 t-cordonnier

OmegaT marker plugins implementations and marker API are incomplete. When we want to extend something related to marker API, it is chance to refactor and enhance marker API. One of places we should improve is marker preference. We should allow marker plugin to extend preference without changing IEditorSettings class and also extending UI parts. IMarker should provide a way to give preference API that marker implementation just return a name of marker, preference key and default values.

Sounds reasonable. Shall I continue with this PR the old way or the interfaces should be extended first? And I'd be happy to help if you need it.

chelobaka avatar Feb 07 '24 10:02 chelobaka

OmegaT marker plugins implementations and marker API are incomplete. When we want to extend something related to marker API, it is chance to refactor and enhance marker API. One of places we should improve is marker preference. We should allow marker plugin to extend preference without changing IEditorSettings class and also extending UI parts. IMarker should provide a way to give preference API that marker implementation just return a name of marker, preference key and default values.

Sounds reasonable. Shall I continue with this PR the old way or the interfaces should be extended first? And I'd be happy to help if you need it.

The work is proposed in #951 An improvement requires hard work to change many, so you can do it with old way, and I'll propose the change with your improvement in a place of configurations and menu items.

miurahr avatar Feb 07 '24 11:02 miurahr

@chelobaka please register the marker plugin in Plugins.properties

Subject: [PATCH] fix: activate marker plugin
---
Index: Plugins.properties
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>NATIVE_TO_ASCII_UTF-8
===================================================================
diff --git a/Plugins.properties b/Plugins.properties
--- a/Plugins.properties	(revision 8d12533d4a0dd8982af01c956d72a844e86f177d)
+++ b/Plugins.properties	(revision 1a5bae2511b0260dd5ee55428d4d540bd8e1a187)
@@ -48,6 +48,7 @@
     org.omegat.filters4.xml.xliff.SdlProject \
     org.omegat.tokenizer.LuceneEnglishTokenizer \
     org.omegat.tokenizer.HunspellTokenizer \
+    org.omegat.gui.mark.AltTranslationsMarker \
     org.omegat.gui.scripting.ScriptingWindow \
     org.omegat.externalfinder.ExternalFinder \
     org.omegat.gui.theme.DefaultFlatTheme \

miurahr avatar Feb 10 '24 06:02 miurahr

@chelobaka Could you please pull master branch to the topic branch here? I've implemented a set of unit tests for markers. Please add a unit test for AltTranslationsMarker?

Here is an example how it can be.

Subject: [PATCH] feat: add test case for AltTranslationsMarker
---
Index: test/data/mark/alternative.tmx
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/test/data/mark/alternative.tmx b/test/data/mark/alternative.tmx
new file mode 100644
--- /dev/null	(revision 155ede3b4d7d909ac2741e90b49b84bfb0dd97ba)
+++ b/test/data/mark/alternative.tmx	(revision 155ede3b4d7d909ac2741e90b49b84bfb0dd97ba)
@@ -0,0 +1,27 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE tmx SYSTEM "tmx11.dtd">
+<tmx version="1.1">
+    <header srclang="en" />
+    <body>
+        <tu>
+            <tuv lang="en">
+                <seg>Edit</seg>
+            </tuv>
+            <tuv lang="fr">
+                <seg>default</seg>
+            </tuv>
+        </tu>
+        <tu>
+            <prop type="file">file1</prop>
+            <prop type="id">1_1</prop>
+            <prop type="prev">prev1</prop>
+            <prop type="next">next1</prop>
+            <tuv lang="en">
+                <seg>Edit</seg>
+            </tuv>
+            <tuv lang="fr">
+                <seg>alternative</seg>
+            </tuv>
+        </tu>
+    </body>
+</tmx>
\ No newline at end of file
Index: test/src/org/omegat/gui/editor/mark/AltTranslationsMarkerTest.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/test/src/org/omegat/gui/editor/mark/AltTranslationsMarkerTest.java b/test/src/org/omegat/gui/editor/mark/AltTranslationsMarkerTest.java
new file mode 100644
--- /dev/null	(revision 155ede3b4d7d909ac2741e90b49b84bfb0dd97ba)
+++ b/test/src/org/omegat/gui/editor/mark/AltTranslationsMarkerTest.java	(revision 155ede3b4d7d909ac2741e90b49b84bfb0dd97ba)
@@ -0,0 +1,122 @@
+/**************************************************************************
+ OmegaT - Computer Assisted Translation (CAT) tool
+          with fuzzy matching, translation memory, keyword search,
+          glossaries, and translation leveraging into updated projects.
+
+ Copyright (C) 2024 Lev Abashkin
+               2024 Hiroshi Miura
+               Home page: https://www.omegat.org/
+               Support center: https://omegat.org/support
+
+ This file is part of OmegaT.
+
+ OmegaT is free software: you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation, either version 3 of the License, or
+ (at your option) any later version.
+
+ OmegaT is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ **************************************************************************/
+package org.omegat.gui.editor.mark;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+
+import java.nio.file.Paths;
+import java.util.Collections;
+import java.util.List;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import org.omegat.core.Core;
+import org.omegat.core.TestCoreInitializer;
+import org.omegat.core.data.EntryKey;
+import org.omegat.core.data.NotLoadedProject;
+import org.omegat.core.data.ProjectProperties;
+import org.omegat.core.data.ProjectTMX;
+import org.omegat.core.data.SourceTextEntry;
+import org.omegat.core.data.TMXEntry;
+import org.omegat.core.segmentation.SRX;
+import org.omegat.core.segmentation.Segmenter;
+import org.omegat.util.Language;
+
+public class AltTranslationsMarkerTest  extends MarkerTestBase {
+
+    private ProjectTMX projectTMX;
+
+    @Before
+    public void preUp() throws Exception {
+        TestCoreInitializer.initEditor(editor);
+        Core.setSegmenter(new Segmenter(SRX.getDefault()));
+        projectTMX = new ProjectTMX(new Language("en"), new Language("fr"), true,
+                Paths.get("test/data/mark/alternative.tmx").toFile(), null);
+        Core.setProject(new NotLoadedProject() {
+            @Override
+            public boolean isProjectLoaded() {
+                return true;
+            }
+
+            @Override
+            public ProjectProperties getProjectProperties() {
+                return new ProjectProperties() {
+                    @Override
+                    public Language getSourceLanguage() {
+                        return new Language("en");
+                    }
+
+                    @Override
+                    public Language getTargetLanguage() {
+                        return new Language("fr");
+                    }
+                };
+            }
+
+            @Override
+            public TMXEntry getTranslationInfo(SourceTextEntry ste) {
+                if (ste == null || projectTMX == null) {
+                    return EMPTY_TRANSLATION;
+                }
+                TMXEntry r = projectTMX.getMultipleTranslation(ste.getKey());
+                if (r == null) {
+                    r = projectTMX.getDefaultTranslation(ste.getSrcText());
+                }
+                if (r == null) {
+                    r = EMPTY_TRANSLATION;
+                }
+                return r;
+            }
+
+        });
+    }
+
+    @Test
+    public void testAltTranslationsMarker() throws Exception {
+        IMarker marker = new AltTranslationsMarker();
+        Core.getEditor().getSettings().setMarkAltTranslations(true);
+        String sourceText = "Edit";
+        String translationText = "default";
+        // default entry: file and
+        EntryKey key0 = new EntryKey("file0", sourceText, "1_0", "prev0", "next0", "path");
+        SourceTextEntry ste0 = new SourceTextEntry(key0, 1, new String[0], sourceText,
+                Collections.emptyList());
+        List<Mark> result = marker.getMarksForEntry(ste0, sourceText, translationText, true);
+        assertNull(result);
+        // alternative entry: file and 1_0
+        translationText = "alternative";
+        EntryKey key1 = new EntryKey("file1", sourceText, "1_1", "prev1", "next1", null);
+        SourceTextEntry ste1 = new SourceTextEntry(key1, 1, new String[0], sourceText,
+                Collections.emptyList());
+        result = marker.getMarksForEntry(ste1, sourceText, translationText, true);
+        assertNotNull(result);
+        assertEquals(1, result.size());
+        // TODO: further checks
+    }
+}

miurahr avatar Feb 10 '24 06:02 miurahr

Here is patches files omegat-alttranslationsmarker-patches.zip for your convenience.

miurahr avatar Feb 10 '24 06:02 miurahr

Marker was registered in Plugin.properties as a marker. Moved it to plugins and added test files. Thank you!

chelobaka avatar Feb 10 '24 10:02 chelobaka

Just did a quick test, the plugin works well but as I suspected there is the problem of accumulation: if I take a segment from tm/auto, and this segment is an alternative translation, then I see only the result of your plugin, the color saying that this comes from tm/auto is recovered by yours.

t-cordonnier avatar Feb 11 '24 07:02 t-cordonnier

Now working on #1082

miurahr avatar Jul 11 '24 05:07 miurahr