| ngraham | |
| dfaure | |
| elvisangelaccio |
| Dolphin | |
| Frameworks |
BUG: 387449
In Dolphin, when a user tries to create a folder named "." or "..", they are presented with a dialog that informs them that since the folder name they entered begins with a ".", the folder they are about to create will be hidden. This is, of course, misleading. This patch instead presents them with a similar dialog, informing them of the error and asking them for a new name.
Before:
After:
| Lint Skipped |
| Unit Tests Skipped |
| src/filewidgets/knewfilemenu.cpp | ||
|---|---|---|
| 874 |
or for anything else. | |
Changed QMessageBox::Warning to QMessageBox::Critical and updated the error message.
New:
| src/filewidgets/knewfilemenu.cpp | ||
|---|---|---|
| 873 | I get newfilemenu.cpp:873:74: error: ‘Error’ is not a member of ‘QMessageBox’ QMessageBox::Critical seems to work though. | |
Nice, I think this is looking great. +1 on the latest wording. A lovely first patch!
To land it for you, we'll need your full name and email address, so it would be great if you could provide that now. In the future, if you submit your patch using arc (see https://community.kde.org/Infrastructure/Phabricator#Using_Arcanist_to_post_patches) this will happen automatically via the git authorship information, so we won't have to bug you.
Any comments from the Frameworks folks?
| src/filewidgets/knewfilemenu.cpp | ||
|---|---|---|
| 873 | Correct: QMessageBox::Error does not exist; it's QMessageBox::Critical. See https://doc.qt.io/qt-5/qmessagebox.html#Icon-enum | |
Hmm, shouldn't this check be at the kio_file level @dfaure ?
If we put it here, it will still fail when you try to create a folder named "." not by using the knewfilemenu in dolphin.
@elvisangelaccio The check has to come before the check for name.startsWith('.') because that's what launches the "The name <name> starts with a ." dialog.
Well, it makes sense for this check to be at the GUI level, the user interaction can be better adapted then, as in this patch.
KIO::mkpath("/home/dfaure/tmp/.") should at least not warn or error in any way IMHO.
(btw mkdir of such a path, on the command line, always fails: either the tmp dir exists and it says "already exists", or it doesn't exist and it can't create the parent dir first)
| src/filewidgets/knewfilemenu.cpp | ||
|---|---|---|
| 860 ↗ | (On Diff #36954) | Please use QLatin1String here, which is faster for string comparisons. |
| 874 ↗ | (On Diff #36954) | Please use semantic markup here: https://api.kde.org/frameworks/ki18n/html/prg_guide.html#kuit_markup tl;dr use <filename>%1</filename> instead of \"%1\" and <nl/> instead of \n |
| 876 ↗ | (On Diff #36954) | Please use QString() here |
| 884 ↗ | (On Diff #36954) | m_text = QString(), like we already do in _k_slotAbortDialog(). |
Sorry for the delay @ngraham . I've been traveling. I'll have these changes complete by the end of today.
