diff --git a/src/config/filenameeditor.cpp b/src/config/filenameeditor.cpp index df6ca298..3e4c812a 100644 --- a/src/config/filenameeditor.cpp +++ b/src/config/filenameeditor.cpp @@ -92,7 +92,7 @@ void FileNameEditor::initWidgets() void FileNameEditor::savePattern() { QString pattern = m_nameEditor->text(); - m_nameHandler->setPattern(pattern); + ConfigHandler().setFilenamePattern(pattern); } void FileNameEditor::showParsedPattern(const QString& p) diff --git a/src/core/capturerequest.cpp b/src/core/capturerequest.cpp index 7087405c..f5874b2e 100644 --- a/src/core/capturerequest.cpp +++ b/src/core/capturerequest.cpp @@ -73,7 +73,7 @@ void CaptureRequest::exportCapture(const QPixmap& p) if (m_path.isEmpty()) { ScreenshotSaver(m_id).saveToFilesystemGUI(p); } else { - ScreenshotSaver(m_id).saveToFilesystem(p, m_path, ""); + ScreenshotSaver(m_id).saveToFilesystem(p, m_path); } } diff --git a/src/core/flameshotdbusadapter.cpp b/src/core/flameshotdbusadapter.cpp index 768c274e..1643b687 100644 --- a/src/core/flameshotdbusadapter.cpp +++ b/src/core/flameshotdbusadapter.cpp @@ -50,9 +50,7 @@ void FlameshotDBusAdapter::fullScreen(QString path, if (toClipboard) { req.addTask(CaptureRequest::CLIPBOARD_SAVE_TASK); } - if (!path.isEmpty()) { - req.addTask(CaptureRequest::FILESYSTEM_SAVE_TASK); - } + req.addTask(CaptureRequest::FILESYSTEM_SAVE_TASK); req.setStaticID(id); Controller::getInstance()->requestCapture(req); } @@ -72,9 +70,7 @@ void FlameshotDBusAdapter::captureScreen(int number, if (toClipboard) { req.addTask(CaptureRequest::CLIPBOARD_SAVE_TASK); } - if (!path.isEmpty()) { - req.addTask(CaptureRequest::FILESYSTEM_SAVE_TASK); - } + req.addTask(CaptureRequest::FILESYSTEM_SAVE_TASK); req.setStaticID(id); Controller::getInstance()->requestCapture(req); } diff --git a/src/main.cpp b/src/main.cpp index 427ddbda..c6334c8b 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -148,7 +148,7 @@ int main(int argc, char* argv[]) // Options CommandOption pathOption( { "p", "path" }, - QObject::tr("Path where the capture will be saved"), + QObject::tr("Existing directory or new file to save to"), QStringLiteral("path")); CommandOption clipboardOption( { "c", "clipboard" }, QObject::tr("Save the capture to the clipboard")); @@ -213,14 +213,17 @@ int main(int argc, char* argv[]) }; const QString pathErr = - QObject::tr("Invalid path, it must be a real path in the system"); + QObject::tr("Invalid path, must be an existing directory or a new file " + "in an existing directory"); auto pathChecker = [pathErr](const QString& pathValue) -> bool { - bool res = QDir(pathValue).exists(); - if (!res) { + QFileInfo fileInfo(pathValue); + if (fileInfo.isDir() || fileInfo.dir().exists()) { + return true; + } else { SystemNotification().sendMessage( QObject::tr(pathErr.toLatin1().data())); + return false; } - return res; }; const QString booleanErr = @@ -290,7 +293,7 @@ int main(int argc, char* argv[]) } sessionBus.call(m); } else if (parser.isSet(guiArgument)) { // GUI - QString pathValue = parser.value(pathOption); + QString pathValue = QDir(parser.value(pathOption)).absolutePath(); int delay = parser.value(delayOption).toInt(); bool toClipboard = parser.isSet(clipboardOption); bool isRaw = parser.isSet(rawImageOption); @@ -321,7 +324,7 @@ int main(int argc, char* argv[]) return waitAfterConnecting(delay, app); } } else if (parser.isSet(fullArgument)) { // FULL - QString pathValue = parser.value(pathOption); + QString pathValue = QDir(parser.value(pathOption)).absolutePath(); int delay = parser.value(delayOption).toInt(); bool toClipboard = parser.isSet(clipboardOption); bool isRaw = parser.isSet(rawImageOption); @@ -376,7 +379,7 @@ int main(int argc, char* argv[]) QString numberStr = parser.value(screenNumberOption); int number = numberStr.startsWith(QLatin1String("-")) ? -1 : numberStr.toInt(); - QString pathValue = parser.value(pathOption); + QString pathValue = QDir(parser.value(pathOption)).absolutePath(); int delay = parser.value(delayOption).toInt(); bool toClipboard = parser.isSet(clipboardOption); bool isRaw = parser.isSet(rawImageOption); diff --git a/src/tools/launcher/applauncherwidget.cpp b/src/tools/launcher/applauncherwidget.cpp index 6d5171f6..7817523a 100644 --- a/src/tools/launcher/applauncherwidget.cpp +++ b/src/tools/launcher/applauncherwidget.cpp @@ -88,7 +88,7 @@ void AppLauncherWidget::launch(const QModelIndex& index) { if (!QFileInfo(m_tempFile).isReadable()) { m_tempFile = - FileNameHandler().generateAbsolutePath(QDir::tempPath()) + ".png"; + FileNameHandler().properScreenshotPath(QDir::tempPath(), "png"); bool ok = m_pixmap.save(m_tempFile); if (!ok) { QMessageBox::about( diff --git a/src/tools/launcher/openwithprogram.cpp b/src/tools/launcher/openwithprogram.cpp index c36ae23e..0e5e00df 100644 --- a/src/tools/launcher/openwithprogram.cpp +++ b/src/tools/launcher/openwithprogram.cpp @@ -23,7 +23,7 @@ void showOpenWithMenu(const QPixmap& capture) { #if defined(Q_OS_WIN) QString tempFile = - FileNameHandler().generateAbsolutePath(QDir::tempPath()) + ".png"; + FileNameHandler().properScreenshotPath(QDir::tempPath(), "png"); bool ok = capture.save(tempFile); if (!ok) { QMessageBox::about(nullptr, diff --git a/src/tools/save/savetool.cpp b/src/tools/save/savetool.cpp index 570b19ed..3d5c2574 100644 --- a/src/tools/save/savetool.cpp +++ b/src/tools/save/savetool.cpp @@ -67,7 +67,7 @@ void SaveTool::pressed(const CaptureContext& context) } } else { bool ok = ScreenshotSaver().saveToFilesystem( - context.selectedScreenshotArea(), context.savePath, ""); + context.selectedScreenshotArea(), context.savePath); if (ok) { emit requestAction(REQ_CAPTURE_DONE_OK); } diff --git a/src/utils/filenamehandler.cpp b/src/utils/filenamehandler.cpp index e629e063..f678bd1a 100644 --- a/src/utils/filenamehandler.cpp +++ b/src/utils/filenamehandler.cpp @@ -5,7 +5,6 @@ #include "src/utils/confighandler.h" #include "src/utils/strfparse.h" #include -#include #include #include #include @@ -52,65 +51,75 @@ QString FileNameHandler::parseFilename(const QString& name) return res; } -QString FileNameHandler::generateAbsolutePath(const QString& path) +/** + * @brief Generate a valid destination path from the possibly incomplete `path`. + * The input `path` can be one of: + * - empty string + * - an existing directory + * - a file in an existing directory + * In each case, the output path will be an absolute path to a file with a + * suffix matching the specified `format`. + * @note + * - If `path` points to a directory, the file name will be generated from the + * formatted file name from the user configuration + * - If `path` points to a file, its suffix will be changed to match `format` + * - If `format` is not given, the suffix will remain untouched, unless `path` + * has no suffix, in which case it will be given the "png" suffix + * - If the path generated by the previous steps points to an existing file, + * "_NUM" will be appended to its base name, where NUM is the first + * available number that produces a non-existent path (starting from 1). + * @param path Possibly incomplete file name to transform + * @param format Desired output file suffix (excluding an initial '.' character) + */ +QString FileNameHandler::properScreenshotPath(QString path, + const QString& format) { - QString directory = path; - QString filename = parsedPattern(); - fixPath(directory, filename); - return directory + filename; -} -// path a images si no existe, add numeration -void FileNameHandler::setPattern(const QString& pattern) -{ - ConfigHandler().setFilenamePattern(pattern); -} + QFileInfo info(path); + QString suffix = info.suffix(); -QString FileNameHandler::absoluteSavePath(QString& directory, QString& filename) -{ - ConfigHandler config; - directory = config.savePath(); - if (directory.isEmpty() || !QDir(directory).exists() || - !QFileInfo(directory).isWritable()) { - directory = - QStandardPaths::writableLocation(QStandardPaths::PicturesLocation); + if (info.isDir()) { + // path is a directory => generate filename from configured pattern + path = QDir(QDir(path).absolutePath() + "/" + parsedPattern()).path(); + } else { + // path points to a file => strip it of its suffix for now + path = QDir(info.dir().absolutePath() + "/" + info.completeBaseName()) + .path(); } - filename = parsedPattern(); - fixPath(directory, filename); - return directory + filename; -} -QString FileNameHandler::absoluteSavePath() -{ - QString dir, file; - return absoluteSavePath(dir, file); -} - -QString FileNameHandler::charArrToQString(const char* c) -{ - return QString::fromLocal8Bit(c, MAX_CHARACTERS); -} - -char* FileNameHandler::QStringToCharArr(const QString& s) -{ - QByteArray ba = s.toLocal8Bit(); - return const_cast(strdup(ba.constData())); -} - -void FileNameHandler::fixPath(QString& directory, QString& filename) -{ - // add '/' at the end of the directory - if (!directory.endsWith(QLatin1String("/"))) { - directory += QLatin1String("/"); + if (!format.isEmpty()) { + // Override suffix to match format + path += "." + format; + } else if (!suffix.isEmpty()) { + // Leave the suffix as it was + path += "." + suffix; + } else { + path += ".png"; } + + if (!QFileInfo::exists(path)) { + return path; + } else { + return autoNumerateDuplicate(path); + } +} + +QString FileNameHandler::autoNumerateDuplicate(QString path) +{ // add numeration in case of repeated filename in the directory // find unused name adding _n where n is a number - QFileInfo checkFile(directory + filename + ".png"); + QFileInfo checkFile(path); + QString directory = checkFile.dir().absolutePath(), + filename = checkFile.completeBaseName(), + suffix = checkFile.suffix(); + if (!suffix.isEmpty()) { + suffix = QStringLiteral(".") + suffix; + } if (checkFile.exists()) { filename += QLatin1String("_"); int i = 1; while (true) { - checkFile.setFile(directory + filename + QString::number(i) + - ".png"); + checkFile.setFile(directory + "/" + filename + QString::number(i) + + suffix); if (!checkFile.exists()) { filename += QString::number(i); break; @@ -118,4 +127,5 @@ void FileNameHandler::fixPath(QString& directory, QString& filename) ++i; } } + return checkFile.filePath(); } diff --git a/src/utils/filenamehandler.h b/src/utils/filenamehandler.h index 1c1068fe..921029ba 100644 --- a/src/utils/filenamehandler.h +++ b/src/utils/filenamehandler.h @@ -13,19 +13,12 @@ public: QString parsedPattern(); QString parseFilename(const QString& name); - QString generateAbsolutePath(const QString& path); - QString absoluteSavePath(QString& directory, QString& filename); - QString absoluteSavePath(); + + QString properScreenshotPath(QString filename, + const QString& format = QString()); static const int MAX_CHARACTERS = 70; -public slots: - void setPattern(const QString& pattern); - private: - // using charArr = char[MAX_CHARACTERS]; - QString charArrToQString(const char* c); - char* QStringToCharArr(const QString& s); - - void fixPath(QString& directory, QString& filename); + QString autoNumerateDuplicate(QString path); }; diff --git a/src/utils/screengrabber.cpp b/src/utils/screengrabber.cpp index 90927c89..7c6c07ab 100644 --- a/src/utils/screengrabber.cpp +++ b/src/utils/screengrabber.cpp @@ -43,9 +43,8 @@ QPixmap ScreenGrabber::grabEntireDesktop(bool& ok) switch (m_info.windowManager()) { case DesktopInfo::GNOME: { // https://github.com/GNOME/gnome-shell/blob/695bfb96160033be55cfb5ac41c121998f98c328/data/org.gnome.Shell.Screenshot.xml - QString path = - FileNameHandler().generateAbsolutePath(QDir::tempPath()) + - ".png"; + QString path = FileNameHandler().properScreenshotPath( + QDir::tempPath(), "png"); QDBusInterface gnomeInterface( QStringLiteral("org.gnome.Shell"), QStringLiteral("/org/gnome/Shell/Screenshot"), diff --git a/src/utils/screenshotsaver.cpp b/src/utils/screenshotsaver.cpp index 796c8cf8..1c37f6a6 100644 --- a/src/utils/screenshotsaver.cpp +++ b/src/utils/screenshotsaver.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #if defined(Q_OS_MACOS) @@ -87,8 +88,7 @@ bool ScreenshotSaver::saveToFilesystem(const QPixmap& capture, const QString& path, const QString& messagePrefix) { - QString completePath = FileNameHandler().generateAbsolutePath(path); - completePath += QLatin1String(".png"); + QString completePath = FileNameHandler().properScreenshotPath(path); bool ok = capture.save(completePath); QString saveMessage = messagePrefix; QString notificationPath = completePath; @@ -97,7 +97,6 @@ bool ScreenshotSaver::saveToFilesystem(const QPixmap& capture, } if (ok) { - ConfigHandler().setSavePath(path); saveMessage += QObject::tr("Capture saved as ") + completePath; Controller::getInstance()->sendCaptureSaved( m_id, QFileInfo(completePath).canonicalFilePath()); @@ -143,7 +142,13 @@ bool ScreenshotSaver::saveToFilesystemGUI(const QPixmap& capture) { bool ok = false; ConfigHandler config; - QString savePath = FileNameHandler().absoluteSavePath(); + QString defaultSavePath = ConfigHandler().savePath(); + if (defaultSavePath.isEmpty() || !QDir(defaultSavePath).exists() || + !QFileInfo(defaultSavePath).isWritable()) { + defaultSavePath = + QStandardPaths::writableLocation(QStandardPaths::PicturesLocation); + } + QString savePath = FileNameHandler().properScreenshotPath(defaultSavePath); #if defined(Q_OS_MACOS) for (QWidget* widget : qApp->topLevelWidgets()) { QString className(widget->metaObject()->className()); @@ -158,10 +163,7 @@ bool ScreenshotSaver::saveToFilesystemGUI(const QPixmap& capture) if (!config.savePathFixed()) { // auto imageFormats = QImageWriter::supportedImageFormats(); savePath = - ShowSaveFileDialog(nullptr, - QObject::tr("Save screenshot"), - FileNameHandler().absoluteSavePath() + - ConfigHandler().getSaveAsFileExtension()); + ShowSaveFileDialog(nullptr, QObject::tr("Save screenshot"), savePath); } if (savePath == "") { return ok; diff --git a/src/utils/screenshotsaver.h b/src/utils/screenshotsaver.h index a5fc0907..73413c1a 100644 --- a/src/utils/screenshotsaver.h +++ b/src/utils/screenshotsaver.h @@ -18,7 +18,7 @@ public: void saveToClipboardMime(const QPixmap& capture, const QString& imageType); bool saveToFilesystem(const QPixmap& capture, const QString& path, - const QString& messagePrefix); + const QString& messagePrefix = ""); bool saveToFilesystemGUI(const QPixmap& capture); private: diff --git a/src/widgets/capture/capturewidget.cpp b/src/widgets/capture/capturewidget.cpp index d36583a8..d6184dce 100644 --- a/src/widgets/capture/capturewidget.cpp +++ b/src/widgets/capture/capturewidget.cpp @@ -1593,8 +1593,7 @@ void CaptureWidget::saveScreenshot() if (m_context.savePath.isEmpty()) { ScreenshotSaver(m_id).saveToFilesystemGUI(pixmap()); } else { - ScreenshotSaver(m_id).saveToFilesystem( - pixmap(), m_context.savePath, ""); + ScreenshotSaver(m_id).saveToFilesystem(pixmap(), m_context.savePath); } close(); } diff --git a/tests/path_option.sh b/tests/path_option.sh new file mode 100644 index 00000000..af9abf60 --- /dev/null +++ b/tests/path_option.sh @@ -0,0 +1,52 @@ +#!/usr/bin/env sh + +# Before running the script make sure a flameshot daemon with a matching version +# is running + +# The first argument to this script is a path to the flameshot executable +[ -n "$1" ] && flameshot="$1" || flameshot='flameshot' + +# TODO Before proper stderr logging is implemented, you will have to look at the +# system notifications + +rm -rf /tmp/flameshot_path_test 2>/dev/null +mkdir -p /tmp/flameshot_path_test +cd /tmp/flameshot_path_test + +echo ">> Nonexistent directory. This command should give an invalid path error." +"$flameshot" screen -p blah/blah + +sleep 2 +echo ">> The output file is specified relative to PWD" +"$flameshot" screen -p relative.png + +sleep 2 +echo ">> Absolute paths work too" +"$flameshot" screen -p /tmp/flameshot_path_test/absolute.png + +sleep 2 +mkdir subdir +echo ">> Redundancy in the path will be removed" +"$flameshot" screen -p /tmp/flameshot_path_test/subdir/..///redundancy_removed.png + +sleep 2 +echo ">> If the destionation is a directory, the file name is generated from strf from the config" +"$flameshot" screen -p ./ + +sleep 2 +echo ">> If the output file has no suffix, it will be added (png)" +"$flameshot" screen -p /tmp/flameshot_path_test/without_suffix + +sleep 2 +echo ">> Other suffixes are supported, and the image format will match it" +"$flameshot" screen -p /tmp/flameshot_path_test/jpg_suffix.jpg + +sleep 2 +echo ">> If the destination path exists, it will have _NUM appended to the base name" +"$flameshot" screen -p /tmp/flameshot_path_test/absolute.png + +sleep 2 +echo ">> Same thing again but without specifying a suffix" +"$flameshot" screen -p /tmp/flameshot_path_test/absolute + +sleep 2