| aacid | |
| ltoscano | |
| shaforostoff | |
| mlaurent |
| Localization |
Entries from deleted files are always shown in the Translation Memory. Through an option in the Configuration of the Translation Memory, this feature allows three things :
| Automatic diff as part of commit; lint not applicable. |
| Automatic diff as part of commit; unit tests not applicable. |
Hello,
This is my first feature contribution for a KDE, I apologize in advance if some things are done incorrectly. Any advice is welcome.
| src/tm/jobs.h | ||
|---|---|---|
| 207 | space after () and before { | |
| 222 | it seems a good idea to add a parent to job as RemoveFileJob(...., QObject *parent = nullptr) | |
| 224 | same | |
| src/tm/tmscanapi.cpp | ||
| 105 ↗ | (On Diff #37627) | not necessary to add an empty line in this commit |
| src/tm/tmtab.cpp | ||
| 56 | it must be in #ifndef NOKDE no ? | |
I rewrote the file deletion algorithm because removing each entry from the words table takes too long due to the text research in BLOBs (it took one hour to run on the kde applications .po directory).
Now instead of calling the entry removal, it just deletes the file data in one query per table (source_strings, target_strings, main, files). The entry stays in the words but this shouldn't have any side effect.
For the comment on
#include <klocalizedstring.h> #include <kmessagebox.h>
being left out of the #ifndef KDE
I followed what was done in editortab.cpp, I'm not sure of the effect of leaving it in or out of the directive but it looked like the safest approach.
| src/tm/tmtab.cpp | ||
|---|---|---|
| 56 | I actually followed what was done in editortab.cpp for this one | |
| src/tm/tmtab.cpp | ||
|---|---|---|
| 56 | so it's a bug in source, but even if it's a bug in other file it's not a good idea to continue to make it :) > for me we need to move in "#ifndef NOKDE" | |
What is a "Deleted file"? A file that did exist and does not exist anymore?
What is the rationale for removing a translation from the translation memory? The fact that a file existed and now doesn't exist anymore doesn't make its translations any less valid/useful as part of the translation memory.
For instance in the context of the translation of KDE, files sometimes move from one folder to another, or their name change (e.g. all the .desktop.po files changed to ._desktop recently).
In that case all the entries will be duplicated and if you click on an entry in the TM to get more context it will more often than not try to open a non-existing file.
So I think this is rather situationnal and depends on translators preferences, which is why it is disabled by default.
If you think this would be better as a setting at the TM level instead of the app level I probably could move it.
Do i understand it correctly that we only remove the file on
openFile and on right click?
You mention rescan but i don't see where it happens for rescan.
Also to me it seems that you copied and pasted a sizable chunk of code in three places, can we get that in one place only?
| src/prefs/lokalize.kcfg | ||
|---|---|---|
| 120 | This is not used? | |
| src/tm/tmtab.cpp | ||
| 535 | Nitpicking seems the spacing is a bit off here. | |
TMTab::openFile() and TMView::deleteFile() still share a sizeable code chunk.
Do you think merging them somehow makes sense?
I've simplified a bit by moving the file existence check into Project()
It will be complicated to merge more (popup test is different, one of the function has a connect call on the job).