Use same logic for "no extension" case with Duplicate feature
ClosedPublic

Authored by ngraham on Mar 23 2020, 7:52 PM.

Details

Summary

In the "no extension" case, we weren't separating out the path and the original filename,
breaking the feature for languages where the word "copy" would be at the beginning of the
filename, not after it (e.g. "copia de foo" in Spanish, and similar in other romance
languages). This patch fixes that by separating the original path and filename in the no
extension case as is done for the other case, which should solve the issue.

BUG: 419070
FIXED-IN: 20.04.0

Test Plan

No changes in English; should fix the issue in Spanish once new translations are done
(see https://bugs.kde.org/show_bug.cgi?id=419070 for details)

Diff Detail

Repository
R318 Dolphin
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
ngraham created this revision.Mar 23 2020, 7:52 PM
Restricted Application added a project: Dolphin. · View Herald TranscriptMar 23 2020, 7:52 PM
Restricted Application added a subscriber: kfm-devel. · View Herald Transcript
ngraham requested review of this revision.Mar 23 2020, 7:52 PM

This involves a string change, but given that it's to fix a bug with the feature not working correctly, I'd like to request a a string change exception from the Localization team.

Is there a reason to include the path in the translated string? Not including it would reduce the risk of translations breaking the feature (as it happens currently with Spanish in the extension case)

I thought that it was important to put all the components of a final string together in the i18n() function to prevent string puzzles. If that's not a concern here, I can remove the path from the translated part of the string.

Seems fine to me, adding @pino who could give more insights.

No response, I guess it's fine.

pino added a comment.Apr 2 2020, 3:57 PM

No response, I guess it's fine.

Sorry, I missed that, also because slightly busier lately.

What about asking to the translators list (yes, writing, not doing random tagging in phab), instead of pinging me directly as sole reference for i18n issues?

ngraham updated this revision to Diff 79151.Apr 2 2020, 4:04 PM

Provisional: don't include the untranslated/untranslatable path component into the final translated string

In D28227#640235, @pino wrote:

Sorry, I missed that, also because slightly busier lately.

No worries!

random tagging in phab

That's how development using Phabricator works. Tagging Localization for opinions on whether it's appropriate to do something like this:

duplicateURL.setPath(originalDirectoryPath + i18nc("<filename> copy", "%1 copy", originalFileName));

Ordinarily I'd say this is a string puzzle, but originalDirectoryPath never needs to be localized, so is it okay?

aacid added a subscriber: aacid.Apr 2 2020, 9:42 PM
aacid added inline comments.
src/views/dolphinview.cpp
740

this is a case in which I think you even want more puzzle. getting this translated to %2%1copy makes no sense, the only thing you want translated here is "%1 copy" and then append the extenstion to that, the extension always has to be at the end right? That's something the filesystem mandates, not the translation

ngraham planned changes to this revision.Apr 2 2020, 11:39 PM
ngraham updated this revision to Diff 79179.Apr 2 2020, 11:43 PM
ngraham marked an inline comment as done.

Make the puzzle simultaneously more and less puzzling, depending on your point of view

aacid added inline comments.Apr 4 2020, 1:58 PM
src/views/dolphinview.cpp
740

i tihnk i would remove the .<extension> part, it's not part of the translatable string so it can be a bit confusing, also it's not like it gives translators anything they can work with, no?

I mean why would it translate differently whether the filename has an extension or not?

ngraham updated this revision to Diff 79307.Apr 4 2020, 3:08 PM
ngraham marked an inline comment as done.

Correct translation context string

aacid added a comment.Apr 5 2020, 4:42 PM

I'm happy with the i18n side

elvisangelaccio accepted this revision.Apr 5 2020, 9:40 PM

Let's go for it then :)

This revision is now accepted and ready to land.Apr 5 2020, 9:40 PM
This revision was automatically updated to reflect the committed changes.