Changeset View
Standalone View
app/fileoperations.cpp
Show First 20 Lines • Show All 44 Lines • ▼ Show 20 Line(s) | |||||
45 | { | 45 | { | ||
46 | 46 | | |||
47 | namespace FileOperations | 47 | namespace FileOperations | ||
48 | { | 48 | { | ||
49 | 49 | | |||
50 | static void copyMoveOrLink(Operation operation, const QList<QUrl>& urlList, QWidget* parent, ContextManager* contextManager) | 50 | static void copyMoveOrLink(Operation operation, const QList<QUrl>& urlList, QWidget* parent, ContextManager* contextManager) | ||
51 | { | 51 | { | ||
52 | Q_ASSERT(!urlList.isEmpty()); | 52 | Q_ASSERT(!urlList.isEmpty()); | ||
53 | const int numberOfImages = urlList.count(); | ||||
53 | 54 | | |||
54 | QFileDialog dialog(parent->nativeParentWidget(), QString()); | 55 | QFileDialog dialog(parent->nativeParentWidget(), QString()); | ||
55 | dialog.setAcceptMode(QFileDialog::AcceptSave); | 56 | dialog.setAcceptMode(QFileDialog::AcceptSave); | ||
56 | 57 | | |||
58 | // Figure out what the window title and buttons should say, | ||||
59 | // depending on the operation and how many images are selected | ||||
broulik: I don't think it's necessary to cache that in a variable | |||||
57 | switch (operation) { | 60 | switch (operation) { | ||
broulik: You can use `constFirst` | |||||
58 | case COPY: | 61 | case COPY: | ||
59 | dialog.setWindowTitle(i18nc("@title:window", "Copy To")); | 62 | if (numberOfImages == 1) { | ||
63 | dialog.setWindowTitle(i18nc("@title:window %1 file name", "Copy %1", urlList.constFirst().fileName())); | ||||
Surely this should be a function call and not an assignment? If this compiled for you (it does not for me), I would be amazed. rkflx: Surely this should be a function call and not an assignment? If this compiled for you (it does… | |||||
Surely indeed! Whoops. I suspect doing repeated incremental builds cached something such that I could compile. I'll fix this, do a clean build, and test again. ngraham: Surely indeed! Whoops. I suspect doing repeated incremental builds cached something such that I… | |||||
64 | } else { | ||||
65 | dialog.setWindowTitle(i18ncp("@title:window %1 number of images", "Copy %1 image", "Copy %1 images", numberOfImages)); | ||||
broulik: `i18np` - p is for plural, you can't assume every language has just two numeri | |||||
rkflx: For the singular, dropping `image` was a good idea. Also do this here please. | |||||
66 | } | ||||
Why do you need the if? If I understood the API docs correctly, a single call to i18ncp should be enough, shouldn't it? rkflx: Why do you need the `if`? If I understood the [API docs](https://api.kde. | |||||
Oh so much simpler. It's amazing when things work as they're supposed to! ngraham: Oh so much simpler. It's amazing when things work as they're supposed to! | |||||
Actually I don't think I can use il8ncp for both the singular and the plural. I'd need to do something like this: i18ncp("@title:window", "Copy %1", "Copy %2 images", urlList.constFirst().fileName()), numberOfImages, ...Bit then it complains about an invalid number of arguments in the format string, which I suppose isn't totally inappropriate since there are two variables but each string only consumes one of them. Can you think of a way around this? ngraham: Actually I don't think I can use `il8ncp` for both the singular and the plural. I'd need to do… | |||||
I mean, it actually does work, but the pluralized string gets "I18N_EXCESS_ARGUMENTS_SUPPLIED" appended to the end. https://cgit.kde.org/ki18n.git/tree/src/klocalizedstring.cpp#n775 I'm tempted to report this as a bug, since in this particular use case it does actually work fine other than the error string that gets automatically appended. ngraham: I mean, it actually does work, but the pluralized string gets "I18N_EXCESS_ARGUMENTS_SUPPLIED"… | |||||
Reported as https://bugs.kde.org/show_bug.cgi?id=386053. So for the time being I think I do need to keep the manual check for one item, or else I'll have to sacrifice showing the filename when only one item is selected. Thoughts? ngraham: Reported as https://bugs.kde.org/show_bug.cgi?id=386053. So for the time being I think I do… | |||||
Are you sure? This is working fine for me: dialog.setWindowTitle(i18ncp("@title:window", "Foo - Copy %1", "Bar - Copy %2 images", urlList.constFirst().fileName(), numberOfImages)); Did you make install? Also, at least on Phab, your code snippet had wrong parentheses… Could you check in a different environment/user/distro? rkflx: Are you sure? This is working fine for me:
dialog.setWindowTitle(i18ncp("@title… | |||||
60 | dialog.setLabelText(QFileDialog::DialogLabel::Accept, i18nc("@action:button", "Copy")); | 67 | dialog.setLabelText(QFileDialog::DialogLabel::Accept, i18nc("@action:button", "Copy")); | ||
61 | break; | 68 | break; | ||
62 | case MOVE: | 69 | case MOVE: | ||
63 | dialog.setWindowTitle(i18nc("@title:window", "Move To")); | 70 | if (numberOfImages == 1) { | ||
71 | dialog.setWindowTitle(i18nc("@title:window %1 file name", "Move %1", urlList.constFirst().fileName())); | ||||
Only the context and singular string are used as message key, so both messages would appear as single message to translator. Context should be used to disambiguate this, e.g. to this message "@title:window %1 file name". ilic: Only the context and singular string are used as message key, so both messages would appear as… | |||||
72 | } else { | ||||
73 | dialog.setWindowTitle(i18ncp("@title:window %1 number of images", "Move %1 image", "Move %1 images", numberOfImages)); | ||||
74 | } | ||||
64 | dialog.setLabelText(QFileDialog::DialogLabel::Accept, i18nc("@action:button", "Move")); | 75 | dialog.setLabelText(QFileDialog::DialogLabel::Accept, i18nc("@action:button", "Move")); | ||
65 | break; | 76 | break; | ||
66 | case LINK: | 77 | case LINK: | ||
67 | dialog.setWindowTitle(i18nc("@title:window", "Link To")); | 78 | if (numberOfImages == 1) { | ||
79 | dialog.setWindowTitle(i18nc("@title:window %1 file name", "Link %1", urlList.constFirst().fileName())); | ||||
ilic: Same here. | |||||
80 | } else { | ||||
81 | dialog.setWindowTitle(i18ncp("@title:window %1 number of images", "Link %1 image", "Link %1 images", numberOfImages)); | ||||
82 | } | ||||
68 | dialog.setLabelText(QFileDialog::DialogLabel::Accept, i18nc("@action:button", "Link")); | 83 | dialog.setLabelText(QFileDialog::DialogLabel::Accept, i18nc("@action:button", "Link")); | ||
69 | break; | 84 | break; | ||
70 | default: | 85 | default: | ||
71 | Q_ASSERT(0); | 86 | Q_ASSERT(0); | ||
72 | } | 87 | } | ||
73 | 88 | | |||
74 | if (urlList.count() == 1) { | 89 | if (numberOfImages == 1) { | ||
75 | dialog.setFileMode(QFileDialog::AnyFile); | 90 | dialog.setFileMode(QFileDialog::AnyFile); | ||
76 | dialog.selectUrl(urlList.first()); | 91 | dialog.selectUrl(urlList.constFirst()); | ||
77 | } else { | 92 | } else { | ||
78 | dialog.setFileMode(QFileDialog::Directory); | 93 | dialog.setFileMode(QFileDialog::Directory); | ||
79 | dialog.setOption(QFileDialog::ShowDirsOnly, true); | 94 | dialog.setOption(QFileDialog::ShowDirsOnly, true); | ||
80 | } | 95 | } | ||
That's a word puzzle :) you cannot assume the operator comes first in every language (e.g. German would be "Foo verschieben" ) better use an individual string for each case even if it means having 6 different i18n statements. broulik: That's a word puzzle :) you cannot assume the operator comes first in every language (e.g. | |||||
81 | 96 | | |||
82 | dialog.setDirectoryUrl(contextManager->targetUrl().adjusted(QUrl::RemoveFilename)); | 97 | dialog.setDirectoryUrl(contextManager->targetUrl().adjusted(QUrl::RemoveFilename)); | ||
83 | if (!dialog.exec()) { | 98 | if (!dialog.exec()) { | ||
84 | return; | 99 | return; | ||
85 | } | 100 | } | ||
86 | 101 | | |||
87 | QUrl destUrl = dialog.selectedUrls().first(); | 102 | QUrl destUrl = dialog.selectedUrls().first(); | ||
88 | contextManager->setTargetUrl(destUrl); | 103 | contextManager->setTargetUrl(destUrl); | ||
▲ Show 20 Lines • Show All 141 Lines • Show Last 20 Lines |
I don't think it's necessary to cache that in a variable