Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add "Reload File" feature #625

Merged
merged 2 commits into from
Aug 27, 2023
Merged

Add "Reload File" feature #625

merged 2 commits into from
Aug 27, 2023

Conversation

jdpurcell
Copy link
Contributor

Implements #587

@jdpurcell jdpurcell mentioned this pull request Aug 27, 2023
Closed
jdpurcell
jdpurcell commented Aug 27, 2023
View reviewed changes
src/qvimagecore.cpp
@@ -68,7 +68,7 @@ QVImageCore::QVImageCore(QObject *parent) : QObject(parent)
settingsUpdated();
}

void QVImageCore::loadFile(const QString &fileName)
void QVImageCore::loadFile(const QString &fileName, bool isReloading)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cache key includes the file size, but this ensures it handles situations in which the size doesn't change (e.g. an uncompressed BMP was re-saved).

jurplel
jurplel requested changes Aug 27, 2023
View reviewed changes
src/qvimagecore.h Outdated
@@ -60,7 +60,7 @@ class QVImageCore : public QObject

explicit QVImageCore(QObject *parent = nullptr);

void loadFile(const QString &fileName);
void loadFile(const QString &fileName, bool isReloaing = false);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed!

src/shortcutmanager.cpp
@@ -44,6 +44,7 @@ void ShortcutManager::initializeShortcutsList()
{
shortcutsList.append({tr("Open"), "open", keyBindingsToStringList(QKeySequence::Open), {}});
shortcutsList.append({tr("Open URL"), "openurl", QStringList(QKeySequence(Qt::CTRL | Qt::SHIFT | Qt::Key_O).toString()), {}});
shortcutsList.append({tr("Reload File"), "reloadfile", keyBindingsToStringList(QKeySequence::Refresh), {}});
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indeed this is cmd+r on macOS, but this conflicts with rename. I don't know which should be prioritized--maybe refresh is more important.

Make sure this doesn't conflict on other platforms too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, you can take a look at my latest commit and see if you like that idea or have a different suggestion perhaps.

@jurplel
Copy link
Owner

jurplel commented Aug 27, 2023

Great

@jurplel jurplel merged commit 6e091fd into jurplel:master Aug 27, 2023
6 checks passed
@jdpurcell jdpurcell deleted the reloadimage branch August 28, 2023 02:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jurplel jurplel jurplel requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jdpurcell @jurplel