Added property to ExportManager to hold timestamp of screenshot that is then used when creating the file name instead of the file's save timestamp.
BUG: 394504
FIXED-IN: 18.12.0
ngraham |
Spectacle |
Added property to ExportManager to hold timestamp of screenshot that is then used when creating the file name instead of the file's save timestamp.
BUG: 394504
FIXED-IN: 18.12.0
Automatic diff as part of commit; lint not applicable. |
Automatic diff as part of commit; unit tests not applicable. |
Thanks for this patch! Sorry for the extended review time.
Unfortunately, it doesn't apply against the current state of git master when I attempt to apply it with arc patch D15106:
Created and checked out branch arcpatch-D15106. Checking patch src/SpectacleCore.cpp... Hunk #1 succeeded at 183 (offset 1 line). Checking patch src/ExportManager.h... Checking patch src/ExportManager.cpp... error: while searching for: KSharedConfigPtr config = KSharedConfig::openConfig(QStringLiteral("spectaclerc")); KConfigGroup generalConfig = KConfigGroup(config, "General"); const QDateTime timestamp = QDateTime::currentDateTime(); QString baseName = generalConfig.readEntry("save-filename-format", "Screenshot_%Y%M%D_%H%m%S"); QString title; error: patch failed: src/ExportManager.cpp:173 Applied patch src/SpectacleCore.cpp cleanly. Applied patch src/ExportManager.h cleanly. Applying patch src/ExportManager.cpp with 1 reject... Hunk #1 applied cleanly. Rejected hunk #2. Patch Failed! Usage Exception: Unable to apply patch!
Does this patch assume and require D15059 first?
Can you rebase it and update the patch? You can do that with git pull --rebase.
Also, since it seems like you're interested in helping out with Spectacle and submitting patches, allow me to recommend setting up the arc command-line tool, which makes submitting and updating patches a breeze (har har har). It's really easy: https://community.kde.org/Infrastructure/Phabricator#Using_Arcanist_to_post_patches
I can offer assistance if you need a hand.
I have reverted the commit that was causing the issue on my local feature branch which I hope allows the diff to work. If it doesn't, I'll go one step further and start a new branch from the upstream repository and reapply the changes manually.
Thank you for your tips and patience with this. I'm new to all of this and didn't quite get the git workflow correct the first time. I was able to install arc and will plan to use that with future diffs.
Hmm, still doesn't work:
$ (master) arc patch D15106 Created and checked out branch arcpatch-D15106. Checking patch src/SpectacleCore.cpp... Hunk #1 succeeded at 183 (offset 1 line). Checking patch src/ExportManager.h... Checking patch src/ExportManager.cpp... error: while searching for: KSharedConfigPtr config = KSharedConfig::openConfig(QStringLiteral("spectaclerc")); KConfigGroup generalConfig = KConfigGroup(config, "General"); const QDateTime timestamp = QDateTime::currentDateTime(); QString baseName = generalConfig.readEntry("save-filename-format", "Screenshot_%Y%M%D_%H%m%S"); QString title; error: patch failed: src/ExportManager.cpp:173 Applied patch src/SpectacleCore.cpp cleanly. Applied patch src/ExportManager.h cleanly. Applying patch src/ExportManager.cpp with 1 reject... Hunk #1 applied cleanly. Rejected hunk #2. Patch Failed! Usage Exception: Unable to apply patch!
You should be able to just re-base your patch on master with git pull --rebase origin master.
Using arc should help, too.
Better now, thanks! And sorry for the extended review time.
This works well and the code looks sane. I'll land it in 18.08.2 since it's a bugfix and doesn't involve any string changes.